From 90fdf594156a6adf39fba8de344290edb2a60e90 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Fri, 12 Jul 2024 09:39:19 +1000
Subject: [PATCH 1/7] Test current stock logic on shipment level

During checkout, stock is adjusted when a shipment is finalised. The
chain is:

* Order state change to complete.
* Trigger Order#finalize! which updates shipments.
* Trigger Shipment#finalize! which adjusts stock on the variant.
* A variant holds stock in stock items or in a variant override.
---
 spec/models/spree/shipment_spec.rb | 31 ++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/spec/models/spree/shipment_spec.rb b/spec/models/spree/shipment_spec.rb
index a559d8b855a..ecaba702058 100644
--- a/spec/models/spree/shipment_spec.rb
+++ b/spec/models/spree/shipment_spec.rb
@@ -264,6 +264,37 @@
     end
   end
 
+  describe "#finalize!" do
+    subject(:shipment) { order.shipments.first }
+    let(:variant) { order.variants.first }
+    let(:order) { create(:order_ready_for_confirmation) }
+
+    it "reduces stock" do
+      variant.on_hand = 5
+
+      expect { shipment.finalize! }
+        .to change { variant.on_hand }.from(5).to(4)
+    end
+
+    it "reduces stock of a variant override" do
+      variant.on_hand = 5
+      variant_override = VariantOverride.create!(
+        variant:,
+        hub: order.distributor,
+        count_on_hand: 7,
+        on_demand: false,
+      )
+
+      expect {
+        shipment.finalize!
+        variant.reload
+        variant_override.reload
+      }
+        .to change { variant_override.count_on_hand }.from(7).to(6)
+        .and change { variant.on_hand }.by(0)
+    end
+  end
+
   context "when order is completed" do
     before do
       allow(order).to receive_messages completed?: true

From 675b7febdf16cfffb00a0579f34494c7eb2bd785 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Fri, 12 Jul 2024 15:17:35 +1000
Subject: [PATCH 2/7] Test stock logic on variant level

VariantOverrides are bolted onto variants to change their logic.
---
 spec/models/spree/variant_stock_spec.rb | 89 +++++++++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 spec/models/spree/variant_stock_spec.rb

diff --git a/spec/models/spree/variant_stock_spec.rb b/spec/models/spree/variant_stock_spec.rb
new file mode 100644
index 00000000000..137d130402f
--- /dev/null
+++ b/spec/models/spree/variant_stock_spec.rb
@@ -0,0 +1,89 @@
+# frozen_string_literal: false
+
+require 'spec_helper'
+
+RSpec.describe Spree::Variant do
+  # This method is defined in app/models/concerns/variant_stock.rb.
+  # There is a separate spec for that concern but here I want to test
+  # the interplay of Spree::Variant and VariantOverride.
+  #
+  # A variant can be scoped to a hub which means that all stock methods
+  # like this one get overridden. Future calls to `variant.move` are then
+  # handled by the ScopeVariantToHub module which may call the
+  # VariantOverride.
+  describe "#move" do
+    subject(:variant) { create(:variant, on_hand: 5) }
+
+    it "changes stock" do
+      expect { variant.move(-2) }.to change { variant.on_hand }.from(5).to(3)
+    end
+
+    it "ignores stock when on demand" do
+      variant.on_demand = true
+
+      expect { variant.move(-2) }.not_to change { variant.on_hand }
+    end
+
+    it "rejects negative stock" do
+      expect { variant.move(-7) }.to raise_error(
+        ActiveRecord::RecordInvalid,
+        "Validation failed: Count on hand must be greater than or equal to 0"
+      )
+    end
+
+    describe "with VariantOverride" do
+      subject(:hub_variant) {
+        Spree::Variant.find(variant.id).tap { |v| scoper.scope(v) }
+      }
+      let(:override) {
+        VariantOverride.create!(
+          variant:,
+          hub: create(:distributor_enterprise),
+          count_on_hand: 7,
+          on_demand: false,
+        )
+      }
+      let(:scoper) { OpenFoodNetwork::ScopeVariantToHub.new(override.hub) }
+
+      it "changes stock only on the variant override" do
+        expect {
+          hub_variant.move(-3)
+          override.reload
+        }
+          .to change { override.count_on_hand }.from(7).to(4)
+          .and change { hub_variant.on_hand }.from(7).to(4)
+          .and change { variant.on_hand }.by(0)
+      end
+
+      it "ignores stock when on demand" do
+        override.update!(on_demand: true, count_on_hand: nil)
+
+        expect {
+          hub_variant.move(-3)
+          override.reload
+        }
+          .not_to change {
+            [
+              override.count_on_hand,
+              hub_variant.on_hand,
+              variant.on_hand,
+            ]
+          }
+      end
+
+      it "doesn't prevent negative stock" do
+        # VariantOverride relies on other stock checks during checkout. :-(
+        expect {
+          hub_variant.move(-8)
+          override.reload
+        }
+          .to change { override.count_on_hand }.from(7).to(-1)
+          .and change { hub_variant.on_hand }.from(7).to(-1)
+          .and change { variant.on_hand }.by(0)
+
+        # The update didn't run validations and now it's invalid:
+        expect(override).not_to be_valid
+      end
+    end
+  end
+end

