diff --git a/core/app/models/spree/stock/packer.rb b/core/app/models/spree/stock/packer.rb index 6820f1601ae..1a24b4fd9ca 100644 --- a/core/app/models/spree/stock/packer.rb +++ b/core/app/models/spree/stock/packer.rb @@ -22,9 +22,10 @@ def default_package inventory_units.group_by(&:variant).each do |variant, variant_inventory_units| units = variant_inventory_units.clone if variant.should_track_inventory? - next unless stock_location.stock_item(variant) + stock_item = stock_item_for(variant.id) + next unless stock_item - on_hand, backordered = stock_location.fill_status(variant, units.count) + on_hand, backordered = stock_item.fill_status(units.count) raise Spree::Order::InsufficientStock unless on_hand > 0 || backordered > 0 package.add_multiple units.slice!(0, on_hand), :on_hand if on_hand > 0 package.add_multiple units.slice!(0, backordered), :backordered if backordered > 0 @@ -37,6 +38,21 @@ def default_package end private + + def stock_item_for(variant_id) + stock_item_lookup[variant_id] + end + + # Returns a lookup table in the form of: + # { => , ...} + def stock_item_lookup + @stock_item_lookup ||= Spree::StockItem. + where(variant_id: inventory_units.map(&:variant_id).uniq). + where(stock_location_id: stock_location.id). + map { |stock_item| [stock_item.variant_id, stock_item] }. + to_h # there is only one stock item per variant in a stock location + end + def build_splitter splitter = nil splitters.reverse.each do |klass| diff --git a/core/app/models/spree/stock_item.rb b/core/app/models/spree/stock_item.rb index 6cb0b1f1e24..30656a6d764 100644 --- a/core/app/models/spree/stock_item.rb +++ b/core/app/models/spree/stock_item.rb @@ -70,6 +70,19 @@ def reduce_count_on_hand_to_zero self.set_count_on_hand(0) if count_on_hand > 0 end + def fill_status(quantity) + if count_on_hand >= quantity + on_hand = quantity + backordered = 0 + else + on_hand = count_on_hand + on_hand = 0 if on_hand < 0 + backordered = backorderable? ? (quantity - on_hand) : 0 + end + + [on_hand, backordered] + end + private def verify_count_on_hand? count_on_hand_changed? && !backorderable? && (count_on_hand < count_on_hand_was) && (count_on_hand < 0) diff --git a/core/app/models/spree/stock_location.rb b/core/app/models/spree/stock_location.rb index bc075ffd5d9..1e794263590 100644 --- a/core/app/models/spree/stock_location.rb +++ b/core/app/models/spree/stock_location.rb @@ -95,17 +95,7 @@ def move(variant, quantity, originator = nil) def fill_status(variant, quantity) if item = stock_item(variant) - - if item.count_on_hand >= quantity - on_hand = quantity - backordered = 0 - else - on_hand = item.count_on_hand - on_hand = 0 if on_hand < 0 - backordered = item.backorderable? ? (quantity - on_hand) : 0 - end - - [on_hand, backordered] + item.fill_status(quantity) else [0, 0] end diff --git a/core/db/migrate/20151219020209_add_stock_item_unique_index.rb b/core/db/migrate/20151219020209_add_stock_item_unique_index.rb new file mode 100644 index 00000000000..abdc8617267 --- /dev/null +++ b/core/db/migrate/20151219020209_add_stock_item_unique_index.rb @@ -0,0 +1,9 @@ +class AddStockItemUniqueIndex < ActiveRecord::Migration + def change + # Add a database-level uniqueness constraint for databases that support it + # (postgres & sqlite) + if connection.adapter_name =~ /postgres|sqlite/i + add_index 'spree_stock_items', ['variant_id', 'stock_location_id'], where: 'deleted_at is null', unique: true + end + end +end diff --git a/core/spec/models/spree/stock/packer_spec.rb b/core/spec/models/spree/stock/packer_spec.rb index 94d1fdefd75..e8c78ae18ab 100644 --- a/core/spec/models/spree/stock/packer_spec.rb +++ b/core/spec/models/spree/stock/packer_spec.rb @@ -28,9 +28,8 @@ module Stock end it 'variants are added as backordered without enough on_hand' do - expect(stock_location).to receive(:fill_status).exactly(5).times.and_return( - *(Array.new(3, [1,0]) + Array.new(2, [0,1])) - ) + inventory_units[0..2].each { |iu| stock_location.stock_item(iu.variant_id).set_count_on_hand(1) } + inventory_units[3..4].each { |iu| stock_location.stock_item(iu.variant_id).set_count_on_hand(0) } package = subject.default_package expect(package.on_hand.size).to eq 3 diff --git a/core/spec/models/spree/stock_location_spec.rb b/core/spec/models/spree/stock_location_spec.rb index 09cbaec6a3a..d5850e16ccd 100644 --- a/core/spec/models/spree/stock_location_spec.rb +++ b/core/spec/models/spree/stock_location_spec.rb @@ -170,10 +170,7 @@ module Spree end it 'zero on_hand with all backordered' do - zero_stock_item = mock_model(StockItem, - count_on_hand: 0, - backorderable?: true) - expect(subject).to receive(:stock_item).with(variant).and_return(zero_stock_item) + stock_item.set_count_on_hand(0) on_hand, backordered = subject.fill_status(variant, 20) expect(on_hand).to eq 0 @@ -182,12 +179,11 @@ module Spree context 'when backordering is not allowed' do before do - @stock_item = mock_model(StockItem, backorderable?: false) - expect(subject).to receive(:stock_item).with(variant).and_return(@stock_item) + stock_item.update!(backorderable: false) end it 'all on_hand' do - allow(@stock_item).to receive_messages(count_on_hand: 10) + stock_item.set_count_on_hand(10) on_hand, backordered = subject.fill_status(variant, 5) expect(on_hand).to eq 5 @@ -195,7 +191,7 @@ module Spree end it 'some on_hand' do - allow(@stock_item).to receive_messages(count_on_hand: 10) + stock_item.set_count_on_hand(10) on_hand, backordered = subject.fill_status(variant, 20) expect(on_hand).to eq 10 @@ -203,7 +199,7 @@ module Spree end it 'zero on_hand' do - allow(@stock_item).to receive_messages(count_on_hand: 0) + stock_item.set_count_on_hand(0) on_hand, backordered = subject.fill_status(variant, 20) expect(on_hand).to eq 0