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.

Sign up for GitHub

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

Check if stock_items are loaded before doing a SUM() #5687

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Check if stock_items are loaded before doing a SUM() #5687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions core/app/models/spree/product.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,8 @@ def possible_promotions
def total_on_hand
if any_variants_not_track_inventory?
Float::INFINITY
elsif stock_items.loaded?
stock_items.sum(&:count_on_hand)
else
stock_items.sum(:count_on_hand)
tvdeyen marked this conversation as resolved.
Show resolved Hide resolved
end
Expand Down
19 changes: 17 additions & 2 deletions core/spec/models/spree/product_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,8 @@ class Extension < Spree::Base
end

context '#total_on_hand' do
let(:product) { create :product }

it 'should be infinite if track_inventory_levels is false' do
stub_spree_preferences(track_inventory_levels: false)
expect(build(:product, variants_including_master: [build(:master_variant)]).total_on_hand).to eql(Float::INFINITY)
Expand All @@ -547,17 +549,30 @@ class Extension < Spree::Base
end

it 'should return sum of stock items count_on_hand' do
product = create(:product)
product.stock_items.first.set_count_on_hand 5
product.variants_including_master.reload # force load association
expect(product.total_on_hand).to eql(5)
end

it 'should return sum of stock items count_on_hand when variants_including_master is not loaded' do
product = create(:product)
product.stock_items.first.set_count_on_hand 5
expect(product.reload.total_on_hand).to eql(5)
end

context 'when the stock items are loaded' do
it 'returns the loaded count_on_hand instead of doing SUM(count_on_hand)' do
product.stock_items.first.set_count_on_hand(5)

product = described_class.includes(:stock_items).find(product.id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this override works properly since it is failing on CircleCI. Also spec setup should be in a before block.


# Set the count on hand to a different number to highlight we loaded and
# must be done from the class level so that we don't update the instance
# product has loaded
Spree::StockItem.find(product.stock_items.first.id).set_count_on_hand(7)

expect(product.total_on_hand).to eql 5
end
end
end

# Regression spec for https://github.com/spree/spree/issues/5588
Expand Down