diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index 7bc718e857..58ca4c2b39 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -8,7 +8,7 @@ Spree::LineItem.class_eval do # Redefining here to add the inverse_of option belongs_to :order, :class_name => "Spree::Order", inverse_of: :line_items - # Allows manual skipping of stock_availability check + # Allows manual skipping of Stock::AvailabilityValidator attr_accessor :skip_stock_check attr_accessible :max_quantity, :final_weight_volume, :price @@ -133,13 +133,6 @@ Spree::LineItem.class_eval do end alias_method_chain :update_inventory, :scoping - # Override of Spree validation method - # Added check for in-memory :skip_stock_check attribute - def stock_availability - return if skip_stock_check || sufficient_stock? - errors.add(:quantity, I18n.t('validation.exceeds_available_stock')) - end - def scoper @scoper ||= OpenFoodNetwork::ScopeVariantToHub.new(order.distributor) end diff --git a/app/models/spree/stock/availability_validator_decorator.rb b/app/models/spree/stock/availability_validator_decorator.rb new file mode 100644 index 0000000000..29f9d0413c --- /dev/null +++ b/app/models/spree/stock/availability_validator_decorator.rb @@ -0,0 +1,41 @@ +Spree::Stock::AvailabilityValidator.class_eval do + def validate(line_item) + # OFN specific check for in-memory :skip_stock_check attribute + return if line_item.skip_stock_check + + quantity_to_validate = line_item.quantity - quantity_in_shipment(line_item) + return if quantity_to_validate < 1 + + validate_quantity(line_item, quantity_to_validate) + end + + private + + # This is an adapted version of a fix to the inventory_units not being considered here. + # See #3090 for details. + # This can be removed after upgrading to Spree 2.4. + def quantity_in_shipment(line_item) + shipment = line_item_shipment(line_item) + return 0 unless shipment + + units = shipment.inventory_units_for(line_item.variant) + units.count + end + + def line_item_shipment(line_item) + return line_item.target_shipment if line_item.target_shipment + return line_item.order.shipments.first if line_item.order.present? && line_item.order.shipments.any? + end + + # This is the spree v2.0.4 implementation of validate + # But using the calculated quantity instead of the line_item.quantity. + def validate_quantity(line_item, quantity) + quantifier = Spree::Stock::Quantifier.new(line_item.variant_id) + return if quantifier.can_supply? quantity + + variant = line_item.variant + display_name = %Q{#{variant.name}} + display_name += %Q{ (#{variant.options_text})} unless variant.options_text.blank? + line_item.errors[:quantity] << Spree.t(:out_of_stock, :scope => :order_populator, :item => display_name.inspect) + end +end diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index e6a3423868..6cbdd615b2 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -139,11 +139,10 @@ feature %q{ end context "submitting data to the server" do - let!(:o1) { create(:order_with_distributor, state: 'complete', completed_at: Time.zone.now ) } - let!(:li1) { create(:line_item_with_shipment, order: o1, :quantity => 5 ) } + let!(:order) { create(:completed_order_with_fees) } before :each do - li1.variant.update_attributes(on_hand: 1, on_demand: false) + order.line_items.second.destroy # we keep only one line item for this test visit '/admin/orders/bulk_management' end @@ -162,13 +161,14 @@ feature %q{ context "when unacceptable data is sent to the server" do it "displays an update button which submits pending changes" do expect(page).to have_no_selector "#save-bar" - fill_in "quantity", :with => li1.variant.on_hand + li1.quantity + 10 + line_item = order.line_items.first + fill_in "quantity", :with => line_item.variant.on_hand + line_item.quantity + 10 expect(page).to have_selector "input[name='quantity'].ng-dirty" expect(page).to have_selector "#save-bar", text: "You have unsaved changes" click_button "Save Changes" expect(page).to have_selector "#save-bar", text: "Fields with red borders contain errors." expect(page).to have_selector "input[name='quantity'].ng-dirty.update-error" - expect(page).to have_content "exceeds available stock. Please ensure line items have a valid quantity." + expect(page).to have_content "is out of stock" end end end diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index 4690285be3..9f85774bf9 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -585,24 +585,5 @@ module Spree }.to change(Spree::OptionValue, :count).by(0) end end - - describe "checking stock availability" do - let(:line_item) { LineItem.new } - - context "when skip_stock_check is not set" do - it "checks stock" do - expect(line_item).to receive(:sufficient_stock?) { true } - line_item.send(:stock_availability) - end - end - - context "when skip_stock_check is set to true" do - before { line_item.skip_stock_check = true } - it "does not check stock" do - expect(line_item).to_not receive(:sufficient_stock?) - line_item.send(:stock_availability) - end - end - end end end diff --git a/spec/models/spree/stock/availability_validator_spec.rb b/spec/models/spree/stock/availability_validator_spec.rb new file mode 100644 index 0000000000..de14a5a1c0 --- /dev/null +++ b/spec/models/spree/stock/availability_validator_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +module Spree + module Stock + describe AvailabilityValidator do + let(:validator) { AvailabilityValidator.new({}) } + + context "line item without existing inventory units" do + let(:order) { create(:order_with_line_items) } + let(:line_item) { order.line_items.first } + + context "available quantity when variant.on_hand > line_item.quantity" do + it "suceeds" do + line_item.quantity = line_item.variant.on_hand - 1 + validator.validate(line_item) + expect(line_item).to be_valid + end + end + + context "unavailable quantity when variant.on_hand < line_item.quantity" do + it "fails" do + line_item.quantity = line_item.variant.on_hand + 1 + validator.validate(line_item) + expect(line_item).not_to be_valid + end + + it "succeeds with line_item skip_stock_check" do + line_item.skip_stock_check = true + line_item.quantity = line_item.variant.on_hand + 1 + validator.validate(line_item) + expect(line_item).to be_valid + end + end + end + + describe "line item with existing inventory units" do + let(:order) { create(:completed_order_with_totals) } + let(:line_item) { order.line_items.first } + + context "when adding all variant.on_hand quantity to existing line_item quantity" do + it "succeeds because it excludes existing inventory units from the validation" do + line_item.quantity += line_item.variant.on_hand + validator.validate(line_item) + expect(line_item).to be_valid + end + + it "fails if one more item is added" do + line_item.quantity += line_item.variant.on_hand + 1 + validator.validate(line_item) + expect(line_item).not_to be_valid + end + end + end + end + end +end