Merge pull request #5288 from solidusio/elia/stock-items-order

Fix random failures in `#determine_target_shipment`
solidusio · Aug 7, 2023 · 4786fc4 · 4786fc4
2 parents 1a81736 + 581d2eb
commit 4786fc4
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 10 deletions.
diff --git a/core/app/models/spree/order_inventory.rb b/core/app/models/spree/order_inventory.rb
@@ -61,10 +61,12 @@ def determine_target_shipment(quantity)
 
       potential_shipments.detect do |shipment|
         shipment.include?(variant)
-      end || potential_shipments.detect do |shipment|
-        stock_item = variant.stock_items.detect { |stock_item| stock_item.stock_location == shipment.stock_location }
-        if stock_item
-          stock_item.backorderable? || stock_item.count_on_hand >= quantity
+      end || begin
+        stock_items = variant.stock_items.sort_by(&:id) # cache stock items to avoid N+1
+
+        potential_shipments.detect do |shipment|
+          stock_item = stock_items.detect { |stock_item| stock_item.stock_location == shipment.stock_location }
+          stock_item.backorderable? || stock_item.count_on_hand >= quantity if stock_item
         end
       end || potential_shipments.detect do |shipment|
         variant.stock_location_ids.include?(shipment.stock_location_id)

diff --git a/core/spec/models/spree/order_inventory_spec.rb b/core/spec/models/spree/order_inventory_spec.rb
@@ -150,7 +150,7 @@
       end
 
       context 'when availability should be considered' do
-        let(:stock_item) { variant.stock_items.last }
+        let(:stock_item) { variant.stock_items.max_by(&:id) }
 
         before do
           variant.stock_items.update_all backorderable: false
@@ -171,14 +171,21 @@
         end
 
         context 'when there is not enough availability at any stock location' do
+          before { stock_item.set_count_on_hand 0 }
+
           it 'falls-back selecting first non-shipped shipment that leaves from same stock_location' do
-            shipment = subject.send(:determine_target_shipment, 1)
+            required_quantity = 1
+            shipment = subject.send(:determine_target_shipment, required_quantity)
             shipment.reload
 
-            expect(shipment.shipped?).to be false
-            expect(shipment.inventory_units_for(variant)).to be_empty
-            expect(variant.stock_location_ids.include?(shipment.stock_location_id)).to be true
-            expect(shipment.stock_location).not_to eql stock_item.stock_location
+            aggregate_failures do
+              expect(stock_item.count_on_hand).to eq(0)
+              expect(stock_item.backorderable?).to eq(false)
+              expect(shipment.shipped?).to be false
+              expect(shipment.inventory_units_for(variant)).to be_empty
+              expect(variant.stock_location_ids.include?(shipment.stock_location_id)).to be true
+              expect(shipment.stock_location).not_to eql stock_item.stock_location
+            end
           end
         end
       end