Fix Rubocop Rails issues

[dacook]

We use Rubocop to standardise our code, which can improve code quality and reduce confusion. When evaluating each rule, consider if it helps with those goals or not. Often the suggested autocorrection is fine, but some cops require more consideration and there may be a better solution, or it might be appropriate to disable them, rather than create unnecessary extra work.
If unsure, please ping @openfoodfoundation/core-devs to request an opinion.

Strategy to fix issues

The normal process to fix a style issue is:

1. Choose a cop to work on, next on the list (but ⚠️ double check there isn't already a PR addressing the cop: https://github.com/openfoodfoundation/openfoodnetwork/pulls)
2. Remove it from .rubocop_todo.yml and run rubocop to see the error(s).
3. Evaluate the rule, and consider applying the suggested fix if it makes sense
   • It may be helpful to commit some fixes separately, or chunked together
   • If it's not a simple fix, it may be worth simply adding a rubocop:disable comment in some cases (disable only offending line where possible)

When a cop or group of cops is complete, open a pull request.

Issues

Most/all of these will require manual fixes.
Generated 27 Feb 2024, up to date as of 19 Mar 2025

• Rubocop Rails/FindEach #12290
• Rubocop Rails/HasManyOrHasOneDependent #12291
• Rubocop Rails/HelperInstanceVariable #12314
• Rubocop Rails/I18nLocaleAssignment and Rails/I18nLocaleTexts #12332
• Rails/InverseOf
• Adresses Rails/LexicallyScoped offenses #13251
• Rubocop Rails/NegateInclude #12338
• Rubocop Rails/Pluck and Rails/PluckInWhere #12346
• Rails/RedundantActiveRecordAllMethod Fixes Rails/RedundantActiveRecordAllMethod issues #12589
• Rails/RedundantPresenceValidationOnBelongsTo (Fix cop RedundantPresenceValidationOnBelongs #12380)
• Rails/RelativeDateConstant Fixes Rails/RelativeDateConstant offense #12601
• Rubocop Rails/ResponseParsedBody #12620
• Rails/RootPathnameMethods (Fixes Rails/RootPathnameMethods offense #12667)
• Rails/SelectMap Fixes Rails/SelectMap offense #12641
• Rails/SkipsModelValidations Fixes Rails/SkipsModelValidations offenses #12647
• Rails/SquishedSQLHeredocs
• Rails/TimeZone
• Rails/TransactionExitStatement (redundant in Rails 7.2)
• Rails/UniqueValidationWithoutIndex Fixes Rails/UniqueValidationWithoutIndex (part of #11482) #13163
• Rails/UnknownEnv Fixes Rails/UnknownEnv offense #12614
• Rails/UnusedRenderContent Fixes offenses raised by Rails/UnusedRenderContent #12582
• Rails/WhereEquals (Fix rubocop FixRailsWhereEquals #12352)
• Rails/WhereExists (Fix Rails/WhereExists rubocop offenses #12363)

[dacook]

Hi @macanudo527 , I see you've started on this so created an issue for tracking.

[macanudo527]

@dacook Can you assign this to me?

[dacook]

Hi @macanudo527 , how have you been? I hope you've had a fulfilling start to the year!
Just wondering if you'd like to pick this up this year, or if we should unassign for now?

[macanudo527]

@dacook Yeah, I've just been too busy with everything. I'd like to work on it, but just can't.

[dacook]

No problems, that's totally understandable. All the best for this season, and I hope to see you again soon!

[anthonyms]

@dacook Kindly assign this to me.
Am I right in concluding that it is a subset of #6055?

[dacook]

Great, yep, that's correct Anthony. Good luck!

[sigmundpetersen]

@dacook maybe we could create sub-issues of the task list in the description?
Then each issue will check itself when a PR is merged towards it.

We can create them as we go. Just click to the far right of each checkbox.

[dacook]

Thanks Sigmund. Yes I hadn't bothered with this because I wasn't sure how big each of the items was. It's possible some are so small that they don't need to have a separate issue/pr..

[cyrillefr]

Hello @dacook ,

The Rails/SquishedSQLHeredocs item is not in the .rubocop_todo.yml file anymore.
It has been fixed in 2023. It can be removed from the list.

Regarding the Rails/TransactionExitStatement, the offense will no longer be effective in Rails 7.2 upwards since the behaviour that has changed will be restored. (I don't know how to tag this item).

[dacook]

Thanks Cyrille, that's interesting, the list was last generated in 2024, I don't know why SquishedSQL was included then. 🤷
I've ticked SquishedSQL and crossed out TransactionExitStatement with a note about 7.2.

Thanks to your latest contribution, we're almost finished!

[dacook]

Hi @cyrillefr , just wondering if you are planning to do any further work on this? If so I will assign the issue to you to avoid double-up with anyone else.

Edit: as you already have a PR open for this issue I've assigned you for now.

[cyrillefr]

Hello @dacook ,

The Rails/LexicallyScopedActionFilter box can be checked with a reference to #13251 instead :)

That leaves this issue with the #13163 which is blocked and it will be done.