diff --git a/CHANGELOG.md b/CHANGELOG.md index ec75373c159..148552dd166 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ * The "Stores" section of Settings in the admin now supports creating and editing multiple stores. +* The `AvailabilityValidator` now correctly detects out of stock when there + are multiple shipments from the same stock location. + * Deprecations * `cache_key_for_taxons` helper has been deprecated in favour of `cache [I18n.locale, @taxons]` diff --git a/core/app/models/spree/inventory_unit.rb b/core/app/models/spree/inventory_unit.rb index 7dbd577da6e..c9440cd87ea 100644 --- a/core/app/models/spree/inventory_unit.rb +++ b/core/app/models/spree/inventory_unit.rb @@ -21,6 +21,7 @@ class InventoryUnit < Spree::Base before_destroy :ensure_can_destroy + scope :pending, -> { where pending: true } scope :backordered, -> { where state: 'backordered' } scope :on_hand, -> { where state: 'on_hand' } scope :pre_shipment, -> { where(state: PRE_SHIPMENT_STATES) } diff --git a/core/app/models/spree/stock/availability_validator.rb b/core/app/models/spree/stock/availability_validator.rb index 399ed823cf1..de9ddaed2d6 100644 --- a/core/app/models/spree/stock/availability_validator.rb +++ b/core/app/models/spree/stock/availability_validator.rb @@ -2,25 +2,9 @@ module Spree module Stock class AvailabilityValidator < ActiveModel::Validator def validate(line_item) - units_by_shipment = line_item.inventory_units.group_by(&:shipment) - - if units_by_shipment.blank? - ensure_in_stock(line_item, line_item.quantity) + if is_valid?(line_item) + true else - units_by_shipment.each do |shipment, inventory_units| - inventory_units.select!(&:pending?) - ensure_in_stock(line_item, inventory_units.size, shipment.stock_location) - end - end - - line_item.errors[:quantity].empty? - end - - private - - def ensure_in_stock(line_item, quantity, stock_location = nil) - quantifier = Stock::Quantifier.new(line_item.variant, stock_location) - unless quantifier.can_supply?(quantity) variant = line_item.variant display_name = variant.name.to_s display_name += %{ (#{variant.options_text})} unless variant.options_text.blank? @@ -29,6 +13,20 @@ def ensure_in_stock(line_item, quantity, stock_location = nil) :selected_quantity_not_available, item: display_name.inspect ) + false + end + end + + private + + def is_valid?(line_item) + if line_item.inventory_units.empty? + Stock::Quantifier.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| + Stock::Quantifier.new(line_item.variant, stock_location_id).can_supply?(quantity) + end end end end diff --git a/core/app/models/spree/stock/quantifier.rb b/core/app/models/spree/stock/quantifier.rb index 52c6d446dfb..8a5b5a1f854 100644 --- a/core/app/models/spree/stock/quantifier.rb +++ b/core/app/models/spree/stock/quantifier.rb @@ -3,15 +3,16 @@ module Stock class Quantifier attr_reader :stock_items + # @param [Variant] variant The variant to check inventory for. + # @param [StockLocation, Integer] stock_location The stock_location to check inventory in. If unspecified it will check inventory in all available StockLocations def initialize(variant, stock_location = nil) @variant = variant - where_args = { variant_id: @variant } + @stock_items = Spree::StockItem.where(variant_id: variant) if stock_location - where_args[:stock_location] = stock_location + @stock_items.where!(stock_location: stock_location) else - where_args[Spree::StockLocation.table_name] = { active: true } + @stock_items.joins!(:stock_location).merge!(Spree::StockLocation.active) end - @stock_items = Spree::StockItem.joins(:stock_location).where(where_args) end # Returns the total number of inventory units on hand for the variant. diff --git a/core/spec/models/spree/stock/availability_validator_spec.rb b/core/spec/models/spree/stock/availability_validator_spec.rb index d32fa39a717..14e93c90589 100644 --- a/core/spec/models/spree/stock/availability_validator_spec.rb +++ b/core/spec/models/spree/stock/availability_validator_spec.rb @@ -54,39 +54,85 @@ module Stock context "line_item is part of a shipment" do let!(:order) { create(:order_with_line_items) } - context "has stock in all stock locations" do + context "has stock in one stock location" do let(:line_item) { order.line_items.first } before do - variant_ids = order.line_items.map(&:variant_id) - Spree::StockItem.where(variant_id: variant_ids).update_all(count_on_hand: 10, backorderable: false) + line_item.variant.stock_items.update_all(count_on_hand: 10, backorderable: false) end include_examples "passes validation" end - context "doesn't have stock in a particular stock location" do - let(:variant) { create(:variant) } - let(:line_item) { order.line_items.find_by(variant_id: variant.id) } + context "with stock in multiple locations" do + let(:line_item) { order.line_items.first } + let(:variant) { line_item.variant } let!(:stock_location_1) { create(:stock_location, name: "Test Warehouse", active: false) } before do - order.contents.add(variant, 1, stock_location_quantities: { stock_location_1.id => 1 }) - order.contents.advance - stock_location_1.stock_items.update_all(count_on_hand: 0, backorderable: false) + shipment = order.shipments.create(stock_location: stock_location_1) + order.contents.add(variant, 1, shipment: shipment) end - include_examples "fails validation" + context "but no stock in either location" do + before do + variant.stock_items.update_all(count_on_hand: 0, backorderable: false) + end + include_examples "fails validation" + end + + context "but no stock in one location" do + before do + stock_location_1.stock_items.update_all(count_on_hand: 0, backorderable: false) + end + + include_examples "fails validation" + end + + context "with enough stock only across locations" do + before do + variant.stock_items.update_all(count_on_hand: 1, backorderable: false) + end + include_examples "passes validation" + end context "but inventory units are finalized" do before do - Spree::InventoryUnit.update_all(pending: false) + order.inventory_units.update_all(pending: false) end include_examples "passes validation" end end end + + context "line_item is split across two shipments" do + let!(:order) { create(:order_with_line_items) } + let(:line_item) { order.line_items.first } + let(:variant) { line_item.variant } + let(:stock_location) { order.shipments.first.stock_location } + + before do + shipment2 = order.shipments.create!(stock_location: order.shipments.first.stock_location) + order.contents.add(variant, 1, shipment: shipment2) + variant.stock_items.first.update_columns(count_on_hand: count_on_hand, backorderable: false) + end + + context "and there is just enough stock" do + let(:count_on_hand) { 2 } + include_examples "passes validation" + end + + context "and there is not enough stock" do + let(:count_on_hand) { 1 } + include_examples "fails validation" + end + + context "and there is no available stock" do + let(:count_on_hand) { 0 } + include_examples "fails validation" + end + end end end end