Rails/HasManyOrHasOneDependent flagging has_many:through associations

[simonc]

Rails/HasManyOrHasOneDependent seems to be wrongly flagging has_many through: associations.

I have the following model

class EmailConfig < ApplicationRecord
  has_many :filterings, dependent: :destroy
  has_many :filters, through: :filterings
end

---

Expected behavior

I expect that running rubocop on this file would not report any offense.

Actual behavior

I get the following report:

app/models/email_config.rb:3:3: C: Rails/HasManyOrHasOneDependent: Specify a :dependent option.
  has_many :filters, through: :filterings
  ^^^^^^^^

Steps to reproduce the problem

Run rubocop on a fail containing the code above.

RuboCop version

$ rubocop -V
0.50.0 (using Parser 2.4.0.0, running on ruby 2.4.2 x86_64-darwin16)

[simonc]

❤️

[bunnymatic]

Interestingly, I have the :destroy on the through connection and I'm seeing the same issue, because I don't have dependent specified on the through table. I want this setup because it does the right thing in the app. If i delete a Tag, the associated Tagging record is removed and the Post is not removed.

class Tag
  has_many :taggings
  has_many :posts, through: :taggings, dependent: :destroy
end

And i get the error because has_many :taggings doesn't have :dependent.
If i'm to follow the cop's rule, I'm not sure what the dependent setting should be for the has_many :taggings line.

Do you have any thoughts on the right way to set this up? or is this an issue with this HasManyDependent cop?

[simonc]

Comming back to this issue, we made a strange discovery rencently.

If we updated an EmailConfig with the following:

email_config.filter_ids #=> [1]

email_config.filter_ids = [2, 3]

The filter with id 1 would not receive a touch. This was an issue since our API was using cache and the payload for the Filter would not update.

Adding the dependent: :destroy option to the has_many :through solved this issue. So I guess that in the end, raising an issue when the option is not present is a good idea after all.

Maybe this should be made optional via config or at least documented 🤔