rubocop / rubocop-rails

A RuboCop extension focused on enforcing Rails best practices and coding conventions.
https://docs.rubocop.org/rubocop-rails
MIT License
821 stars 263 forks source link

`Rails/ApplicationModel` should ignore `db/migrate/**` #1342

Closed searls closed 2 months ago

searls commented 3 months ago

It's a common practice to define AR models inside of migrations in order to retain forward compatibility of the migration by avoiding loading any code from app/ (gems like strong_migrations and my own good_migrations are designed to enforce this).

Example from the codebase I'm working on:

class AddDefaultTrackingValuesToMovements < ActiveRecord::Migration[7.2]
  module Build
    class Movement < ActiveRecord::Base
      self.table_name = "build_movements"
    end
  end

  def change
    Build::Movement.update_all(default_tracking_type: nil)

    change_table :build_movements do |t|
      t.jsonb :default_tracking_values, null: true
    end
  end
end

This results in:

db/migrate/20240809113624_add_default_tracking_values_to_movements.rb:3:22: Rails/ApplicationRecord: Models should subclass `ApplicationRecord`.

Of course, if we fixed this by extending from ApplicationRecord, that'd load the model from app/models/application_record.rb, which could change in a way such that the migration couldn't be safely run again in the future. I think the cop should ignore anything under db/migrate/** as a result

Earlopain commented 3 months ago

This makes sense to me. Do you want to open a PR?

koic commented 2 months ago

This issue has been resolved in RuboCop Rails 2.26.1. Thank you!