From e9f89362f4ebf43d1921981621c80f0cde3a5d9d Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Wed, 20 Mar 2024 16:39:35 +1100
Subject: [PATCH 3/7] Remove validation of positive stock when on demand

We weren't allowing negative stock to stop any bug from accidentally
drawing too much stock. But now we want to implement a backordering
logic that depends on negative stock levels to know how much is needed
to replenish stock levels.
---
 app/models/variant_override.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/models/variant_override.rb b/app/models/variant_override.rb
index cabc309a938..bc1afbbf4e6 100644
--- a/app/models/variant_override.rb
+++ b/app/models/variant_override.rb
@@ -15,7 +15,9 @@ class VariantOverride < ApplicationRecord
   # Need to ensure this can be set by the user.
   validates :default_stock, numericality: { greater_than_or_equal_to: 0 }, allow_nil: true
   validates :price, numericality: { greater_than_or_equal_to: 0 }, allow_nil: true
-  validates :count_on_hand, numericality: { greater_than_or_equal_to: 0 }, allow_nil: true
+  validates :count_on_hand, numericality: {
+    greater_than_or_equal_to: 0, unless: :on_demand?
+  }, allow_nil: true
 
   default_scope { where(permission_revoked_at: nil) }
 

From a1887bdc7684cfc65384715d4c2d44e6d5d97ea9 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Wed, 20 Mar 2024 16:53:39 +1100
Subject: [PATCH 4/7] Update stock levels of on-demand items

We weren't bothering with stock when items were on demand anyway. But we
want to track stock now so that we can backorder more when local stock
levels become negative.
---
 app/models/concerns/variant_stock.rb                    | 3 +--
 lib/open_food_network/scope_variant_to_hub.rb           | 4 ----
 spec/lib/open_food_network/scope_variant_to_hub_spec.rb | 2 ++
 spec/models/spree/line_item_spec.rb                     | 4 ++--
 spec/system/consumer/shopping/variant_overrides_spec.rb | 2 ++
 5 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/app/models/concerns/variant_stock.rb b/app/models/concerns/variant_stock.rb
index f39ab38dc9c..10ea8f136e9 100644
--- a/app/models/concerns/variant_stock.rb
+++ b/app/models/concerns/variant_stock.rb
@@ -112,8 +112,7 @@ def fill_status(quantity)
   #
   # This enables us to override this behaviour for variant overrides
   def move(quantity, originator = nil)
-    # Don't change variant stock if variant is on_demand or has been deleted
-    return if on_demand || deleted_at
+    return if deleted_at
 
     raise_error_if_no_stock_item_available
 
diff --git a/lib/open_food_network/scope_variant_to_hub.rb b/lib/open_food_network/scope_variant_to_hub.rb
index d7a29cbb9c2..90bfef39d40 100644
--- a/lib/open_food_network/scope_variant_to_hub.rb
+++ b/lib/open_food_network/scope_variant_to_hub.rb
@@ -43,11 +43,7 @@ def on_demand
       #   - updates variant_override.count_on_hand
       #   - does not create stock_movement
       #   - does not update stock_item.count_on_hand
-      # If it is a variant override with on_demand:
-      #   - don't change stock or call super (super would change the variant's stock)
       def move(quantity, originator = nil)
-        return if @variant_override&.on_demand
-
         if @variant_override&.stock_overridden?
           @variant_override.move_stock! quantity
         else
