-
-
Notifications
You must be signed in to change notification settings - Fork 263
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
Rails/UniqueValidationWithoutIndex errors on namespaced models #348
Comments
I am also seeing this problem on Rubocop v0.90.0. Not sure what @Timmitry means by namespaced model, but the error happens on a model using single table inheritance in my project. Here is the portion of the schema since I know that maintainers have requested the schema in the past for this issue. create_table "groups", id: :serial, force: :cascade do |t|
t.string "name", limit: 255
t.string "poc_name", limit: 255
t.string "poc_phone", limit: 255
t.string "poc_email", limit: 255
t.datetime "open_at"
t.datetime "close_at"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.string "code", limit: 255
t.text "welcome_message"
t.text "questions"
t.text "delivery_message"
t.string "salesperson_name", limit: 255
t.integer "shipping_cents", default: 0, null: false
t.string "currency", limit: 255
t.string "email_interval", limit: 255
t.integer "salesperson_id"
t.integer "organization_id"
t.boolean "shipping_optional", default: false
t.date "delivery_date"
t.string "type", default: "TemporaryGroup"
t.string "image"
t.integer "days_until_delivery"
t.float "tax", default: 0.0, null: false
end |
@Bialogs With namespaced I meant that the model is in a module, as can be seen by the And if the schema might help, here it is: create_table "measuring_point_concept_sensor_imports", force: :cascade do |t|
t.bigint "node_import_id"
t.string "name"
t.integer "channel_number"
t.string "sensor_name"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["node_import_id"], name: "index_measuring_point_concept_sensor_imports"
end |
#349 fixes an error when the related table is not found. |
I can confirm this behaviour. With my code it happens exactly after updating rubocop-rails from @Tietew: I can confirm your fix removes the error. But I am not sure if this implements the correct behaviour. See, the table does exist in our cases and is also declared in the schema.rb. Simply not running the cop against the concerned model/table seems like a hack. I believe, if I would add a validation without the unique index in such a nested class it would not be detected as an offense. If I have the time I will take a look into it myself and maybe comment your PR. |
@Jay-Schneider Yes, you're right. But my PR just fixes the error -- in my case, the table really doesn't exist. This issue seems to be false negative problem. |
Seeing this problem as well, in my case it is an extremely simple model: class Achievement < FeedItem
validates :text, presence: true, uniqueness: true
end However, since it is using STI, the associated database table is called create_table "feed_items", id: :serial, force: :cascade do |t|
t.integer "user_id"
t.integer "team_id"
t.integer "division_id"
t.string "text"
t.integer "challenge_id"
t.integer "point_value"
t.integer "flag_id"
t.string "type"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end |
This problem also occurs with Modules using ActiveRecord:
Error:
As you can see from the code-snippet, trying to disable the Cop has no effect |
@coding-bunny That's strange... if you move the RuboCop directives to only around the # rubocop:disable Rails/UniqueValidationWithoutIndex
validates :slug, uniqueness: true, format: { with: /\A[\w-]*\z/ }, length: { maximum: 255 }
# rubocop:enable Rails/UniqueValidationWithoutIndex |
Yes, it doesn't matter at all where those comments were placed.
Thank you! |
@coding-bunny I had success disabling the check at the |
Yeah but I don't want the check disabled or the file ignored. |
I have a situation in which this is coming up in which I have the proper index, but I'm using a scoped unique constraint, AND the table is in a secondary database. Also, attempting to disable the cop inline is not helping prevent the error. E.G. class SomeDimension < OLAPRecord
# rubocop:disable Rails/UniqueValidationWithoutIndex
validates :some_field, presence: true, uniqueness: { scope: :some_other_field, case_sensitive: false }
# rubocop:enable Rails/UniqueValidationWithoutIndex
end That still causes an error. |
This issue has been resolved by #349 and RuboCop Rails 2.8.1 has been released. Can you upgrade to the latest RuboCop Rails? |
@koic Do you have any thoughts on @Jay-Schneider critique of that PR in his message above?
I tend to agree with @Jay-Schneider. If the class that the validation is crashing on is actually a subclass of a model with a table in the database, shouldn't the real fix be to walk the inheritance up to the ActiveRecord Base class and check for a matching table? |
I think the |
Expected behavior
Rubocop-rails should not output any errors
Actual behavior
Running rubocop outputs
Steps to reproduce the problem
Sorry, I do not have a reproducible example yet - but I would think that namespaced models cause the problem, since it errors on a few files, which all have a namespace - e.g.:
RuboCop version
Thanks and keep up the great work 👍
The text was updated successfully, but these errors were encountered: