Merge pull request #1693 from jhawthorn/availability_validator

Make AvailabilityValidator check inventory per StockLocation instead of per Shipment

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]`

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) }

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

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.

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