diff --git a/spec/lib/open_food_network/scope_variant_to_hub_spec.rb b/spec/lib/open_food_network/scope_variant_to_hub_spec.rb
index cbff22dc359..705f4ba159e 100644
--- a/spec/lib/open_food_network/scope_variant_to_hub_spec.rb
+++ b/spec/lib/open_food_network/scope_variant_to_hub_spec.rb
@@ -182,6 +182,8 @@ module OpenFoodNetwork
           end
 
           it "doesn't reduce variant's stock" do
+            pending "updating override stock"
+
             v2.move(-2)
             expect(Spree::Variant.find(v2.id).on_hand).to eq 5
           end
diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb
index d715fec503f..b6fecf1f33a 100644
--- a/spec/models/spree/line_item_spec.rb
+++ b/spec/models/spree/line_item_spec.rb
@@ -313,8 +313,8 @@ module Spree
           expect(order.shipment.manifest.first.variant).to eq line_item.variant
         end
 
-        it "does not reduce the variant's stock level" do
-          expect(variant_on_demand.reload.on_hand).to eq 1
+        it "reduces the variant's stock level" do
+          expect(variant_on_demand.reload.on_hand).to eq(-9)
         end
 
         it "does not mark inventory units as backorderd" do
diff --git a/spec/system/consumer/shopping/variant_overrides_spec.rb b/spec/system/consumer/shopping/variant_overrides_spec.rb
index 01dd56c3493..2bbc6796047 100644
--- a/spec/system/consumer/shopping/variant_overrides_spec.rb
+++ b/spec/system/consumer/shopping/variant_overrides_spec.rb
@@ -223,6 +223,8 @@
     end
 
     it "does not subtract stock from variants where the override has on_demand: true" do
+      pending "update override stock"
+
       click_add_to_cart product4_variant1, 2
       click_checkout
       expect do

From cd8dc41b15859cbd86fd95687e7a97df5d6b17dc Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Fri, 12 Jul 2024 12:00:48 +1000
Subject: [PATCH 5/7] Update stock specs and add pending cases

---
 spec/models/spree/variant_stock_spec.rb | 34 +++++++++++++++++--------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/spec/models/spree/variant_stock_spec.rb b/spec/models/spree/variant_stock_spec.rb
index 137d130402f..b36f9896459 100644
--- a/spec/models/spree/variant_stock_spec.rb
+++ b/spec/models/spree/variant_stock_spec.rb
@@ -18,10 +18,10 @@
       expect { variant.move(-2) }.to change { variant.on_hand }.from(5).to(3)
     end
 
-    it "ignores stock when on demand" do
+    it "reduces stock even when on demand" do
       variant.on_demand = true
 
-      expect { variant.move(-2) }.not_to change { variant.on_hand }
+      expect { variant.move(-2) }.to change { variant.on_hand }.from(5).to(3)
     end
 
     it "rejects negative stock" do
@@ -55,20 +55,18 @@
           .and change { variant.on_hand }.by(0)
       end
 
-      it "ignores stock when on demand" do
-        override.update!(on_demand: true, count_on_hand: nil)
+      it "reduces stock when on demand" do
+        pending "VariantOverride allowing stock with on_demand"
+
+        override.update!(on_demand: true, count_on_hand: 7)
 
         expect {
           hub_variant.move(-3)
           override.reload
         }
-          .not_to change {
-            [
-              override.count_on_hand,
-              hub_variant.on_hand,
-              variant.on_hand,
-            ]
-          }
+          .to change { override.count_on_hand }.from(7).to(4)
+          .and change { hub_variant.on_hand }.from(7).to(4)
+          .and change { variant.on_hand }.by(0)
       end
 
       it "doesn't prevent negative stock" do
@@ -84,6 +82,20 @@
         # The update didn't run validations and now it's invalid:
         expect(override).not_to be_valid
       end
+
+      it "doesn't fail on negative stock when on demand" do
+        pending "https://github.com/openfoodfoundation/openfoodnetwork/issues/12586"
+
+        override.update!(on_demand: true, count_on_hand: nil)
+
+        expect {
+          hub_variant.move(-8)
+          override.reload
+        }
+          .to change { override.count_on_hand }.from(7).to(-1)
+          .and change { hub_variant.on_hand }.from(7).to(-1)
+          .and change { variant.on_hand }.by(0)
+      end
     end
   end
 end

From b6c407971dc9a5972cc44591cf54a20ff6939a9e Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Wed, 31 Jul 2024 15:26:53 +1000
Subject: [PATCH 6/7] Allow on-demand VariantOverride to track stock

