Use ActiveRecord's .find_each instead of .each whenever possible

[elia]

Summary

Rubocop 1.56 was released introducing some changes around Rails/FindEach.

Most changes are in specs and not-critical code, the few changes in core models seem innocuous or even improvements for some edge cases (e.g. a store with thousands of stock locations, which is admittedly unlikely anyway).

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

• I have written a thorough PR description.
• I have kept my commits small and atomic.
• I have used clear, explanatory commit messages.

The following are not always needed:

• 📖 I have updated the README to account for my changes.
• 📑 I have documented new code with YARD.
• 🛣️ I have opened a PR to update the guides.
• ✅ I have added automated tests to cover my changes.
• 📸 I have attached screenshots to demo visual changes.

[codecov]

Codecov Report

Merging #5380 (d81437c) into main (152b022) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #5380   +/-   ##
=======================================
  Coverage   88.71%   88.71%           
=======================================
  Files         563      563           
  Lines       13896    13896           
=======================================
  Hits        12328    12328           
  Misses       1568     1568           

Files Changed	Coverage Δ
...e/app/models/spree/reimbursement_tax_calculator.rb	100.00% <100.00%> (ø)
core/app/models/spree/stock_location.rb	100.00% <100.00%> (ø)
core/app/models/spree/variant.rb	100.00% <100.00%> (ø)
core/lib/spree/core/controller_helpers/order.rb	100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[rainerdema]

👍

[AdnanTheExcellent]

@elia I am upgrading to solidus 4.2 and was looking at this PR in the patch notes. I see that all.find_each is being called a few times, which I believe is redundant. you shouldn't need the all anymore.

i.e. Spree::Variant.all.find_each should be Spree::Variant.find_each

see https://edgeguides.rubyonrails.org/active_record_querying.html#retrieving-multiple-objects-in-batches

[tvdeyen]

@elia we have the same Rubocop warnings in v4.1 as well. Do you think it is ok to backport this PR to v4.1 as well? There shouldn't be any breaking changes in here, right?

[tvdeyen]

#5484

[elia]

you shouldn't need the all anymore.

@AdnanTheExcellent fully agree, can you please send a PR to fix that? I'd be happy to help merge it 🙏