From 319cb6ec7889bc651ef8a67a254702192b06d68d Mon Sep 17 00:00:00 2001
From: andrea longhi <andrea@spaghetticode.it>
Date: Wed, 21 Feb 2024 14:30:11 +0100
Subject: [PATCH 01/10] Use let! to remove explicit calls from before block

---
 core/spec/models/spree/fulfilment_changer_spec.rb | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/core/spec/models/spree/fulfilment_changer_spec.rb b/core/spec/models/spree/fulfilment_changer_spec.rb
index b850d2d4af7..f83f36a2d13 100644
--- a/core/spec/models/spree/fulfilment_changer_spec.rb
+++ b/core/spec/models/spree/fulfilment_changer_spec.rb
@@ -6,7 +6,7 @@
   let(:variant) { create(:variant) }
   let(:track_inventory) { true }
 
-  let(:order) do
+  let!(:order) do
     create(
       :completed_order_with_totals,
       line_items_attributes: [
@@ -19,7 +19,7 @@
   end
 
   let(:current_shipment) { order.shipments.first }
-  let(:desired_shipment) { order.shipments.create!(stock_location: desired_stock_location) }
+  let!(:desired_shipment) { order.shipments.create!(stock_location: desired_stock_location) }
   let(:desired_stock_location) { current_shipment.stock_location }
   let(:current_shipment_inventory_unit_count) { 1 }
   let(:quantity) { current_shipment_inventory_unit_count }
@@ -37,7 +37,6 @@
   subject { shipment_splitter.run! }
 
   before do
-    order && desired_shipment
     variant.stock_items.first.update_column(:count_on_hand, 100)
   end
 

From 9062828dd152bec2cd9ee915823e668b642bf860 Mon Sep 17 00:00:00 2001
From: andrea longhi <andrea@spaghetticode.it>
Date: Wed, 21 Feb 2024 16:50:54 +0100
Subject: [PATCH 02/10] Move let statements inside context block

These are already overridden in all other first-level context blocks
so it makes sense to move them in the single block that relies on
these values.
---
 core/spec/models/spree/fulfilment_changer_spec.rb | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/core/spec/models/spree/fulfilment_changer_spec.rb b/core/spec/models/spree/fulfilment_changer_spec.rb
index f83f36a2d13..b54d8522d5f 100644
--- a/core/spec/models/spree/fulfilment_changer_spec.rb
+++ b/core/spec/models/spree/fulfilment_changer_spec.rb
@@ -21,8 +21,6 @@
   let(:current_shipment) { order.shipments.first }
   let!(:desired_shipment) { order.shipments.create!(stock_location: desired_stock_location) }
   let(:desired_stock_location) { current_shipment.stock_location }
-  let(:current_shipment_inventory_unit_count) { 1 }
-  let(:quantity) { current_shipment_inventory_unit_count }
 
   let(:shipment_splitter) do
     described_class.new(
@@ -41,6 +39,9 @@
   end
 
   context "when the current shipment stock location is the same of the target shipment" do
+    let(:current_shipment_inventory_unit_count) { 1 }
+    let(:quantity) { current_shipment_inventory_unit_count }
+
     context "when the stock location is empty" do
       before do
         variant.stock_items.first.update_column(:count_on_hand, 0)

From 57c62ab2b26f41740a3715392b08445de8f53d14 Mon Sep 17 00:00:00 2001
From: andrea longhi <andrea@spaghetticode.it>
Date: Fri, 23 Feb 2024 15:15:24 +0100
Subject: [PATCH 03/10] Extract shared examples

Also, a couple of useless `let!` are removed (these records already
exist).
---
 .../models/spree/fulfilment_changer_spec.rb   | 166 ++++++------------
 1 file changed, 55 insertions(+), 111 deletions(-)

diff --git a/core/spec/models/spree/fulfilment_changer_spec.rb b/core/spec/models/spree/fulfilment_changer_spec.rb
index b54d8522d5f..7c2c5f96a12 100644
--- a/core/spec/models/spree/fulfilment_changer_spec.rb
+++ b/core/spec/models/spree/fulfilment_changer_spec.rb
@@ -32,50 +32,7 @@
     )
   end
 
-  subject { shipment_splitter.run! }
-
-  before do
-    variant.stock_items.first.update_column(:count_on_hand, 100)
-  end
-
-  context "when the current shipment stock location is the same of the target shipment" do
-    let(:current_shipment_inventory_unit_count) { 1 }
-    let(:quantity) { current_shipment_inventory_unit_count }
-
-    context "when the stock location is empty" do
-      before do
-        variant.stock_items.first.update_column(:count_on_hand, 0)
-      end
-
-      context "when the inventory unit is backordered" do
-        before do
-          current_shipment.inventory_units.first.update state: :backordered
-        end
-
-        it "creates a new backordered inventory unit" do
-          subject
-          expect(desired_shipment.inventory_units.first).to be_backordered
-        end
-      end
-
-      context "when the inventory unit is on hand" do
-        before do
-          current_shipment.inventory_units.first.update state: :on_hand
-        end
-
-        it "creates a new on hand inventory unit" do
-          subject
-          expect(desired_shipment.inventory_units.first).to be_on_hand
-        end
-      end
-    end
-  end
-
-  context "when tracking inventory is not set (same as false)" do
-    let(:current_shipment_inventory_unit_count) { 2 }
-    let(:quantity) { 1 }
-    let(:track_inventory) { nil }
-
+  shared_examples_for "moves inventory units between shipments" do
     it "adds the desired inventory units to the desired shipment" do
       expect { subject }.to change { desired_shipment.inventory_units.length }.by(quantity)
     end
@@ -83,12 +40,19 @@
     it "removes the desired inventory units from the current shipment" do
       expect { subject }.to change { current_shipment.inventory_units.length }.by(-quantity)
     end
+  end
 
+  shared_examples_for "recalculates shipping costs and order totals" do
     it "recalculates shipping costs for the current shipment" do
       expect(current_shipment).to receive(:refresh_rates)
       subject
     end
 
+    it "recalculates shipping costs for the new shipment" do
+      expect(desired_shipment).to receive(:refresh_rates)
+      subject
+    end
+
     it 'updates order totals' do
       original_total = order.total
       original_shipment_total = order.shipment_total
@@ -97,15 +61,11 @@
         to change { order.total }.from(original_total).to(original_total + original_shipment_total).
         and change { order.shipment_total }.by(original_shipment_total)
     end
+  end
 
+  shared_examples_for "completes transfer to another stock location without tracking inventory changes" do
     context "when transferring to another stock location" do
       let(:desired_stock_location) { create(:stock_location) }
-      let!(:stock_item) do
-        variant.stock_items.find_or_create_by!(
-          stock_location: desired_stock_location,
-          variant: variant,
-        )
-      end
 
       it "is marked as a successful transfer" do
         expect(subject).to be true
@@ -121,86 +81,70 @@
     end
   end
 
-  context "when not tracking inventory" do
-    let(:current_shipment_inventory_unit_count) { 2 }
-    let(:quantity) { 1 }
-    let(:track_inventory) { false }
-
-    it "adds the desired inventory units to the desired shipment" do
-      expect { subject }.to change { desired_shipment.inventory_units.length }.by(quantity)
-    end
-
-    it "removes the desired inventory units from the current shipment" do
-      expect { subject }.to change { current_shipment.inventory_units.length }.by(-quantity)
-    end
-
-    it "recalculates shipping costs for the current shipment" do
-      expect(current_shipment).to receive(:refresh_rates)
-      subject
-    end
+  subject { shipment_splitter.run! }
 
-    it 'updates order totals' do
-      original_total = order.total
-      original_shipment_total = order.shipment_total
+  before do
+    variant.stock_items.first.update_column(:count_on_hand, 100)
+  end
 
-      expect { subject }.
-        to change { order.total }.from(original_total).to(original_total + original_shipment_total).
-        and change { order.shipment_total }.by(original_shipment_total)
-    end
+  context "when the current shipment stock location is the same of the target shipment" do
+    let(:current_shipment_inventory_unit_count) { 1 }
+    let(:quantity) { current_shipment_inventory_unit_count }
 
-    context "when transferring to another stock location" do
-      let(:desired_stock_location) { create(:stock_location) }
-      let!(:stock_item) do
-        variant.stock_items.find_or_create_by!(
-          stock_location: desired_stock_location,
-          variant: variant,
-        )
+    context "when the stock location is empty" do
+      before do
+        variant.stock_items.first.update_column(:count_on_hand, 0)
       end
 
-      it "is marked as a successful transfer" do
-        expect(subject).to be true
-      end
+      context "when the inventory unit is backordered" do
+        before do
+          current_shipment.inventory_units.first.update state: :backordered
+        end
 
-      it "does not stock in the current stock location" do
-        expect { subject }.not_to change { current_shipment.stock_location.count_on_hand(variant) }
+        it "creates a new backordered inventory unit" do
+          subject
+          expect(desired_shipment.inventory_units.first).to be_backordered
+        end
       end
 
-      it "does not unstock the desired stock location" do
-        expect { subject }.not_to change { desired_shipment.stock_location.count_on_hand(variant) }
+      context "when the inventory unit is on hand" do
+        before do
+          current_shipment.inventory_units.first.update state: :on_hand
+        end
+
+        it "creates a new on hand inventory unit" do
+          subject
+          expect(desired_shipment.inventory_units.first).to be_on_hand
+        end
       end
     end
   end
 
-  context "when the current shipment has enough inventory units" do
+  context "when tracking inventory is not set (same as false)" do
     let(:current_shipment_inventory_unit_count) { 2 }
     let(:quantity) { 1 }
+    let(:track_inventory) { nil }
 
-    it "adds the desired inventory units to the desired shipment" do
-      expect { subject }.to change { desired_shipment.inventory_units.length }.by(quantity)
-    end
-
-    it "removes the desired inventory units from the current shipment" do
-      expect { subject }.to change { current_shipment.inventory_units.length }.by(-quantity)
-    end
+    it_behaves_like "moves inventory units between shipments"
+    it_behaves_like "recalculates shipping costs and order totals"
+    it_behaves_like "completes transfer to another stock location without tracking inventory changes"
+  end
 
-    it "recalculates shipping costs for the current shipment" do
-      expect(current_shipment).to receive(:refresh_rates)
-      subject
-    end
+  context "when not tracking inventory" do
+    let(:current_shipment_inventory_unit_count) { 2 }
+    let(:quantity) { 1 }
+    let(:track_inventory) { false }
 
-    it "recalculates shipping costs for the new shipment" do
-      expect(desired_shipment).to receive(:refresh_rates)
-      subject
-    end
+    it_behaves_like "moves inventory units between shipments"
+    it_behaves_like "completes transfer to another stock location without tracking inventory changes"
+  end
 
-    it 'updates order totals' do
-      original_total = order.total
-      original_shipment_total = order.shipment_total
+  context "when the current shipment has enough inventory units" do
+    let(:current_shipment_inventory_unit_count) { 2 }
+    let(:quantity) { 1 }
 
-      expect { subject }.
-        to change { order.total }.from(original_total).to(original_total + original_shipment_total).
-        and change { order.shipment_total }.by(original_shipment_total)
-    end
+    it_behaves_like "moves inventory units between shipments"
+    it_behaves_like "recalculates shipping costs and order totals"
 
     context "when transferring to another stock location" do
       let(:desired_stock_location) { create(:stock_location) }

From 2ff1bf4257a2db3e2eb84f94d78d0527b84e842c Mon Sep 17 00:00:00 2001
From: andrea longhi <andrea@spaghetticode.it>
Date: Fri, 23 Feb 2024 15:26:33 +0100
Subject: [PATCH 04/10] Reuse existing shared example

The removed spec can be replaced by the shared example, which
includes one further test so it slightly improves coverage.
---
 core/spec/models/spree/fulfilment_changer_spec.rb | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/core/spec/models/spree/fulfilment_changer_spec.rb b/core/spec/models/spree/fulfilment_changer_spec.rb
index 7c2c5f96a12..bb72fef2bfa 100644
--- a/core/spec/models/spree/fulfilment_changer_spec.rb
+++ b/core/spec/models/spree/fulfilment_changer_spec.rb
@@ -350,9 +350,7 @@
     let(:current_shipment_inventory_unit_count) { 30 }
     let(:quantity) { 30 }
 
-    it "adds the desired inventory units to the desired shipment" do
-      expect { subject }.to change { desired_shipment.inventory_units.length }.by(quantity)
-    end
+    it_behaves_like "moves inventory units between shipments"
 
     it "removes the current shipment" do
       expect { subject }.to change { Spree::Shipment.count }.by(-1)
@@ -365,9 +363,7 @@
 
     let(:desired_shipment) { order.shipments.build(stock_location: current_shipment.stock_location) }
 
-    it "adds the desired inventory units to the desired shipment" do
-      expect { subject }.to change { Spree::Shipment.count }.by(1)
-    end
+    it_behaves_like "moves inventory units between shipments"
 
     context "if the desired shipment is invalid" do
       let(:desired_shipment) { order.shipments.build(stock_location_id: 99_999_999) }

From 0d1e74725e5c716c0490e44011b4e3ebdcf999ed Mon Sep 17 00:00:00 2001
From: andrea longhi <andrea@spaghetticode.it>
Date: Fri, 23 Feb 2024 15:27:40 +0100
Subject: [PATCH 05/10] Reuse existing let value

This helps keeping things in sync just in the case that numbers
change.
---
 core/spec/models/spree/fulfilment_changer_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/spec/models/spree/fulfilment_changer_spec.rb b/core/spec/models/spree/fulfilment_changer_spec.rb
index bb72fef2bfa..3f94ae36c3b 100644
--- a/core/spec/models/spree/fulfilment_changer_spec.rb
+++ b/core/spec/models/spree/fulfilment_changer_spec.rb
@@ -348,7 +348,7 @@
 
   context "when the current shipment is emptied out by the transfer" do
     let(:current_shipment_inventory_unit_count) { 30 }
-    let(:quantity) { 30 }
+    let(:quantity) { current_shipment_inventory_unit_count }
 
     it_behaves_like "moves inventory units between shipments"
 

From 0079208dbc1f03599f8350c26c6857e48c647fe7 Mon Sep 17 00:00:00 2001
From: andrea longhi <andrea@spaghetticode.it>
Date: Fri, 23 Feb 2024 15:49:40 +0100
Subject: [PATCH 06/10] Extract shared example for later reuse

These specs don't really require a specific first level scenario,
so we're first extracting them to a shared example, and with the
following commit we're going to use it in other existing scenarios.
---
 .../models/spree/fulfilment_changer_spec.rb   | 48 ++++++++++---------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/core/spec/models/spree/fulfilment_changer_spec.rb b/core/spec/models/spree/fulfilment_changer_spec.rb
index 3f94ae36c3b..f75300b44d0 100644
--- a/core/spec/models/spree/fulfilment_changer_spec.rb
+++ b/core/spec/models/spree/fulfilment_changer_spec.rb
@@ -81,45 +81,47 @@
     end
   end
 
-  subject { shipment_splitter.run! }
-
-  before do
-    variant.stock_items.first.update_column(:count_on_hand, 100)
-  end
-
-  context "when the current shipment stock location is the same of the target shipment" do
-    let(:current_shipment_inventory_unit_count) { 1 }
-    let(:quantity) { current_shipment_inventory_unit_count }
-
+  shared_examples_for "properly manages inventory units" do
     context "when the stock location is empty" do
-      before do
-        variant.stock_items.first.update_column(:count_on_hand, 0)
-      end
+      let(:stock_item) { variant.stock_items.find_by!(stock_location: current_shipment.stock_location) }
+
+      before { stock_item.update_column(:count_on_hand, 0) }
 
-      context "when the inventory unit is backordered" do
+      context "when there are backordered inventory units" do
         before do
-          current_shipment.inventory_units.first.update state: :backordered
+          current_shipment.inventory_units.first.update(state: :backordered)
         end
 
-        it "creates a new backordered inventory unit" do
-          subject
-          expect(desired_shipment.inventory_units.first).to be_backordered
+        it "doesn't change the order inventory units state" do
+          expect { subject }.not_to change { order.inventory_units.map(&:state).sort }
         end
       end
 
-      context "when the inventory unit is on hand" do
+      context "when all inventory units are on hand" do
         before do
-          current_shipment.inventory_units.first.update state: :on_hand
+          current_shipment.inventory_units.update_all(state: :on_hand)
         end
 
-        it "creates a new on hand inventory unit" do
-          subject
-          expect(desired_shipment.inventory_units.first).to be_on_hand
+        it "doesn't change the order inventory units state" do
+          expect { subject }.not_to change { order.inventory_units.map(&:state).sort }
         end
       end
     end
   end
 
+  subject { shipment_splitter.run! }
+
+  before do
+    variant.stock_items.first.update_column(:count_on_hand, 100)
+  end
+
+  context "when the current shipment stock location is the same of the target shipment" do
+    let(:current_shipment_inventory_unit_count) { 1 }
+    let(:quantity) { current_shipment_inventory_unit_count }
+
+    it_behaves_like "properly manages inventory units"
+  end
+
   context "when tracking inventory is not set (same as false)" do
     let(:current_shipment_inventory_unit_count) { 2 }
     let(:quantity) { 1 }

From 67e609bb6a48ac15760388ba789d5d8be84b7fd2 Mon Sep 17 00:00:00 2001
From: andrea longhi <andrea@spaghetticode.it>
Date: Fri, 23 Feb 2024 16:37:00 +0100
Subject: [PATCH 07/10] Rework and reuse inventory units shared example

The existing scenario is reworked in order to expose an issue
when moving a few items of a variant (not all of them) to a
shipment with same stock location as the original one.
---
 .../models/spree/fulfilment_changer_spec.rb   | 58 +++++++++++++++----
 1 file changed, 46 insertions(+), 12 deletions(-)

diff --git a/core/spec/models/spree/fulfilment_changer_spec.rb b/core/spec/models/spree/fulfilment_changer_spec.rb
index f75300b44d0..ee6ec44c599 100644
--- a/core/spec/models/spree/fulfilment_changer_spec.rb
+++ b/core/spec/models/spree/fulfilment_changer_spec.rb
@@ -82,24 +82,48 @@
   end
 
   shared_examples_for "properly manages inventory units" do
-    context "when the stock location is empty" do
-      let(:stock_item) { variant.stock_items.find_by!(stock_location: current_shipment.stock_location) }
+    let(:stock_item) { variant.stock_items.find_by!(stock_location: current_shipment.stock_location) }
 
-      before { stock_item.update_column(:count_on_hand, 0) }
+    context "when there are backordered inventory units" do
+      let(:backordered_units_count) { 1 }
 
-      context "when there are backordered inventory units" do
+      before do
+        current_shipment.inventory_units.limit(backordered_units_count).update!(state: :backordered)
+      end
+
+      context "when the stock is zero or negative" do
         before do
-          current_shipment.inventory_units.first.update(state: :backordered)
+          stock_item.update_column(:count_on_hand, -backordered_units_count)
         end
 
-        it "doesn't change the order inventory units state" do
-          expect { subject }.not_to change { order.inventory_units.map(&:state).sort }
+        it "doesn't change inventory units state" do
+          expect { subject }
+          .not_to change { order.inventory_units.map(&:state).sort }
+          .from(%w[backordered on_hand])
         end
       end
 
-      context "when all inventory units are on hand" do
+      context "when backordered items can become on hand" do
+        before do
+          stock_item.update_column(:count_on_hand, backordered_units_count)
+        end
+
+        it "makes all inventory units on hand" do
+          expect { subject }
+            .to change { order.inventory_units.map(&:state).sort }
+            .from(%w[backordered on_hand]).to(%w[on_hand on_hand])
+        end
+      end
+    end
+
+    context "when all inventory units are on hand" do
+      before do
+        current_shipment.inventory_units.update_all(state: :on_hand)
+      end
+
+      context "when the stock is negative" do
         before do
-          current_shipment.inventory_units.update_all(state: :on_hand)
+          stock_item.update_column(:count_on_hand, -1)
         end
 
         it "doesn't change the order inventory units state" do
@@ -107,6 +131,14 @@
         end
       end
     end
+
+    context "when the stock location is empty" do
+      before { stock_item.update_column(:count_on_hand, 0) }
+
+      it "doesn't change the order inventory units state" do
+        expect { subject }.not_to change { order.inventory_units.map(&:state).sort }
+      end
+    end
   end
 
   subject { shipment_splitter.run! }
@@ -115,10 +147,11 @@
     variant.stock_items.first.update_column(:count_on_hand, 100)
   end
 
-  context "when the current shipment stock location is the same of the target shipment" do
-    let(:current_shipment_inventory_unit_count) { 1 }
-    let(:quantity) { current_shipment_inventory_unit_count }
+  context "when tracking inventory (default behavior)" do
+    let(:current_shipment_inventory_unit_count) { 2 }
+    let(:quantity) { 1 }
 
+    it_behaves_like "moves inventory units between shipments"
     it_behaves_like "properly manages inventory units"
   end
 
@@ -366,6 +399,7 @@
     let(:desired_shipment) { order.shipments.build(stock_location: current_shipment.stock_location) }
 
     it_behaves_like "moves inventory units between shipments"
+    it_behaves_like "properly manages inventory units"
 
     context "if the desired shipment is invalid" do
       let(:desired_shipment) { order.shipments.build(stock_location_id: 99_999_999) }

From 0524dcdbf9b2548ab5f065ed2aa5beadb01aa7a3 Mon Sep 17 00:00:00 2001
From: andrea longhi <andrea@spaghetticode.it>
Date: Thu, 7 Mar 2024 14:51:00 +0100
Subject: [PATCH 08/10] Refactor Quantifier initialization

This is propedeutic for the following commit.
---
 core/app/models/spree/stock/quantifier.rb     | 27 ++++++++++++-------
 .../models/spree/stock/quantifier_spec.rb     | 15 +++++------
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/core/app/models/spree/stock/quantifier.rb b/core/app/models/spree/stock/quantifier.rb
index cb5f31112a9..13fe5c4217d 100644
--- a/core/app/models/spree/stock/quantifier.rb
+++ b/core/app/models/spree/stock/quantifier.rb
@@ -11,14 +11,8 @@ class Quantifier
       #        If unspecified it will check inventory in all available StockLocations
       def initialize(variant, stock_location_or_id = nil)
         @variant = variant
-        @stock_items = variant.stock_items.select do |stock_item|
-          if stock_location_or_id
-            stock_item.stock_location == stock_location_or_id ||
-              stock_item.stock_location_id == stock_location_or_id
-          else
-            stock_item.stock_location.active?
-          end
-        end
+        @stock_location_or_id = stock_location_or_id
+        @stock_items = variant_stock_items
       end
 
       # Returns the total number of inventory units on hand for the variant.
@@ -26,7 +20,7 @@ def initialize(variant, stock_location_or_id = nil)
       # @return [Fixnum] number of inventory units on hand, or infinity if
       #   inventory is not tracked on the variant.
       def total_on_hand
-        if @variant.should_track_inventory?
+        if variant.should_track_inventory?
           stock_items.sum(&:count_on_hand)
         else
           Float::INFINITY
@@ -48,6 +42,21 @@ def backorderable?
       def can_supply?(required)
         total_on_hand >= required || backorderable?
       end
+
+      private
+
+      attr_reader :variant, :stock_location_or_id
+
+      def variant_stock_items
+        variant.stock_items.select do |stock_item|
+          if stock_location_or_id
+            stock_item.stock_location == stock_location_or_id ||
+              stock_item.stock_location_id == stock_location_or_id
+          else
+            stock_item.stock_location.active?
+          end
+        end
+      end
     end
   end
 end
diff --git a/core/spec/models/spree/stock/quantifier_spec.rb b/core/spec/models/spree/stock/quantifier_spec.rb
index 469f19295be..4bc01f0d270 100644
--- a/core/spec/models/spree/stock/quantifier_spec.rb
+++ b/core/spec/models/spree/stock/quantifier_spec.rb
@@ -2,17 +2,16 @@
 
 require 'rails_helper'
 
-RSpec.shared_examples_for 'unlimited supply' do
-  it 'can_supply? any amount' do
-    expect(subject.can_supply?(1)).to eq true
-    expect(subject.can_supply?(101)).to eq true
-    expect(subject.can_supply?(100_001)).to eq true
-  end
-end
-
 module Spree
   module Stock
     RSpec.describe Quantifier, type: :model do
+      shared_examples_for 'unlimited supply' do
+        it 'can_supply? any amount' do
+          expect(subject.can_supply?(1)).to eq true
+          expect(subject.can_supply?(101)).to eq true
+          expect(subject.can_supply?(100_001)).to eq true
+        end
+      end
       let(:target_stock_location) { nil }
       let!(:stock_location) { create :stock_location_with_items }
       let!(:stock_item) { stock_location.stock_items.order(:id).first }

From 2e17aeb0ca593eafbba734fb2d874d2446c00884 Mon Sep 17 00:00:00 2001
From: andrea longhi <andrea@spaghetticode.it>
Date: Thu, 7 Mar 2024 14:52:34 +0100
Subject: [PATCH 09/10] Add Quantifier#positive_stock method

When the quantifier is initialized with a stock location or its ID,
the new method returns the positive amount of stock on hand or zero,
if the stock for the variant is negative. Otherwise, it returns nil.
---
 core/app/models/spree/stock/quantifier.rb     | 15 ++++++
 .../models/spree/stock/quantifier_spec.rb     | 48 +++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/core/app/models/spree/stock/quantifier.rb b/core/app/models/spree/stock/quantifier.rb
index 13fe5c4217d..260c11a3e94 100644
--- a/core/app/models/spree/stock/quantifier.rb
+++ b/core/app/models/spree/stock/quantifier.rb
@@ -43,10 +43,25 @@ def can_supply?(required)
         total_on_hand >= required || backorderable?
       end
 
+      def positive_stock
+        return unless stock_location
+
+        on_hand = stock_location.count_on_hand(variant)
+        on_hand.positive? ? on_hand : 0
+      end
+
       private
 
       attr_reader :variant, :stock_location_or_id
 
+      def stock_location
+        @stock_location ||= if stock_location_or_id.is_a?(Spree::StockLocation)
+          stock_location_or_id
+        else
+          Spree::StockLocation.find_by(id: stock_location_or_id)
+        end
+      end
+
       def variant_stock_items
         variant.stock_items.select do |stock_item|
           if stock_location_or_id
diff --git a/core/spec/models/spree/stock/quantifier_spec.rb b/core/spec/models/spree/stock/quantifier_spec.rb
index 4bc01f0d270..51d219f6ce7 100644
--- a/core/spec/models/spree/stock/quantifier_spec.rb
+++ b/core/spec/models/spree/stock/quantifier_spec.rb
@@ -12,6 +12,25 @@ module Stock
           expect(subject.can_supply?(100_001)).to eq true
         end
       end
+
+      shared_examples_for "returns the positive stock on hand" do
+        context "when the stock location has no stock for the variant" do
+          it { is_expected.to be_zero }
+        end
+
+        context "when the stock location has negative stock for the variant" do
+          before { stock_item.set_count_on_hand(-1) }
+
+          it { is_expected.to be_zero }
+        end
+
+        context "when the stock location has positive stock for the variant" do
+          before { stock_item.set_count_on_hand(10) }
+
+          it { is_expected.to eq(10) }
+        end
+      end
+
       let(:target_stock_location) { nil }
       let!(:stock_location) { create :stock_location_with_items }
       let!(:stock_item) { stock_location.stock_items.order(:id).first }
@@ -107,6 +126,35 @@ module Stock
           expect(subject.can_supply?(6)).to eq false
         end
       end
+
+      describe "#positive_stock" do
+        let(:variant) { create(:variant) }
+        let(:stock_location) { create(:stock_location) }
+        let(:stock_item) { stock_location.set_up_stock_item(variant) }
+        let(:instance) { described_class.new(variant, stock_location_or_id) }
+
+        subject { instance.positive_stock }
+
+        context "when stock location is not present" do
+          let(:stock_location_or_id) { nil }
+
+          it { is_expected.to be_nil }
+        end
+
+        context "when stock_location_id is present" do
+          context "when stock_location_id is a stock location" do
+            let(:stock_location_or_id) { stock_location }
+
+            it_behaves_like "returns the positive stock on hand"
+          end
+
+          context "when stock_location_id is a stock location id" do
+            let(:stock_location_or_id) { stock_location.id }
+
+            it_behaves_like "returns the positive stock on hand"
+          end
+        end
+      end
     end
   end
 end

From a1bb98ef2ccb11a719c5db5761b2210f5a20bb98 Mon Sep 17 00:00:00 2001
From: andrea longhi <andrea@spaghetticode.it>
Date: Mon, 26 Feb 2024 16:19:43 +0100
Subject: [PATCH 10/10] Properly calculate inventory item quantities to be
 moved

The backordered quantity count should differ depending on whether
moving to the same or a different stock location. For this reason,
the way we calculate `available_quantity` changes as follows:

* when the stock location differs:
  the stock on hand at the new shipment stock location;
* when the stock location is the same:
  the sum of the stock on hand at the shipment stock location
  plus the number of on_hand inventory items from the shipment

The explicit `backordered_quantity` variable is introduced to track
the number of backordered items for the target shipment. The value
is calculated as follows:

* when the stock location differs:
  the quantity to be moved minus the positive available quantity at
  the stock location;
* when the stock location is the same:
  the shipment total quantity for the variant minus the positive
  available quantity at the stock location.

Also, we start the process by moving backordered items first to
to make sure no pending backordered item remains. If the backordered
count decreased, we're going to leave a few to be later moved and
transformed to on hand, while if the backordered count increased, we
are going to move also some previously on hand items.
---
 core/app/models/spree/fulfilment_changer.rb | 41 ++++++++++++++++-----
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/core/app/models/spree/fulfilment_changer.rb b/core/app/models/spree/fulfilment_changer.rb
index 61a4234716f..1178c08f8e6 100644
--- a/core/app/models/spree/fulfilment_changer.rb
+++ b/core/app/models/spree/fulfilment_changer.rb
@@ -86,9 +86,9 @@ def run!
     # we can take from the desired location, we could end up with some items being backordered.
     def run_tracking_inventory
       # Retrieve how many on hand items we can take from desired stock location
-      available_quantity = [desired_shipment.stock_location.count_on_hand(variant), default_on_hand_quantity].max
-
+      available_quantity = get_available_quantity
       new_on_hand_quantity = [available_quantity, quantity].min
+      backordered_quantity = get_backordered_quantity(available_quantity, new_on_hand_quantity)
       unstock_quantity = desired_shipment.stock_location.backorderable?(variant) ? quantity : new_on_hand_quantity
 
       ActiveRecord::Base.transaction do
@@ -105,19 +105,21 @@ def run_tracking_inventory
         # These two statements are the heart of this class. We change the number
         # of inventory units requested from one shipment to the other.
         # We order by state, because `'backordered' < 'on_hand'`.
+        # We start to move the new actual backordered quantity, so the remaining
+        # quantity can be set to on_hand state.
         current_shipment.
           inventory_units.
           where(variant: variant).
           order(state: :asc).
-          limit(new_on_hand_quantity).
-          update_all(shipment_id: desired_shipment.id, state: :on_hand)
+          limit(backordered_quantity).
+          update_all(shipment_id: desired_shipment.id, state: :backordered)
 
         current_shipment.
           inventory_units.
           where(variant: variant).
           order(state: :asc).
-          limit(quantity - new_on_hand_quantity).
-          update_all(shipment_id: desired_shipment.id, state: :backordered)
+          limit(quantity - backordered_quantity).
+          update_all(shipment_id: desired_shipment.id, state: :on_hand)
       end
     end
 
@@ -141,11 +143,22 @@ def handle_stock_counts?
       current_shipment.order.completed? && current_stock_location != desired_stock_location
     end
 
-    def default_on_hand_quantity
+    def get_available_quantity
+      if current_stock_location != desired_stock_location
+        desired_location_quantifier.positive_stock
+      else
+        sl_availability = current_location_quantifier.positive_stock
+        shipment_availability = current_shipment.inventory_units.where(variant: variant).on_hand.count
+        sl_availability + shipment_availability
+      end
+    end
+
+    def get_backordered_quantity(available_quantity, new_on_hand_quantity)
       if current_stock_location != desired_stock_location
-        0
+        quantity - new_on_hand_quantity
       else
-        current_shipment.inventory_units.where(variant: variant).on_hand.count
+        shipment_quantity = current_shipment.inventory_units.where(variant: variant).size
+        shipment_quantity - available_quantity
       end
     end
 
@@ -156,11 +169,19 @@ def current_shipment_not_already_shipped
     end
 
     def enough_stock_at_desired_location
-      unless Spree::Stock::Quantifier.new(variant, desired_stock_location).can_supply?(quantity)
+      unless desired_location_quantifier.can_supply?(quantity)
         errors.add(:desired_shipment, :not_enough_stock_at_desired_location)
       end
     end
 
+    def desired_location_quantifier
+      @desired_location_quantifier ||= Spree::Stock::Quantifier.new(variant, desired_stock_location)
+    end
+
+    def current_location_quantifier
+      @current_location_quantifier ||= Spree::Stock::Quantifier.new(variant, current_stock_location)
+    end
+
     def desired_shipment_different_from_current
       if desired_shipment.id == current_shipment.id
         errors.add(:desired_shipment, :can_not_transfer_within_same_shipment)