We allowed this for producer stock and need to do the same for inventory
stock. This will allow us to create backorders for missing, but promised
stock.
---
 .../stock_settings_override_validation.rb     | 13 ++-------
 config/locales/en.yml                         |  1 -
 spec/models/spree/variant_stock_spec.rb       |  2 --
 spec/models/variant_override_spec.rb          |  7 ++---
 spec/system/admin/product_import_spec.rb      | 27 -------------------
 5 files changed, 4 insertions(+), 46 deletions(-)

diff --git a/app/models/concerns/stock_settings_override_validation.rb b/app/models/concerns/stock_settings_override_validation.rb
index 7040cb50e06..b2036991d3b 100644
--- a/app/models/concerns/stock_settings_override_validation.rb
+++ b/app/models/concerns/stock_settings_override_validation.rb
@@ -6,14 +6,14 @@
 # `count_on_hand` can either be: nil or a number
 #
 # This means that a variant override can be in six different stock states
-# but only three of them are valid.
+# but only four of them are valid.
 #
 # | on_demand | count_on_hand | stock_overridden? | use_producer_stock_settings? | valid? |
 # |-----------|---------------|-------------------|------------------------------|--------|
 # | 1         | nil           | false             | false                        | true   |
 # | 0         | x             | true              | false                        | true   |
 # | nil       | nil           | false             | true                         | true   |
-# | 1         | x             | ?                 | ?                            | false  |
+# | 1         | x             | true              | false                        | true   |
 # | 0         | nil           | ?                 | ?                            | false  |
 # | nil       | x             | ?                 | ?                            | false  |
 #
@@ -27,7 +27,6 @@ module StockSettingsOverrideValidation
 
   def require_compatible_on_demand_and_count_on_hand
     disallow_count_on_hand_if_using_producer_stock_settings
-    disallow_count_on_hand_if_on_demand
     require_count_on_hand_if_limited_stock
   end
 
@@ -39,14 +38,6 @@ def disallow_count_on_hand_if_using_producer_stock_settings
     errors.add(:count_on_hand, error_message)
   end
 
-  def disallow_count_on_hand_if_on_demand
-    return unless on_demand? && count_on_hand.present?
-
-    error_message = I18n.t("count_on_hand.on_demand_but_count_on_hand_set",
-                           scope: i18n_scope_for_stock_settings_override_validation_error)
-    errors.add(:count_on_hand, error_message)
-  end
-
   def require_count_on_hand_if_limited_stock
     return unless on_demand == false && count_on_hand.blank?
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index f2f0ed92ab9..d5743302fa9 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -127,7 +127,6 @@ en:
         variant_override:
           count_on_hand:
             using_producer_stock_settings_but_count_on_hand_set: "must be blank because using producer stock settings"
-            on_demand_but_count_on_hand_set: "must be blank if on demand"
             limited_stock_but_no_count_on_hand: "must be specified because forcing limited stock"
       messages:
         confirmation: "doesn't match %{attribute}"
diff --git a/spec/models/spree/variant_stock_spec.rb b/spec/models/spree/variant_stock_spec.rb
index b36f9896459..4a999fac6c9 100644
--- a/spec/models/spree/variant_stock_spec.rb
+++ b/spec/models/spree/variant_stock_spec.rb
@@ -56,8 +56,6 @@
       end
 
       it "reduces stock when on demand" do
