ruby-on-railsrubymigrationrubocop

Rails safety_assured and rubocop disagreement on drop column migration


In my Rails project I am using safety_assured and rubocop gems simultaneously. I intend to run a simple migration to delete a column, something along the lines of:

# db/migrate/...
+ class RemoveEnabledFromUser < ActiveRecord::Migration[7.0]
+   def up
+     #safety_assured do
+       remove_column :users, :enabled
+     #end
+   end
+ 
+   def down
+     safety_assured do
+       add_column :users, :enabled, :boolean, default: true
+     end
+   end
+ end

# db/schema.rb
  create_table "users", force: :cascade do |t|
    # ...
    t.string "name", null: false
-   t.boolean "enabled", default: true
    # ...
  end

# app/models/User.rb
  class User < ApplicationRecord
+   self.ignored_columns += ["enabled"]
    # ...
  end

The safety_assured gem is informing me that I should include self.ignored_columns in the model, and rubocop is saying that I should remove since it does not exist in the schema (Rails/UnusedIgnoredColumns).

I'm not experienced in Rails, but I assume ActiveRecord infers the schema from querying the database (MySQL/PostgreSQL) and that db/schema.rb is just a helper for other things (seeding dev databases for example). So there is a danger of the User schema changing unexpectedly while the server is running.

Is it safe to omit ignored_columns in this situation?


Solution

  • Adding the ignored_columns in the same commit (deploy) as removing the column does not add anything.

    Assuming your deploy process does not differ much from the standard.

    With the current implementation, when you deploy your code, first the migrations will run. The old code will still be running and serving requests in order to avoid downtime.

    When the column drops, this will be a surprise to Rails (database is different than it was just a moment ago) and errors might be raised.

    After deploy finishes, the column is no longer there and ignored_columns is indeed useless as Rubocop suggests.

    The correct approach would be to first add ignored_columns for the columns you intend to remove and deploy that.

    Then and only then add the migration to remove the column and remove ignored_columns in the same commit. Then deploy that.

    Now with the first deploy Rails will start ignoring the column and you don't have to consider it anymore in your code, but it will still be there in the database in order to not break during the deploy.

    The 2nd deploy is cleanup which will remove the column (already ignored so no errors), and remove the ignored_columns (not needed since column is gone).

    Both strong_migrations (safety_assured) and Rubocop are correct here. You are just misunderstanding strong migrations. I suggest you re-read their README.md. They tell you to:

    Be sure to ignore the column:

    ...

    Deploy the code, then wrap this step in a safety_assured { ... } block.