From 3b6fe8f073e11672b256b4326d8c12654c100d8d Mon Sep 17 00:00:00 2001 From: Abhay V Ashokan <35297280+AbhayVAshokan@users.noreply.github.com> Date: Wed, 20 Nov 2024 18:10:44 +0530 Subject: [PATCH] Added logic to handle table length and the RuboCop failures in the down 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 2e190b7f64a12911194d64f95e32556f0db9ebad. * 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 --- .github/pull_request_template.md | 27 ++++++++++ .github/workflows/auto_release.yml | 28 +++++++++++ .github/workflows/bump_version.yml | 50 +++++++++++++++++++ .neetoci/default.yml | 2 +- .rbenv | 2 +- .rubocop.yml | 2 +- .ruby-version | 2 +- .../cop/neeto/unsafe_column_deletion.rb | 12 +++++ .../cop/neeto/unsafe_table_deletion.rb | 15 +++++- .../cop/neeto/unsafe_column_deletion_spec.rb | 14 ++++++ .../cop/neeto/unsafe_table_deletion_spec.rb | 28 ++++++++++- 11 files changed, 175 insertions(+), 7 deletions(-) create mode 100644 .github/pull_request_template.md create mode 100644 .github/workflows/auto_release.yml create mode 100644 .github/workflows/bump_version.yml diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..80d5fcf --- /dev/null +++ b/.github/pull_request_template.md @@ -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** + + diff --git a/.github/workflows/auto_release.yml b/.github/workflows/auto_release.yml new file mode 100644 index 0000000..980e555 --- /dev/null +++ b/.github/workflows/auto_release.yml @@ -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 diff --git a/.github/workflows/bump_version.yml b/.github/workflows/bump_version.yml new file mode 100644 index 0000000..6cf07cb --- /dev/null +++ b/.github/workflows/bump_version.yml @@ -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"] + }) diff --git a/.neetoci/default.yml b/.neetoci/default.yml index 7ba88b4..bbb0ee2 100644 --- a/.neetoci/default.yml +++ b/.neetoci/default.yml @@ -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 diff --git a/.rbenv b/.rbenv index 351227f..fa7adc7 100644 --- a/.rbenv +++ b/.rbenv @@ -1 +1 @@ -3.2.4 +3.3.5 diff --git a/.rubocop.yml b/.rubocop.yml index 26292db..2a71f42 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -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 diff --git a/.ruby-version b/.ruby-version index 351227f..fa7adc7 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -3.2.4 +3.3.5 diff --git a/lib/rubocop/cop/neeto/unsafe_column_deletion.rb b/lib/rubocop/cop/neeto/unsafe_column_deletion.rb index 28a8171..13aa441 100644 --- a/lib/rubocop/cop/neeto/unsafe_column_deletion.rb +++ b/lib/rubocop/cop/neeto/unsafe_column_deletion.rb @@ -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) @@ -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 diff --git a/lib/rubocop/cop/neeto/unsafe_table_deletion.rb b/lib/rubocop/cop/neeto/unsafe_table_deletion.rb index 81efcd9..7146f33 100644 --- a/lib/rubocop/cop/neeto/unsafe_table_deletion.rb +++ b/lib/rubocop/cop/neeto/unsafe_table_deletion.rb @@ -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 @@ -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 diff --git a/spec/rubocop/cop/neeto/unsafe_column_deletion_spec.rb b/spec/rubocop/cop/neeto/unsafe_column_deletion_spec.rb index e7382bf..c282e8d 100644 --- a/spec/rubocop/cop/neeto/unsafe_column_deletion_spec.rb +++ b/spec/rubocop/cop/neeto/unsafe_column_deletion_spec.rb @@ -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) diff --git a/spec/rubocop/cop/neeto/unsafe_table_deletion_spec.rb b/spec/rubocop/cop/neeto/unsafe_table_deletion_spec.rb index 8ec3684..c5c6a9a 100644 --- a/spec/rubocop/cop/neeto/unsafe_table_deletion_spec.rb +++ b/spec/rubocop/cop/neeto/unsafe_table_deletion_spec.rb @@ -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