search

gregnavis / active_record_doctor

Identify database issues before they hit production.
MIT License
1.76k stars 55 forks source link

missing_presence_validation: ignore counter cache columns #180

Open fatkodima opened 7 months ago

fatkodima commented 7 months ago

This check produces false positives for counter cache columns. These are expected to be set by ActiveRecord, not by users.

fatkodima commented 2 months ago

@gregnavis Can you please merge all the open PRs?

gregnavis commented 2 months ago

Thanks for the nudge, @fatkodima! I won't merge them all today, as I'd like to see some changes made, but I definitely need to pay attention to the outstanding PRs. I left some comments on a few other PRs.

Here, I'm not sure what to do with older Rails versions. I think we'll drop 4.2 soon (I'm sure you'd love that decision!), but that would require a new major release.

For now, I suggest the following: Rails versions that fail in CI due to outdated APIs should:

  1. Show a warning to the user with the following content: "missing_presence_validation: counter cache columns will receive complaints about missing presence validations in Rails versions older than X.Y; you can add them to ignore_attributes"
  2. Skip counter cache detection for those older Rails versions.

@fatkodima, does that make sense?

fatkodima commented 2 months ago

Sorry, I am lazy and will wait when you drop the old version.

Imo, you can easily drop activerecord < 7 and release v2 version of the gem. People on rails 4.2 don't care about best practices, so they are not the target audience of this gem.

gregnavis commented 2 months ago

@fatkodima, you are not lazy at all! I totally understand supporting 4.2 is a total pain in the ass.

I marked this and a few other PRs for 2.0.