Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.1] Use ActiveRecord's .find_each instead of .each whenever possible #5484

Merged
merged 2 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Update all find_each "violations"
  • Loading branch information
elia authored and tvdeyen committed Nov 2, 2023
commit 9f9918129939a528614f496085267b5fe7f6137c
4 changes: 4 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -339,3 +339,7 @@ Lint/MissingSuper:
Exclude:
- 'core/lib/spree/deprecation.rb' # this is a known class that doesn't require super
- 'core/lib/spree/preferences/configuration.rb' # this class has no superclass defining `self.inherited`

Rails/FindEach:
Exclude:
- 'db/migrate/**/*'
2 changes: 1 addition & 1 deletion api/spec/requests/spree/api/checkouts_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ module Spree::Api

# Regression Spec for https://github.com/spree/spree/issues/5389 and https://github.com/spree/spree/issues/5880
it "can update addresses but not transition to delivery w/o shipping setup" do
Spree::ShippingMethod.all.each(&:destroy)
Spree::ShippingMethod.all.find_each(&:destroy)
put spree.api_checkout_path(order),
params: { order_token: order.guest_token, order: {
bill_address_attributes: address,
Expand Down
2 changes: 1 addition & 1 deletion backend/spec/features/admin/orders/order_details_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@

product.stock_items.where.not(stock_location_id: stock_location_ids).discard_all

product.stock_items.where(stock_location_id: stock_location_ids).each do |stock_item|
product.stock_items.where(stock_location_id: stock_location_ids).find_each do |stock_item|
stock_item.set_count_on_hand 1
end

Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/reimbursement_tax_calculator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module Spree
class ReimbursementTaxCalculator
class << self
def call(reimbursement)
reimbursement.return_items.includes(:inventory_unit).each do |return_item|
reimbursement.return_items.includes(:inventory_unit).find_each do |return_item|
set_tax!(return_item)
end
end
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/stock_location.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def create_stock_items

def ensure_one_default
if default
Spree::StockLocation.where(default: true).where.not(id: id).each do |stock_location|
Spree::StockLocation.where(default: true).where.not(id: id).find_each do |stock_location|
stock_location.default = false
stock_location.save!
end
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/variant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ def set_cost_currency
end

def create_stock_items
StockLocation.where(propagate_all_variants: true).each do |stock_location|
StockLocation.where(propagate_all_variants: true).find_each do |stock_location|
stock_location.propagate_variant(self)
end
end
Expand Down
2 changes: 1 addition & 1 deletion core/db/default/spree/states.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def create_states(subregions, country)
end

ActiveRecord::Base.transaction do
Spree::Country.all.each do |country|
Spree::Country.all.find_each do |country|
carmen_country = Carmen::Country.coded(country.iso)
next unless carmen_country.subregions?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def up
t.timestamps
end

StoreCreditUpdateReason.all.each do |update_reason|
StoreCreditUpdateReason.all.find_each do |update_reason|
StoreCreditReason.create!(name: update_reason.name)
end

Expand Down Expand Up @@ -45,7 +45,7 @@ def down
end
end

StoreCreditReason.all.each do |store_credit_reason|
StoreCreditReason.all.find_each do |store_credit_reason|
StoreCreditUpdateReason.create!(name: store_credit_reason.name)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

class SetPromotionsWithAnyPolicyToAllIfPossible < ActiveRecord::Migration[5.2]
def up
Spree::Promotion.where(match_policy: :any).includes(:promotion_rules).all.each do |promotion|
Spree::Promotion.where(match_policy: :any).includes(:promotion_rules).all.find_each do |promotion|
if promotion.promotion_rules.length <= 1
promotion.update!(match_policy: :all)
else
Expand Down
2 changes: 1 addition & 1 deletion core/lib/spree/core/controller_helpers/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def associate_user

def set_current_order
if spree_current_user && current_order
spree_current_user.orders.by_store(current_store).incomplete.where('id != ?', current_order.id).each do |order|
spree_current_user.orders.by_store(current_store).incomplete.where('id != ?', current_order.id).find_each do |order|
current_order.merge!(order, spree_current_user)
end
end
Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/exchange_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ module Spree

context "when it cannot create shipments for all items" do
before do
StockItem.where(variant_id: return_item.exchange_variant_id).each(&:destroy)
StockItem.where(variant_id: return_item.exchange_variant_id).find_each(&:destroy)
end

it 'raises an UnableToCreateShipments error' do
Expand Down
8 changes: 4 additions & 4 deletions core/spec/models/spree/stock/estimator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ module Stock
end

it "sorts shipping rates by cost" do
ShippingMethod.all.each(&:destroy)
ShippingMethod.all.find_each(&:destroy)
create(:shipping_method, cost: 5)
create(:shipping_method, cost: 3)
create(:shipping_method, cost: 4)
Expand All @@ -94,7 +94,7 @@ module Stock
end

context "general shipping methods" do
before { Spree::ShippingMethod.all.each(&:destroy) }
before { Spree::ShippingMethod.all.find_each(&:destroy) }

context 'with a custom shipping calculator with no preference' do
class Spree::Calculator::Shipping::NoPreferences < Spree::ShippingCalculator
Expand Down Expand Up @@ -168,7 +168,7 @@ def compute_package(_package)
end

context "involves backend only shipping methods" do
before{ Spree::ShippingMethod.all.each(&:destroy) }
before{ Spree::ShippingMethod.all.find_each(&:destroy) }
let!(:backend_method) { create(:shipping_method, available_to_users: false, cost: 0.00) }
let!(:generic_method) { create(:shipping_method, cost: 5.00) }

Expand All @@ -183,7 +183,7 @@ def compute_package(_package)
end

context "excludes shipping methods from other stores" do
before{ Spree::ShippingMethod.all.each(&:destroy) }
before{ Spree::ShippingMethod.all.find_each(&:destroy) }

let!(:other_method) do
create(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

# We need to remove the price for FR from the database so it is created in memory, and then run VatPriceGenerator twice to trigger the duplicate price issue.
it "will not build duplicate prices on multiple runs" do
variant.prices.where(country_iso: "FR").each(&:destroy)
variant.prices.where(country_iso: "FR").find_each(&:destroy)
variant.reload
described_class.new(variant).run
expect { subject }.not_to change { variant.prices.size }
Expand Down
2 changes: 1 addition & 1 deletion sample/db/samples/product_option_types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"Solidus T-Shirt", "Solidus Long Sleeve", "Solidus Women's T-Shirt"
]

Spree::Product.all.each do |product|
Spree::Product.all.find_each do |product|
product.option_types = [size]
product.option_types << color if colored_clothes.include?(product.name)
product.save!
Expand Down
2 changes: 1 addition & 1 deletion sample/db/samples/stock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
location.active = true
location.save!

Spree::Variant.all.each do |variant|
Spree::Variant.all.find_each do |variant|
variant.stock_items.each do |stock_item|
Spree::StockMovement.create(quantity: 10, stock_item: stock_item)
end
Expand Down