From 7487b9a830f960ec846cd3e13de063cb818db2da Mon Sep 17 00:00:00 2001 From: Jared Norman Date: Thu, 15 Oct 2026 12:33:52 -0700 Subject: [PATCH] Handle different line item/inventory unit variants When a line item had inventory units, the validator counted pending units per stock location and checked line_item.variant against each count. This assumed every unit shared the line item's variant. This assumption doesn't hold when stores use solidus_product_assembly or implement their own bundling systems. It also doesn't hold even without such customizations: `ReturnItem#build_exchange_inventory_unit` attaches a unit with the exchange variant to the original line item. This means that an exchange on an order that gets canceled and resumed (admittedly not a common thing) would get validated against the original variant's stock, not the exchanged variant. Technically a bug. The `InventoryUnitsFinalizer` was fixed ages ago to unstock each unit's own variant. This change updates the validator to check stock the same way, against the inventory unit's variant. For ordinary stores, this doesn't change anything. For stores that implement bundling or use solidus_product_assembly, we can simplify things. Previously Product Assembly replaced this whole class; after this it'll only have to make one small modification. References: - Discussion #4489 - The finalizer was fixed in d3411e6b - https://jardo.dev/an-update-on-bundles-in-solidus --- .../spree/stock/availability_validator.rb | 10 ++++-- .../stock/availability_validator_spec.rb | 36 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/core/app/models/spree/stock/availability_validator.rb b/core/app/models/spree/stock/availability_validator.rb index dbf05cc1bf0..31b6f6ba219 100644 --- a/core/app/models/spree/stock/availability_validator.rb +++ b/core/app/models/spree/stock/availability_validator.rb @@ -25,9 +25,13 @@ def is_valid?(line_item) if line_item.inventory_units.empty? Spree::Config.stock.quantifier_class.new(line_item.variant).can_supply?(line_item.quantity) else - quantity_by_stock_location_id = line_item.inventory_units.pending.joins(:shipment).group(:stock_location_id).count - quantity_by_stock_location_id.all? do |stock_location_id, quantity| - Spree::Config.stock.quantifier_class.new(line_item.variant, stock_location_id).can_supply?(quantity) + quantity_by_variant_and_stock_location = line_item.inventory_units.pending.joins(:shipment) + .group(:variant_id, :stock_location_id).count + variant_ids = quantity_by_variant_and_stock_location.keys.map(&:first) + variants = Spree::Variant.with_discarded.where(id: variant_ids).index_by(&:id) + + quantity_by_variant_and_stock_location.all? do |(variant_id, stock_location_id), quantity| + Spree::Config.stock.quantifier_class.new(variants.fetch(variant_id), stock_location_id).can_supply?(quantity) end end end diff --git a/core/spec/models/spree/stock/availability_validator_spec.rb b/core/spec/models/spree/stock/availability_validator_spec.rb index f74262005e8..76ccc95d3aa 100644 --- a/core/spec/models/spree/stock/availability_validator_spec.rb +++ b/core/spec/models/spree/stock/availability_validator_spec.rb @@ -106,6 +106,42 @@ module Stock include_examples "passes validation" end end + + context "when an inventory unit has a different variant than the line item" do + let(:line_item) { order.line_items.first } + let(:other_variant) { create(:variant) } + + before do + create(:inventory_unit, variant: other_variant, line_item:, shipment: order.shipments.first) + line_item.variant.stock_items.update_all(count_on_hand: 10, backorderable: false) + end + + context "and the other variant has no stock" do + before do + other_variant.stock_items.update_all(count_on_hand: 0, backorderable: false) + end + + include_examples "fails validation" + end + + context "and the other variant has stock" do + before do + other_variant.stock_items.update_all(count_on_hand: 1, backorderable: false) + end + + include_examples "passes validation" + end + + context "and only the other variant's units are pending" do + before do + line_item.inventory_units.where(variant: line_item.variant).update_all(pending: false) + line_item.variant.stock_items.update_all(count_on_hand: 0, backorderable: false) + other_variant.stock_items.update_all(count_on_hand: 1, backorderable: false) + end + + include_examples "passes validation" + end + end end context "line_item is split across two shipments" do