Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added logic to handle table length and the RuboCop failures in the down method #12

Merged
merged 15 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 [email protected]\n password ${{ secrets.GEMFURY_API_KEY }}\nmachine git.fury.io\n login [email protected]\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/[email protected]
with:
ruby-version: "3.3.5"
bundler-cache: true

- name: Bump version and create PR
uses: bigbinary/[email protected]
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
Loading