Skip to content

Commit

Permalink
Revert instructions for removing a column with NOT NULL and no defaul…
Browse files Browse the repository at this point in the history
…t value - #197
  • Loading branch information
ankane committed Nov 3, 2024
1 parent 17945d8 commit d18024e
Show file tree
Hide file tree
Showing 7 changed files with 2 additions and 44 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

- Added `skip_databases` option
- Added warning for unsupported adapters
- Improved instructions for removing a column with `NOT NULL` and no default value
- Improved output for `db:forward`, `db:rollback`, `db:migrate:up`, and `db:migrate:down`

## 2.0.2 (2024-10-30)
Expand Down
10 changes: 0 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,6 @@ end
4. Deploy and run the migration
5. Remove the line added in step 1

Note: For columns with `NOT NULL` and no default value, remove `NOT NULL` before any other steps.

```ruby
class RemoveNotNull < ActiveRecord::Migration[7.2]
def change
change_column_null :users, :some_column, true
end
end
```

### Changing the type of a column

#### Bad
Expand Down
10 changes: 1 addition & 9 deletions lib/strong_migrations/checks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -319,19 +319,11 @@ def check_remove_column(method, *args)

code = "self.ignored_columns += #{columns.map(&:to_s).inspect}"

table_columns = connection.columns(args[0]).index_by(&:name) rescue {}
null_columns = columns.select { |c| table_columns[c.to_s] && !table_columns[c.to_s].null && table_columns[c.to_s].default.nil? }
if null_columns.any?
commands = null_columns.map { |c| command_str(:change_column_null, [args[0], c, true]) }
null_code = "First, remove NOT NULL:\n\nclass RemoveNotNull < ActiveRecord::Migration#{migration_suffix}\n def change\n #{commands.join("\n ")}\n end\nend\n\nDeploy and run the migration. "
end

raise_error :remove_column,
model: model_name(args[0]),
code: code,
command: command_str(method, args),
column_suffix: columns.size > 1 ? "s" : "",
null_code: null_code
column_suffix: columns.size > 1 ? "s" : ""
end

def check_remove_index(*args)
Expand Down
2 changes: 1 addition & 1 deletion lib/strong_migrations/error_messages.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def change
"Changing the type is safe, but setting NOT NULL is not.",

remove_column: "Active Record caches attributes, which causes problems
when removing columns. %{null_code}Ignore the column%{column_suffix}:
when removing columns. Be sure to ignore the column%{column_suffix}:
class %{model} < %{base_model}
%{code}
Expand Down
12 changes: 0 additions & 12 deletions test/migrations/remove_column.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,3 @@ def change
remove_belongs_to :users, :device
end
end

class RemoveColumnNull < TestMigration
def change
remove_column :devices, :name, :string
end
end

class RemoveColumnsNull < TestMigration
def change
remove_columns :devices, :name, :city, :country
end
end
8 changes: 0 additions & 8 deletions test/remove_column_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,4 @@ def test_remove_reference_polymorphic
def test_remove_belongs_to
assert_unsafe RemoveBelongsTo
end

def test_remove_column_null
assert_unsafe RemoveColumnNull
end

def test_remove_columns_null
assert_unsafe RemoveColumnsNull
end
end
3 changes: 0 additions & 3 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,6 @@ def connection_class
end

create_table :devices do |t|
t.string :name, null: false
t.string :city, null: false
t.string :country, null: false, default: ""
end
end

Expand Down

0 comments on commit d18024e

Please sign in to comment.