-        pending "VariantOverride allowing stock with on_demand"
-
         override.update!(on_demand: true, count_on_hand: 7)
 
         expect {
diff --git a/spec/models/variant_override_spec.rb b/spec/models/variant_override_spec.rb
index 15c9a1f4bf6..38022e206ea 100644
--- a/spec/models/variant_override_spec.rb
+++ b/spec/models/variant_override_spec.rb
@@ -97,11 +97,8 @@
         context "when count_on_hand is set" do
           let(:count_on_hand) { 1 }
 
-          it "is invalid" do
-            expect(variant_override).not_to be_valid
-            error_message = I18n.t("on_demand_but_count_on_hand_set",
-                                   scope: [i18n_scope_for_error, "count_on_hand"])
-            expect(variant_override.errors[:count_on_hand]).to eq([error_message])
+          it "is valid" do
+            expect(variant_override).to be_valid
           end
         end
       end
diff --git a/spec/system/admin/product_import_spec.rb b/spec/system/admin/product_import_spec.rb
index e916a56a74f..49c90699180 100644
--- a/spec/system/admin/product_import_spec.rb
+++ b/spec/system/admin/product_import_spec.rb
@@ -624,33 +624,6 @@
       expect(page).not_to have_content "line 3: Sprouts"
     end
 
-    it "handles on_demand and on_hand validations with inventory - With both values set" do
-      csv_data = <<~CSV
-        name, distributor, producer, category, on_hand, price, units, on_demand
-        Beans, Another Enterprise, User Enterprise, Vegetables, 6, 3.20, 500, 1
-        Sprouts, Another Enterprise, User Enterprise, Vegetables, 6, 6.50, 500, 1
-        Cabbage, Another Enterprise, User Enterprise, Vegetables, 0, 1.50, 500, 1
-      CSV
-      File.write('/tmp/test.csv', csv_data)
-
-      visit main_app.admin_product_import_path
-      select 'Inventories', from: "settings_import_into"
-      attach_file 'file', '/tmp/test.csv'
-      click_button 'Upload'
-
-      proceed_to_validation
-
-      expect(page).to have_selector '.item-count', text: "3"
-      expect(page).to have_selector '.invalid-count', text: "3"
-
-      find('div.header-description', text: 'Items contain errors').click
-      expect(page).to have_content "line 2: Beans - Count_on_hand must be blank if on demand"
-      expect(page).to have_content "line 3: Sprouts - Count_on_hand must be blank if on demand"
-      expect(page).to have_content "line 4: Cabbage - Count_on_hand must be blank if on demand"
-      expect(page).to have_content "Imported file contains invalid entries"
-      expect(page).not_to have_selector 'input[type=submit][value="Save"]'
-    end
-
     it "imports lines with all allowed units" do
       csv_data = CSV.generate do |csv|
         csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type",

From 2201d2e8c24322b1b21c3c1481c033984fb73817 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Wed, 31 Jul 2024 16:48:50 +1000
Subject: [PATCH 7/7] VariantOverride with on_demand now overriding stock

Otherwise we would try to take stock from the producer stock level
without respecting their on-demand settings. So from now on:
If stock level or on_demand are set on the override then it's not using
producer stock levels.
---
 .../concerns/stock_settings_override_validation.rb  |  2 +-
 app/models/concerns/variant_stock.rb                |  2 +-
 app/models/variant_override.rb                      |  5 ++---
 .../open_food_network/scope_variant_to_hub_spec.rb  |  5 ++---
 spec/models/spree/variant_stock_spec.rb             |  6 ++----
 spec/models/variant_override_spec.rb                | 13 ++++++++-----
 .../consumer/shopping/variant_overrides_spec.rb     |  6 ++----
 7 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/app/models/concerns/stock_settings_override_validation.rb b/app/models/concerns/stock_settings_override_validation.rb
index b2036991d3b..ecc3f2ec3bd 100644
--- a/app/models/concerns/stock_settings_override_validation.rb
+++ b/app/models/concerns/stock_settings_override_validation.rb
@@ -10,7 +10,7 @@
 #
 # | on_demand | count_on_hand | stock_overridden? | use_producer_stock_settings? | valid? |
 # |-----------|---------------|-------------------|------------------------------|--------|
-# | 1         | nil           | false             | false                        | true   |
+# | 1         | nil           | true              | false                        | true   |
 # | 0         | x             | true              | false                        | true   |
 # | nil       | nil           | false             | true                         | true   |
 # | 1         | x             | true              | false                        | true   |
diff --git a/app/models/concerns/variant_stock.rb b/app/models/concerns/variant_stock.rb
index 10ea8f136e9..2e5023d46cc 100644
--- a/app/models/concerns/variant_stock.rb
+++ b/app/models/concerns/variant_stock.rb
@@ -96,7 +96,7 @@ def can_supply?(quantity)
   # Here we depend only on variant.total_on_hand and variant.on_demand.
   #   This way, variant_overrides only need to override variant.total_on_hand and variant.on_demand.
   def fill_status(quantity)
-    on_hand = if total_on_hand >= quantity || on_demand
+    on_hand = if total_on_hand.to_i >= quantity || on_demand
                 quantity
               else
                 [0, total_on_hand].max
diff --git a/app/models/variant_override.rb b/app/models/variant_override.rb
index bc1afbbf4e6..79a28508f2b 100644
--- a/app/models/variant_override.rb
+++ b/app/models/variant_override.rb
@@ -38,9 +38,8 @@ def self.indexed(hub)
   end
 
   def stock_overridden?
-    # If count_on_hand is present, it means on_demand is false
-    #   See StockSettingsOverrideValidation for details
-    count_on_hand.present?
+    # Testing for not nil because for a boolean `false.present?` is false.
+    !on_demand.nil? || !count_on_hand.nil?
   end
 
   def use_producer_stock_settings?
diff --git a/spec/lib/open_food_network/scope_variant_to_hub_spec.rb b/spec/lib/open_food_network/scope_variant_to_hub_spec.rb
index 705f4ba159e..faa5d0583b6 100644
--- a/spec/lib/open_food_network/scope_variant_to_hub_spec.rb
+++ b/spec/lib/open_food_network/scope_variant_to_hub_spec.rb
@@ -181,11 +181,10 @@ module OpenFoodNetwork
             scoper.scope v2
           end
 
-          it "doesn't reduce variant's stock" do
-            pending "updating override stock"
-
+          it "reduces override stock, not variant's stock" do
             v2.move(-2)
             expect(Spree::Variant.find(v2.id).on_hand).to eq 5
+            expect(v2.on_hand).to eq(-2)
           end
         end
 
diff --git a/spec/models/spree/variant_stock_spec.rb b/spec/models/spree/variant_stock_spec.rb
index 4a999fac6c9..224fa3e855a 100644
--- a/spec/models/spree/variant_stock_spec.rb
+++ b/spec/models/spree/variant_stock_spec.rb
@@ -82,16 +82,14 @@
       end
 
       it "doesn't fail on negative stock when on demand" do
-        pending "https://github.com/openfoodfoundation/openfoodnetwork/issues/12586"
-
         override.update!(on_demand: true, count_on_hand: nil)
 
         expect {
           hub_variant.move(-8)
           override.reload
         }
-          .to change { override.count_on_hand }.from(7).to(-1)
-          .and change { hub_variant.on_hand }.from(7).to(-1)
+          .to change { override.count_on_hand }.from(nil).to(-8)
+          .and change { hub_variant.on_hand }.from(nil).to(-8)
           .and change { variant.on_hand }.by(0)
       end
     end
diff --git a/spec/models/variant_override_spec.rb b/spec/models/variant_override_spec.rb
index 38022e206ea..41122e6e71e 100644
--- a/spec/models/variant_override_spec.rb
+++ b/spec/models/variant_override_spec.rb
@@ -165,7 +165,7 @@
 
   describe "with nil count on hand" do
     let(:variant_override) do
-      build_stubbed(
+      build(
         :variant_override,
         variant: build_stubbed(:variant),
         hub: build_stubbed(:distributor_enterprise),
@@ -175,15 +175,18 @@
     end
 
     describe "stock_overridden?" do
-      it "returns false" do
-        expect(variant_override.stock_overridden?).to be false
+      it "returns true" do
+        expect(variant_override.stock_overridden?).to be true
       end
     end
 
     describe "move_stock!" do
       it "silently logs an error" do
-        expect(Bugsnag).to receive(:notify)
-        variant_override.move_stock!(5)
+        expect {
+          variant_override.move_stock!(5)
+        }.to change {
+          variant_override.count_on_hand
+        }.from(nil).to(5)
       end
     end
   end
diff --git a/spec/system/consumer/shopping/variant_overrides_spec.rb b/spec/system/consumer/shopping/variant_overrides_spec.rb
index 2bbc6796047..4cc8d11eb02 100644
--- a/spec/system/consumer/shopping/variant_overrides_spec.rb
+++ b/spec/system/consumer/shopping/variant_overrides_spec.rb
@@ -222,15 +222,13 @@
       expect(product1_variant1_override.reload.count_on_hand).to be_nil
     end
 
-    it "does not subtract stock from variants where the override has on_demand: true" do
-      pending "update override stock"
-
+    it "subtracts stock from override but not variants where the override has on_demand: true" do
       click_add_to_cart product4_variant1, 2
       click_checkout
       expect do
         complete_checkout
       end.to change { product4_variant1.reload.on_hand }.by(0)
-      expect(product4_variant1_override.reload.count_on_hand).to be_nil
+      expect(product4_variant1_override.reload.count_on_hand).to eq(-2)
     end
 
     it "does not show out of stock flags on order confirmation page" do