Skip to content

Commit

Permalink
Added logic to handle table length and the RuboCop failures in the do…
Browse files Browse the repository at this point in the history
…wn method (#12)

* Updated ruby version to 3.3.5

* Added spec for handling long table names

* Added logic to handle long table names

* Added spec to test whether down method will be skipped

* Added logic to skip the RuboCop rule for the down method

* Added spec for removing columns in down method

* Prevented the column deletion rule from being called inside the down method

* Release v0.1.2

* Revert "Release v0.1.2"

This reverts commit 2e190b7.

* Added pull request template

* Added GitHub workflow to auto release a new version

* Bumped ruby version in NeetoCI and RuboCop configs

* Fixed spec failures

* Cleaned up the logic to find the down method using each_ancestor

* Remove unused require statement
  • Loading branch information
AbhayVAshokan authored Nov 20, 2024
1 parent adbb949 commit 3b6fe8f
Show file tree
Hide file tree
Showing 11 changed files with 175 additions and 7 deletions.
27 changes: 27 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
- Fixes #issue-number

**Checklist**

- [ ] I have performed a self-review of my code.
- [ ] I have made corresponding changes to the documentation.
- [ ] I have added the necessary label (patch/minor/major - If package publish
is required).
- [ ] I have followed the suggested description format and styling.

**Reviewers**

<!---
------------- FORMAT FOR DESCRIPTION -------------
Prefix the change with one of these keywords:
- Added: for new features.
- Changed: for changes in existing functionality.
- Deprecated: for soon-to-be removed features.
- Removed: for now removed features.
- Fixed: for any bug fixes.
- Security: in case of vulnerabilities.
Points to note:
- The description shall be represented in bullet points.
- Add the keyword BREAKING in bold style for changes that could potentially break the engine, eg: **BREAKING**.
--->
28 changes: 28 additions & 0 deletions .github/workflows/auto_release.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: release
on:
pull_request:
branches:
- main
types:
- closed
jobs:
release-please:
permissions: write-all
runs-on: ubuntu-latest
if: >-
${{ github.event.pull_request.merged == true && contains(github.event.pull_request.labels.*.name, 'skip-version-bump') }}
steps:
- uses: actions/checkout@v2
- uses: ruby/setup-ruby@v1
with:
ruby-version: 3.3.5
- run: bundle install
# Publish
- name: publish gem
run: |
gem install gemfury -v 0.12.0
touch $HOME/.netrc
chmod 0600 $HOME/.netrc
printf "machine api.fury.io\n login udai.gupta@bigbinary.com\n password ${{ secrets.GEMFURY_API_KEY }}\nmachine git.fury.io\n login udai.gupta@bigbinary.com\n password ${{ secrets.GEMFURY_API_KEY }}\n" > $HOME/.netrc
gem build
fury push *.gem --as neeto-live
50 changes: 50 additions & 0 deletions .github/workflows/bump_version.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
name: Bump version
on:
pull_request:
branches:
- main
types:
- closed
jobs:
release:
name: Bump version
permissions: write-all
runs-on: ubuntu-latest
if: >-
${{ github.event.pull_request.merged == true && !contains(github.event.pull_request.labels.*.name, 'skip-version-bump') }}
steps:
- name: Checkout code
uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
with:
token: ${{ secrets.GITHUB_TOKEN }}

- name: Setup ruby
uses: ruby/setup-ruby@v1.195.0
with:
ruby-version: "3.3.5"
bundler-cache: true

- name: Bump version and create PR
uses: bigbinary/bump-product-version@v1.1.5
with:
labels: ${{ join(github.event.pull_request.labels.*.name, ',') }}
token: ${{ secrets.GITHUB_TOKEN }}
default_bump_label: patch

- name: Find bump version PR
uses: juliangruber/find-pull-request-action@v1
id: find-pull-request
with:
branch: bump-gem-version
state: all

- name: Add mergepr label
uses: actions/github-script@v6
with:
script: |
github.rest.issues.addLabels({
issue_number: ${{ steps.find-pull-request.outputs.number }},
owner: context.repo.owner,
repo: context.repo.repo,
labels: ["mergepr"]
})
2 changes: 1 addition & 1 deletion .neetoci/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ plan: standard-2

commands:
- checkout
- neetoci-version ruby 3.2.4
- neetoci-version ruby 3.3.5
- bundle config path 'vendor/bundle'
- cache restore
- bundle install --jobs 1
Expand Down
2 changes: 1 addition & 1 deletion .rbenv
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.2.4
3.3.5
2 changes: 1 addition & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# cspell:Disable

AllCops:
TargetRubyVersion: 3.2.4
TargetRubyVersion: 3.3.5
# RuboCop has a bunch of cops enabled by default. This setting tells RuboCop
# to ignore them, so only the ones explicitly set in this file are enabled.
DisabledByDefault: true
Expand Down
2 changes: 1 addition & 1 deletion .ruby-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.2.4
3.3.5
12 changes: 12 additions & 0 deletions lib/rubocop/cop/neeto/unsafe_column_deletion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,17 @@ class UnsafeColumnDeletion < Base
(block (send nil? :change_table _ ...) ...)
PATTERN

def_node_matcher :down_method?, <<~PATTERN
(def :down ...)
PATTERN

def on_send(node)
return unless unsafe_remove_column?(node)

node.each_ancestor do |parent|
return if down_method?(parent)
end

unsafe_remove_column?(node) do |table_name, column_name|
message = format(MSG_REMOVE_COLUMN, table_name:, column_name:)
add_offense(node, message:) unless SAFE_COLUMN_NAME_REGEX.match?(column_name)
Expand All @@ -67,6 +75,10 @@ def on_send(node)
def on_block(node)
return unless unsafe_remove_column_change_table?(node)

node.each_ancestor do |parent|
return if down_method?(parent)
end

node.each_descendant(:send) do |send_node|
next unless send_node.method_name == :remove

Expand Down
15 changes: 13 additions & 2 deletions lib/rubocop/cop/neeto/unsafe_table_deletion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,14 @@ module Neeto
# end
#
class UnsafeTableDeletion < Base
# Length of "_deprecated_on_xxxx_xx_xx" = 25, Max permitted length of table: 63
MAX_TABLE_NAME_LENGTH = 38
SAFE_TABLE_NAME_REGEX = /\w+_deprecated_on_\d{4}_\d{2}_\d{2}/
CURRENT_DATE = DateTime.now.strftime("%Y_%m_%d")

MSG = "'drop_table' is a dangerous operation. If not used correctly, " \
"it could cause irreversible data loss. You must perform " \
"'rename_table :%{table_name}, :%{table_name}_deprecated_on_#{CURRENT_DATE}' " \
"'rename_table :%{table_name}, :%{truncated_table_name}_deprecated_on_#{CURRENT_DATE}' " \
"instead. The renamed table can be safely dropped in a future migration."

RESTRICT_ON_SEND = %i[drop_table].freeze
Expand All @@ -45,11 +47,20 @@ class UnsafeTableDeletion < Base
(send nil? :drop_table ({sym str} $_) ...)
PATTERN

def_node_matcher :down_method?, <<~PATTERN
(def :down ...)
PATTERN

def on_send(node)
return unless unsafe_drop_table?(node)

node.each_ancestor do |parent|
return if down_method?(parent)
end

unsafe_drop_table?(node) do |table_name|
message = format(MSG, table_name:)
truncated_table_name = table_name.slice(0, MAX_TABLE_NAME_LENGTH)
message = format(MSG, table_name:, truncated_table_name:)
add_offense(node, message:) unless SAFE_TABLE_NAME_REGEX.match?(table_name)
end
end
Expand Down
14 changes: 14 additions & 0 deletions spec/rubocop/cop/neeto/unsafe_column_deletion_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,20 @@
expect_no_offenses(snippet)
end

it "does not register an offense when the column is dropped in the down method" do
snippet = <<~RUBY
def down
remove_column :users, :email
change_table :users do |t|
t.remove :email
end
end
RUBY

expect_no_offenses(snippet)
end

private

def offense(table_name, column_name, change_table = false)
Expand Down
28 changes: 27 additions & 1 deletion spec/rubocop/cop/neeto/unsafe_table_deletion_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,36 @@
expect_no_offenses(snippet)
end

it "registers an offense with a sliced table name when the table name is too long" do
table_name = "question_multiple_choice_option_entities"

snippet = <<~RUBY
drop_table :#{table_name}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{offense(table_name)}
RUBY

expect_offense(snippet)
truncated_table_name = table_name
.slice(0, RuboCop::Cop::Neeto::UnsafeTableDeletion::MAX_TABLE_NAME_LENGTH)

expect(offense(table_name)).to include(truncated_table_name)
end

it "does not register an offense when the table is dropped in the down method" do
snippet = <<~RUBY
def down
drop_table :users
end
RUBY

expect_no_offenses(snippet)
end

private

def offense(table_name)
message = format(RuboCop::Cop::Neeto::UnsafeTableDeletion::MSG, table_name:)
truncated_table_name = table_name.slice(0, RuboCop::Cop::Neeto::UnsafeTableDeletion::MAX_TABLE_NAME_LENGTH)
message = format(RuboCop::Cop::Neeto::UnsafeTableDeletion::MSG, table_name:, truncated_table_name:)
"Neeto/UnsafeTableDeletion: #{message}"
end
end

0 comments on commit 3b6fe8f

Please sign in to comment.