Skip to content

Commit

Permalink
[Fix #1280] Handle change_column_null for BulkChangeTable
Browse files Browse the repository at this point in the history
change_column_null is only combinable with PostgreSQL, and is only
available as change_null within a change_table block since Rails 6.1,
so target to only those cases.
  • Loading branch information
ccutrer committed May 31, 2024
1 parent f49ee58 commit e71b373
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 2 deletions.
1 change: 1 addition & 0 deletions changelog/fix_change_column_null_in_bulk_change_table.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1280](https://github.com/rubocop/rubocop-rails/issues/1280): Look for change_column_null for `BulkChangeTable`. ([@ccutrer][])
10 changes: 8 additions & 2 deletions lib/rubocop/cop/rails/bulk_change_table.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,10 @@ class BulkChangeTable < Base
MYSQL_COMBINABLE_ALTER_METHODS = %i[rename_column add_index remove_index].freeze

POSTGRESQL_COMBINABLE_TRANSFORMATIONS = %i[change_default].freeze
POSTGRESQL_COMBINABLE_TRANSFORMATIONS_SINCE_6_1 = %i[change_null].freeze

POSTGRESQL_COMBINABLE_ALTER_METHODS = %i[change_column_default].freeze
POSTGRESQL_COMBINABLE_ALTER_METHODS_SINCE_6_1 = %i[change_column_null].freeze

def on_def(node)
return unless support_bulk_alter?
Expand Down Expand Up @@ -196,7 +198,9 @@ def combinable_alter_methods
when MYSQL
COMBINABLE_ALTER_METHODS + MYSQL_COMBINABLE_ALTER_METHODS
when POSTGRESQL
COMBINABLE_ALTER_METHODS + POSTGRESQL_COMBINABLE_ALTER_METHODS
result = COMBINABLE_ALTER_METHODS + POSTGRESQL_COMBINABLE_ALTER_METHODS
result += POSTGRESQL_COMBINABLE_ALTER_METHODS_SINCE_6_1 if target_rails_version >= 6.1
result
end
end

Expand All @@ -205,7 +209,9 @@ def combinable_transformations
when MYSQL
COMBINABLE_TRANSFORMATIONS + MYSQL_COMBINABLE_TRANSFORMATIONS
when POSTGRESQL
COMBINABLE_TRANSFORMATIONS + POSTGRESQL_COMBINABLE_TRANSFORMATIONS
result = COMBINABLE_TRANSFORMATIONS + POSTGRESQL_COMBINABLE_TRANSFORMATIONS
result += POSTGRESQL_COMBINABLE_TRANSFORMATIONS_SINCE_6_1 if target_rails_version >= 6.1
result
end
end

Expand Down
75 changes: 75 additions & 0 deletions spec/rubocop/cop/rails/bulk_change_table_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,26 @@ def change
end
RUBY
end

it 'does not register an offense for multiple `change_column_null`' do
expect_no_offenses(<<~RUBY)
def change
change_column_null :users, :name, false
change_column_null :users, :address, false
end
RUBY
end

it 'does not register an offense for multiple `t.change_null`' do
expect_no_offenses(<<~RUBY)
def change
change_table :users do |t|
t.change_null :name, false
t.change_null :address, false
end
end
RUBY
end
end

context 'when database is PostgreSQL' do
Expand All @@ -430,13 +450,68 @@ def change
it_behaves_like 'offense'
it_behaves_like 'no offense for mysql'
it_behaves_like 'offense for postgresql'

it 'does not register an offense for multiple `change_column_null`' do
expect_no_offenses(<<~RUBY)
def change
change_column_null :users, :name, false
change_column_null :users, :address, false
end
RUBY
end

it 'does not register an offense for multiple `t.change_null`' do
expect_no_offenses(<<~RUBY)
def change
change_table :users do |t|
t.change_null :name, false
t.change_null :address, false
end
end
RUBY
end
end

context 'with Rails 5.1', :rails51 do
it_behaves_like 'no offense'
it_behaves_like 'no offense for mysql'
it_behaves_like 'no offense for postgresql'
end

context 'with Rails 6.1', :rails61 do
it 'registers an offense for multiple `change_column_null`' do
expect_offense(<<~RUBY)
def change
change_column_null :users, :name, false
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ You can use `change_table :users, bulk: true` to combine alter queries.
change_column_null :users, :address, false
end
RUBY
end

it 'registers an offense for multiple `t.change_null`' do
expect_offense(<<~RUBY)
def change
change_table :users do |t|
^^^^^^^^^^^^^^^^^^^ You can combine alter queries using `bulk: true` options.
t.change_null :name, false
t.change_null :address, false
end
end
RUBY
end

it 'does not register an offense for multiple `t.change_null` with `bulk: true`' do
expect_no_offenses(<<~RUBY)
def change
change_table :users, bulk: true do |t|
t.change_null :name, false
t.change_null :address, false
end
end
RUBY
end
end
end

context 'when `database.yml` is exists' do
Expand Down

0 comments on commit e71b373

Please sign in to comment.