diff --git a/core/app/models/spree/fulfilment_changer.rb b/core/app/models/spree/fulfilment_changer.rb index 61a4234716f..1178c08f8e6 100644 --- a/core/app/models/spree/fulfilment_changer.rb +++ b/core/app/models/spree/fulfilment_changer.rb @@ -86,9 +86,9 @@ def run! # we can take from the desired location, we could end up with some items being backordered. def run_tracking_inventory # Retrieve how many on hand items we can take from desired stock location - available_quantity = [desired_shipment.stock_location.count_on_hand(variant), default_on_hand_quantity].max - + available_quantity = get_available_quantity new_on_hand_quantity = [available_quantity, quantity].min + backordered_quantity = get_backordered_quantity(available_quantity, new_on_hand_quantity) unstock_quantity = desired_shipment.stock_location.backorderable?(variant) ? quantity : new_on_hand_quantity ActiveRecord::Base.transaction do @@ -105,19 +105,21 @@ def run_tracking_inventory # These two statements are the heart of this class. We change the number # of inventory units requested from one shipment to the other. # We order by state, because `'backordered' < 'on_hand'`. + # We start to move the new actual backordered quantity, so the remaining + # quantity can be set to on_hand state. current_shipment. inventory_units. where(variant: variant). order(state: :asc). - limit(new_on_hand_quantity). - update_all(shipment_id: desired_shipment.id, state: :on_hand) + limit(backordered_quantity). + update_all(shipment_id: desired_shipment.id, state: :backordered) current_shipment. inventory_units. where(variant: variant). order(state: :asc). - limit(quantity - new_on_hand_quantity). - update_all(shipment_id: desired_shipment.id, state: :backordered) + limit(quantity - backordered_quantity). + update_all(shipment_id: desired_shipment.id, state: :on_hand) end end @@ -141,11 +143,22 @@ def handle_stock_counts? current_shipment.order.completed? && current_stock_location != desired_stock_location end - def default_on_hand_quantity + def get_available_quantity + if current_stock_location != desired_stock_location + desired_location_quantifier.positive_stock + else + sl_availability = current_location_quantifier.positive_stock + shipment_availability = current_shipment.inventory_units.where(variant: variant).on_hand.count + sl_availability + shipment_availability + end + end + + def get_backordered_quantity(available_quantity, new_on_hand_quantity) if current_stock_location != desired_stock_location - 0 + quantity - new_on_hand_quantity else - current_shipment.inventory_units.where(variant: variant).on_hand.count + shipment_quantity = current_shipment.inventory_units.where(variant: variant).size + shipment_quantity - available_quantity end end @@ -156,11 +169,19 @@ def current_shipment_not_already_shipped end def enough_stock_at_desired_location - unless Spree::Stock::Quantifier.new(variant, desired_stock_location).can_supply?(quantity) + unless desired_location_quantifier.can_supply?(quantity) errors.add(:desired_shipment, :not_enough_stock_at_desired_location) end end + def desired_location_quantifier + @desired_location_quantifier ||= Spree::Stock::Quantifier.new(variant, desired_stock_location) + end + + def current_location_quantifier + @current_location_quantifier ||= Spree::Stock::Quantifier.new(variant, current_stock_location) + end + def desired_shipment_different_from_current if desired_shipment.id == current_shipment.id errors.add(:desired_shipment, :can_not_transfer_within_same_shipment) diff --git a/core/app/models/spree/stock/quantifier.rb b/core/app/models/spree/stock/quantifier.rb index cb5f31112a9..260c11a3e94 100644 --- a/core/app/models/spree/stock/quantifier.rb +++ b/core/app/models/spree/stock/quantifier.rb @@ -11,14 +11,8 @@ class Quantifier # If unspecified it will check inventory in all available StockLocations def initialize(variant, stock_location_or_id = nil) @variant = variant - @stock_items = variant.stock_items.select do |stock_item| - if stock_location_or_id - stock_item.stock_location == stock_location_or_id || - stock_item.stock_location_id == stock_location_or_id - else - stock_item.stock_location.active? - end - end + @stock_location_or_id = stock_location_or_id + @stock_items = variant_stock_items end # Returns the total number of inventory units on hand for the variant. @@ -26,7 +20,7 @@ def initialize(variant, stock_location_or_id = nil) # @return [Fixnum] number of inventory units on hand, or infinity if # inventory is not tracked on the variant. def total_on_hand - if @variant.should_track_inventory? + if variant.should_track_inventory? stock_items.sum(&:count_on_hand) else Float::INFINITY @@ -48,6 +42,36 @@ def backorderable? def can_supply?(required) total_on_hand >= required || backorderable? end + + def positive_stock + return unless stock_location + + on_hand = stock_location.count_on_hand(variant) + on_hand.positive? ? on_hand : 0 + end + + private + + attr_reader :variant, :stock_location_or_id + + def stock_location + @stock_location ||= if stock_location_or_id.is_a?(Spree::StockLocation) + stock_location_or_id + else + Spree::StockLocation.find_by(id: stock_location_or_id) + end + end + + def variant_stock_items + variant.stock_items.select do |stock_item| + if stock_location_or_id + stock_item.stock_location == stock_location_or_id || + stock_item.stock_location_id == stock_location_or_id + else + stock_item.stock_location.active? + end + end + end end end end diff --git a/core/spec/models/spree/fulfilment_changer_spec.rb b/core/spec/models/spree/fulfilment_changer_spec.rb index b850d2d4af7..ee6ec44c599 100644 --- a/core/spec/models/spree/fulfilment_changer_spec.rb +++ b/core/spec/models/spree/fulfilment_changer_spec.rb @@ -6,7 +6,7 @@ let(:variant) { create(:variant) } let(:track_inventory) { true } - let(:order) do + let!(:order) do create( :completed_order_with_totals, line_items_attributes: [ @@ -19,10 +19,8 @@ end let(:current_shipment) { order.shipments.first } - let(:desired_shipment) { order.shipments.create!(stock_location: desired_stock_location) } + let!(:desired_shipment) { order.shipments.create!(stock_location: desired_stock_location) } let(:desired_stock_location) { current_shipment.stock_location } - let(:current_shipment_inventory_unit_count) { 1 } - let(:quantity) { current_shipment_inventory_unit_count } let(:shipment_splitter) do described_class.new( @@ -34,48 +32,7 @@ ) end - subject { shipment_splitter.run! } - - before do - order && desired_shipment - variant.stock_items.first.update_column(:count_on_hand, 100) - end - - context "when the current shipment stock location is the same of the target shipment" do - context "when the stock location is empty" do - before do - variant.stock_items.first.update_column(:count_on_hand, 0) - end - - context "when the inventory unit is backordered" do - before do - current_shipment.inventory_units.first.update state: :backordered - end - - it "creates a new backordered inventory unit" do - subject - expect(desired_shipment.inventory_units.first).to be_backordered - end - end - - context "when the inventory unit is on hand" do - before do - current_shipment.inventory_units.first.update state: :on_hand - end - - it "creates a new on hand inventory unit" do - subject - expect(desired_shipment.inventory_units.first).to be_on_hand - end - end - end - end - - context "when tracking inventory is not set (same as false)" do - let(:current_shipment_inventory_unit_count) { 2 } - let(:quantity) { 1 } - let(:track_inventory) { nil } - + shared_examples_for "moves inventory units between shipments" do it "adds the desired inventory units to the desired shipment" do expect { subject }.to change { desired_shipment.inventory_units.length }.by(quantity) end @@ -83,12 +40,19 @@ it "removes the desired inventory units from the current shipment" do expect { subject }.to change { current_shipment.inventory_units.length }.by(-quantity) end + end + shared_examples_for "recalculates shipping costs and order totals" do it "recalculates shipping costs for the current shipment" do expect(current_shipment).to receive(:refresh_rates) subject end + it "recalculates shipping costs for the new shipment" do + expect(desired_shipment).to receive(:refresh_rates) + subject + end + it 'updates order totals' do original_total = order.total original_shipment_total = order.shipment_total @@ -97,15 +61,11 @@ to change { order.total }.from(original_total).to(original_total + original_shipment_total). and change { order.shipment_total }.by(original_shipment_total) end + end + shared_examples_for "completes transfer to another stock location without tracking inventory changes" do context "when transferring to another stock location" do let(:desired_stock_location) { create(:stock_location) } - let!(:stock_item) do - variant.stock_items.find_or_create_by!( - stock_location: desired_stock_location, - variant: variant, - ) - end it "is marked as a successful transfer" do expect(subject).to be true @@ -121,86 +81,105 @@ end end - context "when not tracking inventory" do - let(:current_shipment_inventory_unit_count) { 2 } - let(:quantity) { 1 } - let(:track_inventory) { false } + shared_examples_for "properly manages inventory units" do + let(:stock_item) { variant.stock_items.find_by!(stock_location: current_shipment.stock_location) } - it "adds the desired inventory units to the desired shipment" do - expect { subject }.to change { desired_shipment.inventory_units.length }.by(quantity) - end + context "when there are backordered inventory units" do + let(:backordered_units_count) { 1 } - it "removes the desired inventory units from the current shipment" do - expect { subject }.to change { current_shipment.inventory_units.length }.by(-quantity) - end + before do + current_shipment.inventory_units.limit(backordered_units_count).update!(state: :backordered) + end - it "recalculates shipping costs for the current shipment" do - expect(current_shipment).to receive(:refresh_rates) - subject - end + context "when the stock is zero or negative" do + before do + stock_item.update_column(:count_on_hand, -backordered_units_count) + end - it 'updates order totals' do - original_total = order.total - original_shipment_total = order.shipment_total + it "doesn't change inventory units state" do + expect { subject } + .not_to change { order.inventory_units.map(&:state).sort } + .from(%w[backordered on_hand]) + end + end - expect { subject }. - to change { order.total }.from(original_total).to(original_total + original_shipment_total). - and change { order.shipment_total }.by(original_shipment_total) - end + context "when backordered items can become on hand" do + before do + stock_item.update_column(:count_on_hand, backordered_units_count) + end - context "when transferring to another stock location" do - let(:desired_stock_location) { create(:stock_location) } - let!(:stock_item) do - variant.stock_items.find_or_create_by!( - stock_location: desired_stock_location, - variant: variant, - ) + it "makes all inventory units on hand" do + expect { subject } + .to change { order.inventory_units.map(&:state).sort } + .from(%w[backordered on_hand]).to(%w[on_hand on_hand]) + end end + end - it "is marked as a successful transfer" do - expect(subject).to be true + context "when all inventory units are on hand" do + before do + current_shipment.inventory_units.update_all(state: :on_hand) end - it "does not stock in the current stock location" do - expect { subject }.not_to change { current_shipment.stock_location.count_on_hand(variant) } + context "when the stock is negative" do + before do + stock_item.update_column(:count_on_hand, -1) + end + + it "doesn't change the order inventory units state" do + expect { subject }.not_to change { order.inventory_units.map(&:state).sort } + end end + end - it "does not unstock the desired stock location" do - expect { subject }.not_to change { desired_shipment.stock_location.count_on_hand(variant) } + context "when the stock location is empty" do + before { stock_item.update_column(:count_on_hand, 0) } + + it "doesn't change the order inventory units state" do + expect { subject }.not_to change { order.inventory_units.map(&:state).sort } end end end - context "when the current shipment has enough inventory units" do + subject { shipment_splitter.run! } + + before do + variant.stock_items.first.update_column(:count_on_hand, 100) + end + + context "when tracking inventory (default behavior)" do let(:current_shipment_inventory_unit_count) { 2 } let(:quantity) { 1 } - it "adds the desired inventory units to the desired shipment" do - expect { subject }.to change { desired_shipment.inventory_units.length }.by(quantity) - end + it_behaves_like "moves inventory units between shipments" + it_behaves_like "properly manages inventory units" + end - it "removes the desired inventory units from the current shipment" do - expect { subject }.to change { current_shipment.inventory_units.length }.by(-quantity) - end + context "when tracking inventory is not set (same as false)" do + let(:current_shipment_inventory_unit_count) { 2 } + let(:quantity) { 1 } + let(:track_inventory) { nil } - it "recalculates shipping costs for the current shipment" do - expect(current_shipment).to receive(:refresh_rates) - subject - end + it_behaves_like "moves inventory units between shipments" + it_behaves_like "recalculates shipping costs and order totals" + it_behaves_like "completes transfer to another stock location without tracking inventory changes" + end - it "recalculates shipping costs for the new shipment" do - expect(desired_shipment).to receive(:refresh_rates) - subject - end + context "when not tracking inventory" do + let(:current_shipment_inventory_unit_count) { 2 } + let(:quantity) { 1 } + let(:track_inventory) { false } - it 'updates order totals' do - original_total = order.total - original_shipment_total = order.shipment_total + it_behaves_like "moves inventory units between shipments" + it_behaves_like "completes transfer to another stock location without tracking inventory changes" + end - expect { subject }. - to change { order.total }.from(original_total).to(original_total + original_shipment_total). - and change { order.shipment_total }.by(original_shipment_total) - end + context "when the current shipment has enough inventory units" do + let(:current_shipment_inventory_unit_count) { 2 } + let(:quantity) { 1 } + + it_behaves_like "moves inventory units between shipments" + it_behaves_like "recalculates shipping costs and order totals" context "when transferring to another stock location" do let(:desired_stock_location) { create(:stock_location) } @@ -404,11 +383,9 @@ context "when the current shipment is emptied out by the transfer" do let(:current_shipment_inventory_unit_count) { 30 } - let(:quantity) { 30 } + let(:quantity) { current_shipment_inventory_unit_count } - it "adds the desired inventory units to the desired shipment" do - expect { subject }.to change { desired_shipment.inventory_units.length }.by(quantity) - end + it_behaves_like "moves inventory units between shipments" it "removes the current shipment" do expect { subject }.to change { Spree::Shipment.count }.by(-1) @@ -421,9 +398,8 @@ let(:desired_shipment) { order.shipments.build(stock_location: current_shipment.stock_location) } - it "adds the desired inventory units to the desired shipment" do - expect { subject }.to change { Spree::Shipment.count }.by(1) - end + it_behaves_like "moves inventory units between shipments" + it_behaves_like "properly manages inventory units" context "if the desired shipment is invalid" do let(:desired_shipment) { order.shipments.build(stock_location_id: 99_999_999) } diff --git a/core/spec/models/spree/stock/quantifier_spec.rb b/core/spec/models/spree/stock/quantifier_spec.rb index 469f19295be..51d219f6ce7 100644 --- a/core/spec/models/spree/stock/quantifier_spec.rb +++ b/core/spec/models/spree/stock/quantifier_spec.rb @@ -2,17 +2,35 @@ require 'rails_helper' -RSpec.shared_examples_for 'unlimited supply' do - it 'can_supply? any amount' do - expect(subject.can_supply?(1)).to eq true - expect(subject.can_supply?(101)).to eq true - expect(subject.can_supply?(100_001)).to eq true - end -end - module Spree module Stock RSpec.describe Quantifier, type: :model do + shared_examples_for 'unlimited supply' do + it 'can_supply? any amount' do + expect(subject.can_supply?(1)).to eq true + expect(subject.can_supply?(101)).to eq true + expect(subject.can_supply?(100_001)).to eq true + end + end + + shared_examples_for "returns the positive stock on hand" do + context "when the stock location has no stock for the variant" do + it { is_expected.to be_zero } + end + + context "when the stock location has negative stock for the variant" do + before { stock_item.set_count_on_hand(-1) } + + it { is_expected.to be_zero } + end + + context "when the stock location has positive stock for the variant" do + before { stock_item.set_count_on_hand(10) } + + it { is_expected.to eq(10) } + end + end + let(:target_stock_location) { nil } let!(:stock_location) { create :stock_location_with_items } let!(:stock_item) { stock_location.stock_items.order(:id).first } @@ -108,6 +126,35 @@ module Stock expect(subject.can_supply?(6)).to eq false end end + + describe "#positive_stock" do + let(:variant) { create(:variant) } + let(:stock_location) { create(:stock_location) } + let(:stock_item) { stock_location.set_up_stock_item(variant) } + let(:instance) { described_class.new(variant, stock_location_or_id) } + + subject { instance.positive_stock } + + context "when stock location is not present" do + let(:stock_location_or_id) { nil } + + it { is_expected.to be_nil } + end + + context "when stock_location_id is present" do + context "when stock_location_id is a stock location" do + let(:stock_location_or_id) { stock_location } + + it_behaves_like "returns the positive stock on hand" + end + + context "when stock_location_id is a stock location id" do + let(:stock_location_or_id) { stock_location.id } + + it_behaves_like "returns the positive stock on hand" + end + end + end end end end