Better docs for `dependent: :destroy`

[arianf]

We follow a pattern for soft deleting with the is_retired flag, given the example here:

class Post < ApplicationRecord
  has_many :comments, -> { where(comments: { is_retired: false }) }, inverse_of: :post
  has_many :all_comments, dependent: :destroy, class_name: 'Comment', inverse_of: :post
end

The above would trigger this cop: https://rails.rubystyle.guide/#has_many-has_one-dependent-option on the first line for missing a dependent.

# bad
class Post < ApplicationRecord
  has_many :comments
end

# good
class Post < ApplicationRecord
  has_many :comments, dependent: :destroy
end

Looking at the rubocop docs and the rails docs, it's not clear that you can pass in nil to fix the cop. Even in the rails list of arguments you can pass in, it's not clear that nil would work, see self.valid_dependent_options

However if the value is nil, the check doesn't happen (kind of obvious since it's similar to having it unset), see self.define_callbacks(model, reflection)

So it might be good to add to the example of how to fix the cop, in cases where a user really doesn't want to have a dependent delete.

# good
class Post < ApplicationRecord
  has_many :comments, dependent: :destroy
end

# good, only if you really don't want to have a dependent destroy
class Post < ApplicationRecord
  has_many :comments, dependent: nil
end

[pirj]

dependent: nil is a completely valid option according to Rails docs:

nil do nothing (default).

The cop's docs are clear concerning this:

      #   # good
...
      #     has_many :articles, dependent: nil

Would you like to submit a note to the corresponding guideline? Something like:

Explicitly specify dependent: nil to express the intent to do nothing with associated records when a destroying a record

To me, it seems to be a reasonable approach that goes along with Rails docs and the cop.

If you find the cop's docs unclear, I encourage you to go ahead and make it more clear.