diff --git a/api/spec/requests/spree/api/orders_controller_spec.rb b/api/spec/requests/spree/api/orders_controller_spec.rb index b64793ccc92..3ef6105a7a6 100644 --- a/api/spec/requests/spree/api/orders_controller_spec.rb +++ b/api/spec/requests/spree/api/orders_controller_spec.rb @@ -506,6 +506,7 @@ module Spree context "with a line item" do let(:order) { create(:order_with_line_items) } + let(:line_item) { order.line_items.first } it "can empty an order" do create(:adjustment, order: order, adjustable: order) diff --git a/core/app/models/spree/stock/adjuster.rb b/core/app/models/spree/stock/adjuster.rb deleted file mode 100644 index fdc825e24c0..00000000000 --- a/core/app/models/spree/stock/adjuster.rb +++ /dev/null @@ -1,28 +0,0 @@ -module Spree - module Stock - # Used by Prioritizer to adjust item quantities. - # - # See spec/models/spree/stock/prioritizer_spec.rb for use cases. - class Adjuster - attr_accessor :inventory_unit, :status, :fulfilled - - def initialize(inventory_unit, status) - @inventory_unit = inventory_unit - @status = status - @fulfilled = false - end - - def adjust(package) - if fulfilled? - package.remove(inventory_unit) - else - self.fulfilled = true - end - end - - def fulfilled? - fulfilled - end - end - end -end diff --git a/core/app/models/spree/stock/availability.rb b/core/app/models/spree/stock/availability.rb new file mode 100644 index 00000000000..c6dbad39380 --- /dev/null +++ b/core/app/models/spree/stock/availability.rb @@ -0,0 +1,70 @@ +module Spree + module Stock + # This class manages checking stock availability efficiently for a set of + # Variants and StockLocations. + # + # This serves a similar role to Spree::Stock::Quantifier, but is more + # efficient by checking multiple variants at once. + class Availability + # @param variants [Array] variants to check stock of + # @param stock_locations [Array] stock_locations to check for stock in + def initialize(variants:, stock_locations: Spree::StockLocation.active) + @variants = variants + @variant_map = variants.index_by(&:id) + @stock_locations = stock_locations + end + + # Get the on_hand stock quantities + # @return [HashSpree::StockQuantities>] A map of stock_location_ids to the stock quantities available in that location + def on_hand_by_stock_location_id + counts_on_hand.to_a.group_by do |(_, stock_location_id), _| + stock_location_id + end.transform_values do |values| + Spree::StockQuantities.new( + values.map do |(variant_id, _), count| + variant = @variant_map[variant_id] + count = Float::INFINITY if !variant.should_track_inventory? + count = 0 if count < 0 + [variant, count] + end.to_h + ) + end + end + + # Get the on_hand stock quantities + # @return [HashSpree::StockQuantities>] A map of stock_location_ids to the stock quantities available in that location + def backorderable_by_stock_location_id + backorderables.group_by(&:second).transform_values do |variant_ids| + Spree::StockQuantities.new( + variant_ids.map do |variant_id, _| + variant = @variant_map[variant_id] + [variant, Float::INFINITY] + end.to_h + ) + end + end + + private + + def counts_on_hand + @counts_on_hand ||= + stock_item_scope. + group(:variant_id, :stock_location_id). + sum(:count_on_hand) + end + + def backorderables + @backorderables ||= + stock_item_scope. + where(backorderable: true). + pluck(:variant_id, :stock_location_id) + end + + def stock_item_scope + Spree::StockItem. + where(variant_id: @variants). + where(stock_location_id: @stock_locations) + end + end + end +end diff --git a/core/app/models/spree/stock/coordinator.rb b/core/app/models/spree/stock/coordinator.rb deleted file mode 100644 index bafc449548f..00000000000 --- a/core/app/models/spree/stock/coordinator.rb +++ /dev/null @@ -1,150 +0,0 @@ -module Spree - module Stock - class Coordinator - attr_reader :order, :inventory_units - - def initialize(order, inventory_units = nil) - @order = order - @inventory_units = inventory_units || InventoryUnitBuilder.new(order).units - @preallocated_inventory_units = [] - end - - def shipments - packages.map(&:shipment) - end - - private - - def packages - packages = build_location_configured_packages - packages = build_packages(packages) - packages = prioritize_packages(packages) - packages.each do |package| - package.shipment = package.to_shipment - end - packages = estimate_packages(packages) - validate_packages(packages) - packages - end - - # Build packages for the inventory units that have preferred stock locations first - # - # Certain variants have been selected to be fulfilled from a particular stock - # location during the process of the order being created. The rest of the - # service objects the coordinator uses do a lot of automated logic to - # determine which stock location is best for the inventory unit to be - # fulfilled from, but for these special snowflakes we KNOW which stock - # location they should be fulfilled from. So rather than sending these units - # through the rest of the packing / prioritization, lets just put them - # in packages we know they should be in and deal with other automatically- - # handled inventory units otherwise. - def build_location_configured_packages(packages = []) - order.order_stock_locations.where(shipment_fulfilled: false).group_by(&:stock_location).each do |stock_location, stock_location_configurations| - units = stock_location_configurations.flat_map do |stock_location_configuration| - unallocated_inventory_units.select { |iu| iu.variant == stock_location_configuration.variant }.take(stock_location_configuration.quantity) - end - packer = build_packer(stock_location, units) - packages += packer.packages - @preallocated_inventory_units += units - end - packages - end - - # Build packages as per stock location - # - # It needs to check whether each stock location holds at least one stock - # item for the order. In case none is found it wouldn't make any sense - # to build a package because it would be empty. Plus we avoid errors down - # the stack because it would assume the stock location has stock items - # for the given order - # - # Returns an array of Package instances - def build_packages(packages = []) - stock_location_variant_ids.each do |stock_location, variant_ids| - units_for_location = unallocated_inventory_units.select { |unit| variant_ids.include?(unit.variant_id) } - packer = build_packer(stock_location, units_for_location) - packages += packer.packages - end - packages - end - - # This finds the variants we're looking for in each active stock location. - # It returns a hash like: - # { - # => , - # => , - # ..., - # } - # This is done in an awkward way for performance reasons. It uses two - # queries that are kept as performant as possible, and only loads the - # minimum required ActiveRecord objects. - def stock_location_variant_ids - # associate the variant ids we're interested in with stock location ids - location_variant_ids = Spree::StockItem. - where(variant_id: unallocated_variant_ids). - joins(:stock_location). - merge(Spree::StockLocation.active). - pluck(:stock_location_id, :variant_id) - - # load activerecord objects for the stock location ids and turn them - # into a lookup hash like: - # { - # => , - # ..., - # } - location_lookup = Spree::StockLocation. - where(id: location_variant_ids.map(&:first).uniq). - map { |l| [l.id, l] }. - to_h - - # build the final lookup hash of - # { => , ...} - # using the previous results - location_variant_ids.each_with_object({}) do |(location_id, variant_id), hash| - location = location_lookup[location_id] - hash[location] ||= Set.new - hash[location] << variant_id - end - end - - def unallocated_inventory_units - inventory_units - @preallocated_inventory_units - end - - def unallocated_variant_ids - unallocated_inventory_units.map(&:variant_id).uniq - end - - def prioritize_packages(packages) - prioritizer = Prioritizer.new(inventory_units, packages) - prioritizer.prioritized_packages - end - - def estimate_packages(packages) - estimator = Spree::Config.stock.estimator_class.new - packages.each do |package| - package.shipment.shipping_rates = estimator.shipping_rates(package) - end - packages - end - - def validate_packages(packages) - desired_quantity = inventory_units.size - packaged_quantity = packages.sum(&:quantity) - if packaged_quantity != desired_quantity - raise Spree::Order::InsufficientStock, - "Was only able to package #{packaged_quantity} inventory units of #{desired_quantity} requested" - end - end - - def build_packer(stock_location, inventory_units) - Packer.new(stock_location, inventory_units, splitters(stock_location)) - end - - def splitters(_stock_location) - # extension point to return custom splitters for a location - Rails.application.config.spree.stock_splitters - end - end - end -end diff --git a/core/app/models/spree/stock/packer.rb b/core/app/models/spree/stock/packer.rb deleted file mode 100644 index 34f400cf3d4..00000000000 --- a/core/app/models/spree/stock/packer.rb +++ /dev/null @@ -1,51 +0,0 @@ -module Spree - module Stock - class Packer - attr_reader :stock_location, :inventory_units, :splitters - - def initialize(stock_location, inventory_units, splitters = [Splitter::Base]) - @stock_location = stock_location - @inventory_units = inventory_units - @splitters = splitters - end - - def packages - Spree::Stock::SplitterChain.new(stock_location, splitters).split([default_package]) - end - - def default_package - package = Package.new(stock_location) - inventory_units.group_by(&:variant).each do |variant, variant_inventory_units| - units = variant_inventory_units.clone - if variant.should_track_inventory? - stock_item = stock_item_for(variant.id) - next unless stock_item - - on_hand, backordered = stock_item.fill_status(units.count) - package.add_multiple units.slice!(0, on_hand), :on_hand if on_hand > 0 - package.add_multiple units.slice!(0, backordered), :backordered if backordered > 0 - else - package.add_multiple units - end - end - package - end - - private - - def stock_item_for(variant_id) - stock_item_lookup[variant_id] - end - - # Returns a lookup table in the form of: - # { => , ...} - def stock_item_lookup - @stock_item_lookup ||= Spree::StockItem. - where(variant_id: inventory_units.map(&:variant_id).uniq). - where(stock_location_id: stock_location.id). - map { |stock_item| [stock_item.variant_id, stock_item] }. - to_h # there is only one stock item per variant in a stock location - end - end - end -end diff --git a/core/app/models/spree/stock/prioritizer.rb b/core/app/models/spree/stock/prioritizer.rb deleted file mode 100644 index b153300e47f..00000000000 --- a/core/app/models/spree/stock/prioritizer.rb +++ /dev/null @@ -1,48 +0,0 @@ -module Spree - module Stock - class Prioritizer - attr_reader :packages, :inventory_units - - def initialize(inventory_units, packages, adjuster_class = Adjuster) - @inventory_units = inventory_units - @packages = packages - @adjuster_class = adjuster_class - end - - def prioritized_packages - sort_packages - adjust_packages - prune_packages - packages - end - - private - - def adjust_packages - inventory_units.each do |inventory_unit| - adjuster = @adjuster_class.new(inventory_unit, :on_hand) - - visit_packages(adjuster) - - adjuster.status = :backordered - visit_packages(adjuster) - end - end - - def visit_packages(adjuster) - packages.each do |package| - item = package.find_item adjuster.inventory_unit, adjuster.status - adjuster.adjust(package) if item - end - end - - def sort_packages - # order packages by preferred stock_locations - end - - def prune_packages - packages.reject!(&:empty?) - end - end - end -end diff --git a/core/app/models/spree/stock/simple_coordinator.rb b/core/app/models/spree/stock/simple_coordinator.rb new file mode 100644 index 00000000000..300c63b22c6 --- /dev/null +++ b/core/app/models/spree/stock/simple_coordinator.rb @@ -0,0 +1,105 @@ +module Spree + module Stock + # A simple implementation of Stock Coordination + # + # The algorithm for allocating inventory is naive: + # * For each available Stock Location + # * Allocate as much on hand inventory as possible from this location + # * Remove the amount allocated from the amount desired + # * Repeat but for backordered inventory + # * Combine allocated and on hand inventory into a single shipment per-location + # + # After allocation, splitters are run on each Package (as configured in + # Rails.application.config.spree.stock_splitters) + # + # Finally, shipping rates are calculated using the class configured as + # Spree::Config.stock.estimator_class. + class SimpleCoordinator + attr_reader :order + + def initialize(order, inventory_units = nil) + @order = order + @inventory_units = inventory_units || InventoryUnitBuilder.new(order).units + @splitters = Rails.application.config.spree.stock_splitters + @stock_locations = Spree::StockLocation.active + + @inventory_units_by_variant = @inventory_units.group_by(&:variant) + @desired = Spree::StockQuantities.new(@inventory_units_by_variant.transform_values(&:count)) + @availability = Spree::Stock::Availability.new( + variants: @desired.variants, + stock_locations: @stock_locations + ) + end + + def shipments + @shipments ||= build_shipments + end + + private + + def build_shipments + # Allocate any available on hand inventory + on_hand_packages = allocate_inventory(@availability.on_hand_by_stock_location_id) + + # allocate any remaining desired inventory from backorders + backordered_packages = allocate_inventory(@availability.backorderable_by_stock_location_id) + + unless @desired.empty? + raise Spree::Order::InsufficientStock + end + + packages = @stock_locations.map do |stock_location| + # Combine on_hand and backorders into a single package per-location + on_hand = on_hand_packages[stock_location.id] || Spree::StockQuantities.new + backordered = backordered_packages[stock_location.id] || Spree::StockQuantities.new + + # Skip this location it has no inventory + next if on_hand.empty? && backordered.empty? + + # Turn our raw quantities into a Stock::Package + package = Spree::Stock::Package.new(stock_location) + package.add_multiple(get_units(on_hand), :on_hand) + package.add_multiple(get_units(backordered), :backordered) + + package + end.compact + + # Split the packages + packages = split_packages(packages) + + # Turn the Stock::Packages into a Shipment with rates + packages.map do |package| + shipment = package.shipment = package.to_shipment + shipment.shipping_rates = Spree::Config.stock.estimator_class.new.shipping_rates(package) + shipment + end + end + + def split_packages(initial_packages) + initial_packages.flat_map do |initial_package| + stock_location = initial_package.stock_location + Spree::Stock::SplitterChain.new(stock_location, @splitters).split([initial_package]) + end + end + + def allocate_inventory(availability_by_location) + availability_by_location.transform_values do |available| + # Find the desired inventory which is available at this location + packaged = available & @desired + + # Remove found inventory from desired + @desired -= packaged + + packaged + end + end + + def get_units(quantities) + # Change our raw quantities back into inventory units + quantities.flat_map do |variant, quantity| + @inventory_units_by_variant[variant].shift(quantity) + end + end + end + end +end diff --git a/core/app/models/spree/stock_quantities.rb b/core/app/models/spree/stock_quantities.rb new file mode 100644 index 00000000000..5da2bf3f3d4 --- /dev/null +++ b/core/app/models/spree/stock_quantities.rb @@ -0,0 +1,81 @@ +module Spree + # A value object to hold a map of variants to their quantities + class StockQuantities + attr_reader :quantities + include Enumerable + + # @param quantities [HashNumeric>] + def initialize(quantities = {}) + raise ArgumentError unless quantities.keys.all?{ |v| v.is_a?(Spree::Variant) } + raise ArgumentError unless quantities.values.all?{ |v| v.is_a?(Numeric) } + + @quantities = quantities + end + + # @yield [variant, quantity] + def each(&block) + @quantities.each(&block) + end + + # @param variant [Spree::Variant] + # @return [Integer] the quantity of variant + def [](variant) + @quantities[variant] + end + + # @return [Array] the variants being tracked + def variants + @quantities.keys.uniq + end + + # Adds two StockQuantities together + # @return [Spree::StockQuantities] + def +(other) + combine_with(other) do |_variant, a, b| + (a || 0) + (b || 0) + end + end + + # Subtracts another StockQuantities from this one + # @return [Spree::StockQuantities] + def -(other) + combine_with(other) do |_variant, a, b| + (a || 0) - (b || 0) + end + end + + # Finds the intersection or common subset of two StockQuantities: the + # stock which exists in both StockQuantities. + # @return [Spree::StockQuantities] + def &(other) + combine_with(other) do |_variant, a, b| + next unless a && b + [a, b].min + end + end + + # A StockQuantities is empty if all variants have zero quantity + # @return [true,false] + def empty? + @quantities.values.all?(&:zero?) + end + + def ==(other) + self.class == other.class && + quantities == other.quantities + end + + protected + + def combine_with(other) + self.class.new( + (variants | other.variants).map do |variant| + a = self[variant] + b = other[variant] + value = yield variant, a, b + [variant, value] + end.to_h.compact + ) + end + end +end diff --git a/core/lib/spree/core/stock_configuration.rb b/core/lib/spree/core/stock_configuration.rb index a7ae44bee2f..2800dc6807d 100644 --- a/core/lib/spree/core/stock_configuration.rb +++ b/core/lib/spree/core/stock_configuration.rb @@ -5,7 +5,7 @@ class StockConfiguration attr_writer :estimator_class def coordinator_class - @coordinator_class ||= '::Spree::Stock::Coordinator' + @coordinator_class ||= '::Spree::Stock::SimpleCoordinator' @coordinator_class.constantize end diff --git a/core/lib/spree/testing_support/factories/stock_packer_factory.rb b/core/lib/spree/testing_support/factories/stock_packer_factory.rb deleted file mode 100644 index 7a977d3790f..00000000000 --- a/core/lib/spree/testing_support/factories/stock_packer_factory.rb +++ /dev/null @@ -1,13 +0,0 @@ -require 'spree/testing_support/factories/stock_location_factory' - -FactoryGirl.define do - # must use build() - factory :stock_packer, class: Spree::Stock::Packer do - transient do - stock_location { build(:stock_location) } - contents [] - end - - initialize_with { new(stock_location, contents) } - end -end diff --git a/core/spec/lib/spree/core/stock_configuration_spec.rb b/core/spec/lib/spree/core/stock_configuration_spec.rb index 8afd3c0f621..455a6db2062 100644 --- a/core/spec/lib/spree/core/stock_configuration_spec.rb +++ b/core/spec/lib/spree/core/stock_configuration_spec.rb @@ -6,7 +6,7 @@ subject { stock_configuration.coordinator_class } it "returns Spree::Stock::Coordinator" do - is_expected.to be ::Spree::Stock::Coordinator + is_expected.to be ::Spree::Stock::SimpleCoordinator end context "with another constant name assiged" do diff --git a/core/spec/lib/spree/core/testing_support/factories/stock_packer_factory_spec.rb b/core/spec/lib/spree/core/testing_support/factories/stock_packer_factory_spec.rb deleted file mode 100644 index 08c86db0bc8..00000000000 --- a/core/spec/lib/spree/core/testing_support/factories/stock_packer_factory_spec.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'rails_helper' -require 'spree/testing_support/factories/stock_packer_factory' - -RSpec.describe 'stock packer factory' do - let(:factory_class) { Spree::Stock::Packer } - - describe 'plain stock packer' do - let(:factory) { :stock_packer } - - it "builds successfully" do - expect(build(factory)).to be_a(factory_class) - end - - # No test for .create, as it's a PORO - end -end diff --git a/core/spec/models/spree/order_spec.rb b/core/spec/models/spree/order_spec.rb index 0cf12d2b1c4..5147f0c24d1 100644 --- a/core/spec/models/spree/order_spec.rb +++ b/core/spec/models/spree/order_spec.rb @@ -1066,7 +1066,7 @@ def generate subject(:order) { create(:order) } it "assigns the coordinator returned shipments to its shipments" do shipment = build(:shipment) - allow_any_instance_of(Spree::Stock::Coordinator).to receive(:shipments).and_return([shipment]) + allow_any_instance_of(Spree::Stock::SimpleCoordinator).to receive(:shipments).and_return([shipment]) subject.create_proposed_shipments expect(subject.shipments).to eq [shipment] end diff --git a/core/spec/models/spree/stock/availability_spec.rb b/core/spec/models/spree/stock/availability_spec.rb new file mode 100644 index 00000000000..d66c6be4f2c --- /dev/null +++ b/core/spec/models/spree/stock/availability_spec.rb @@ -0,0 +1,127 @@ +require 'rails_helper' + +module Spree::Stock + describe Availability do + let(:variants) { Spree::Variant.all.to_a } + let(:infinity) { Float::INFINITY } + + let(:availability) { described_class.new(variants: variants) } + + let!(:stock_location1) { create(:stock_location) } + + subject { availability } + + describe "#on_hand_by_stock_location_id" do + subject { availability.on_hand_by_stock_location_id } + + context 'with a single variant' do + let!(:variant) { create(:master_variant) } + let(:stock_item) { variant.stock_items[0] } + + context 'with count_on_hand positive' do + before { stock_item.set_count_on_hand(2) } + + it "returns the correct value" do + expect(subject).to eq(stock_location1.id => Spree::StockQuantities.new(variant => 2)) + end + + context 'and backorderable false' do + before { stock_item.update!(backorderable: false) } + + it "returns the correct value" do + expect(subject).to eq(stock_location1.id => Spree::StockQuantities.new(variant => 2)) + end + end + end + + context 'with count_on_hand 0' do + before { stock_item.set_count_on_hand(0) } + + it "returns zero on_hand" do + expect(subject).to eq(stock_location1.id => Spree::StockQuantities.new(variant => 0)) + end + end + + context 'with count_on_hand negative' do + before { stock_item.set_count_on_hand(-1) } + + it "returns zero on_hand" do + expect(subject).to eq(stock_location1.id => Spree::StockQuantities.new(variant => 0)) + end + end + + context 'with no stock_item' do + before { stock_item.destroy! } + + it "returns empty hash" do + expect(subject).to eq({}) + end + end + + context 'with track_inventory=false' do + before { variant.update!(track_inventory: false) } + + it "has infinite inventory " do + expect(subject).to eq(stock_location1.id => Spree::StockQuantities.new(variant => infinity)) + end + end + + context 'with config.track_inventory_levels=false' do + before { Spree::Config.track_inventory_levels = false } + + it "has infinite inventory " do + expect(subject).to eq(stock_location1.id => Spree::StockQuantities.new(variant => infinity)) + end + end + end + end + + describe "#backorderable_by_stock_location_id" do + subject { availability.backorderable_by_stock_location_id } + + context 'with a single variant' do + let!(:variant) { create(:master_variant) } + let(:stock_item) { variant.stock_items[0] } + + context 'with backorderable false' do + before { stock_item.update!(backorderable: false) } + + context 'and positive count_on_hand' do + before { stock_item.set_count_on_hand(2) } + it { is_expected.to eq({}) } + end + + context 'and 0 count_on_hand' do + before { stock_item.set_count_on_hand(0) } + it { is_expected.to eq({}) } + end + end + + context 'with backorderable true' do + before { stock_item.update!(backorderable: true) } + + context 'and positive count_on_hand' do + before { stock_item.set_count_on_hand(2) } + it { is_expected.to eq(stock_location1.id => Spree::StockQuantities.new(variant => infinity)) } + end + + context 'and 0 count_on_hand' do + before { stock_item.set_count_on_hand(0) } + it { is_expected.to eq(stock_location1.id => Spree::StockQuantities.new(variant => infinity)) } + end + + context 'and negative count_on_hand' do + before { stock_item.set_count_on_hand(-1) } + it { is_expected.to eq(stock_location1.id => Spree::StockQuantities.new(variant => infinity)) } + end + end + + context 'with no stock_item' do + before { stock_item.destroy! } + + it { is_expected.to eq({}) } + end + end + end + end +end diff --git a/core/spec/models/spree/stock/packer_spec.rb b/core/spec/models/spree/stock/packer_spec.rb deleted file mode 100644 index 7664b021395..00000000000 --- a/core/spec/models/spree/stock/packer_spec.rb +++ /dev/null @@ -1,90 +0,0 @@ -require 'rails_helper' - -module Spree - module Stock - describe Packer, type: :model do - let!(:inventory_units) { Array.new(5) { build(:inventory_unit) } } - let(:stock_location) { create(:stock_location) } - - subject { Packer.new(stock_location, inventory_units) } - - context 'packages' do - it 'builds an array of packages' do - packages = subject.packages - expect(packages.size).to eq 1 - expect(packages.first.contents.size).to eq 5 - end - - it 'allows users to set splitters to an empty array' do - packages = Packer.new(stock_location, inventory_units, []).packages - expect(packages.size).to eq 1 - end - end - - context 'default_package' do - it 'contains all the items' do - package = subject.default_package - expect(package.contents.size).to eq 5 - end - - it 'variants are added as backordered without enough on_hand' do - inventory_units[0..2].each { |iu| stock_location.stock_item(iu.variant_id).set_count_on_hand(1) } - inventory_units[3..4].each { |iu| stock_location.stock_item(iu.variant_id).set_count_on_hand(0) } - - package = subject.default_package - expect(package.on_hand.size).to eq 3 - expect(package.backordered.size).to eq 2 - end - - context "location doesn't have order items in stock" do - let(:stock_location) { create(:stock_location, propagate_all_variants: false) } - let(:packer) { Packer.new(stock_location, inventory_units) } - - it "builds an empty package" do - expect(packer.default_package).to be_empty - end - end - - context "none on hand and not backorderable" do - let(:packer) { Packer.new(stock_location, inventory_units) } - - before do - stock_location.stock_items.update_all(backorderable: false) - stock_location.stock_items.each { |si| si.set_count_on_hand(0) } - end - - it "builds an empty package" do - expect(packer.default_package).to be_empty - end - end - - context "doesn't track inventory levels" do - let(:variant) { build(:variant) } - let(:order) { build(:order_with_line_items, line_items_count: 1) } - let(:line_item) { order.line_items.first } - let(:inventory_units) { - Array.new(30) do - build( - :inventory_unit, - order: order, - line_item: line_item, - variant: variant - ) - end - } - - before { Config.track_inventory_levels = false } - - it "doesn't bother stock items status in stock location" do - expect(subject.stock_location).not_to receive(:fill_status) - subject.default_package - end - - it "still creates package with proper quantity" do - expect(subject.default_package.quantity).to eql 30 - end - end - end - end - end -end diff --git a/core/spec/models/spree/stock/prioritizer_spec.rb b/core/spec/models/spree/stock/prioritizer_spec.rb deleted file mode 100644 index efd3408fa3d..00000000000 --- a/core/spec/models/spree/stock/prioritizer_spec.rb +++ /dev/null @@ -1,125 +0,0 @@ -require 'rails_helper' - -module Spree - module Stock - describe Prioritizer, type: :model do - let(:order) { mock_model(Order) } - let(:stock_location) { build(:stock_location) } - let(:variant) { build(:variant) } - - def inventory_units - @inventory_units ||= [] - end - - def build_inventory_unit - mock_model(InventoryUnit, variant: variant).tap do |unit| - inventory_units << unit - end - end - - def pack - package = Package.new(order) - yield(package) if block_given? - package - end - - it 'keeps a single package' do - package1 = pack do |package| - package.add build_inventory_unit - package.add build_inventory_unit - end - - packages = [package1] - prioritizer = Prioritizer.new(inventory_units, packages) - packages = prioritizer.prioritized_packages - expect(packages.size).to eq 1 - end - - it 'removes duplicate packages' do - package1 = pack do |package| - package.add build_inventory_unit - package.add build_inventory_unit - end - - package2 = pack do |package| - package.add inventory_units.first - package.add inventory_units.last - end - - packages = [package1, package2] - prioritizer = Prioritizer.new(inventory_units, packages) - packages = prioritizer.prioritized_packages - expect(packages.size).to eq 1 - end - - it 'split over 2 packages' do - package1 = pack do |package| - package.add build_inventory_unit - end - package2 = pack do |package| - package.add build_inventory_unit - end - - packages = [package1, package2] - prioritizer = Prioritizer.new(inventory_units, packages) - packages = prioritizer.prioritized_packages - expect(packages.size).to eq 2 - end - - it '1st has some, 2nd has remaining' do - 5.times { build_inventory_unit } - - package1 = pack do |package| - 2.times { |i| package.add inventory_units[i] } - end - package2 = pack do |package| - 5.times { |i| package.add inventory_units[i] } - end - - packages = [package1, package2] - prioritizer = Prioritizer.new(inventory_units, packages) - packages = prioritizer.prioritized_packages - expect(packages.count).to eq 2 - expect(packages[0].quantity).to eq 2 - expect(packages[1].quantity).to eq 3 - end - - it '1st has backorder, 2nd has some' do - 5.times { build_inventory_unit } - - package1 = pack do |package| - 5.times { |i| package.add inventory_units[i], :backordered } - end - package2 = pack do |package| - 2.times { |i| package.add inventory_units[i] } - end - - packages = [package1, package2] - prioritizer = Prioritizer.new(inventory_units, packages) - packages = prioritizer.prioritized_packages - - expect(packages[0].quantity(:backordered)).to eq 3 - expect(packages[1].quantity(:on_hand)).to eq 2 - end - - it '1st has backorder, 2nd has all' do - 5.times { build_inventory_unit } - - package1 = pack do |package| - 3.times { |i| package.add inventory_units[i], :backordered } - end - package2 = pack do |package| - 5.times { |i| package.add inventory_units[i] } - end - - packages = [package1, package2] - prioritizer = Prioritizer.new(inventory_units, packages) - packages = prioritizer.prioritized_packages - expect(packages[0]).to eq package2 - expect(packages[1]).to be_nil - expect(packages[0].quantity(:backordered)).to eq 0 - expect(packages[0].quantity(:on_hand)).to eq 5 - end - end - end -end diff --git a/core/spec/models/spree/stock/coordinator_spec.rb b/core/spec/models/spree/stock/simple_coordinator_spec.rb similarity index 80% rename from core/spec/models/spree/stock/coordinator_spec.rb rename to core/spec/models/spree/stock/simple_coordinator_spec.rb index ed47d72aa86..b20a1a09116 100644 --- a/core/spec/models/spree/stock/coordinator_spec.rb +++ b/core/spec/models/spree/stock/simple_coordinator_spec.rb @@ -2,21 +2,12 @@ module Spree module Stock - describe Coordinator, type: :model do + describe SimpleCoordinator, type: :model do let(:order) { create(:order_with_line_items, line_items_count: 2) } - subject { Coordinator.new(order) } + subject { SimpleCoordinator.new(order) } describe "#shipments" do - it "builds, prioritizes and estimates" do - expect(subject).to receive(:build_location_configured_packages).ordered.and_call_original - expect(subject).to receive(:build_packages).ordered.and_call_original - expect(subject).to receive(:prioritize_packages).ordered.and_call_original - expect(subject).to receive(:estimate_packages).ordered.and_call_original - expect(subject).to receive(:validate_packages).ordered.and_call_original - subject.shipments - end - it 'uses the pluggable estimator class' do expect(Spree::Config.stock).to receive(:estimator_class).and_call_original subject.shipments @@ -173,7 +164,6 @@ module Stock context "with sufficient inventory only across both locations" do let(:location_1_inventory) { 2 } let(:location_2_inventory) { 3 } - before { pending "This is broken. The coordinator packages this incorrectly" } it_behaves_like "a fulfillable package" end @@ -214,28 +204,57 @@ module Stock context "and insufficient inventory" do let(:location_1_inventory) { 0 } let(:location_2_inventory) { 3 } - before { pending "This is broken. The coordinator packages this incorrectly" } it_behaves_like "an unfulfillable package" end end end - end - - context "build location configured packages" do - context "there are configured stock locations" do - let!(:stock_location) { order.variants.first.stock_locations.first } - let!(:stock_location_2) { create(:stock_location) } + context 'with three stock locations' do + let!(:stock_location_2) { create(:stock_location, propagate_all_variants: false, active: true) } + let!(:stock_location_3) { create(:stock_location, propagate_all_variants: false, active: true) } before do - line_item_1 = order.line_items.first - line_item_2 = order.line_items.last - order.order_stock_locations.create(stock_location_id: stock_location.id, quantity: line_item_1.quantity, variant_id: line_item_1.variant_id) - order.order_stock_locations.create(stock_location_id: stock_location_2.id, quantity: line_item_2.quantity, variant_id: line_item_2.variant_id) + stock_item2 = variant.stock_items.create!(stock_location: stock_location_2, backorderable: false) + stock_item2.set_count_on_hand(location_2_inventory) + + stock_item3 = variant.stock_items.create!(stock_location: stock_location_3, backorderable: false) + stock_item3.set_count_on_hand(location_3_inventory) + end + + # Regression test for https://github.com/solidusio/solidus/issues/2122 + context "with sufficient inventory in first two locations" do + let(:location_1_inventory) { 3 } + let(:location_2_inventory) { 3 } + let(:location_3_inventory) { 3 } + + it_behaves_like "a fulfillable package" + + it "creates only two packages" do + expect(shipments.count).to eq(2) + end end - it "builds a shipment for each associated stock location" do - shipments = subject.shipments - expect(shipments.map(&:stock_location)).to match_array([stock_location, stock_location_2]) + context "with sufficient inventory only across all three locations" do + let(:location_1_inventory) { 2 } + let(:location_2_inventory) { 2 } + let(:location_3_inventory) { 2 } + + it_behaves_like "a fulfillable package" + + it "creates three packages" do + expect(shipments.count).to eq(3) + end + end + + context "with sufficient inventory only across all three locations" do + let(:location_1_inventory) { 2 } + let(:location_2_inventory) { 2 } + let(:location_3_inventory) { 2 } + + it_behaves_like "a fulfillable package" + + it "creates three packages" do + expect(shipments.count).to eq(3) + end end end end diff --git a/core/spec/models/spree/stock/splitter/base_spec.rb b/core/spec/models/spree/stock/splitter/base_spec.rb index d51b6db8e3c..3132da915f4 100644 --- a/core/spec/models/spree/stock/splitter/base_spec.rb +++ b/core/spec/models/spree/stock/splitter/base_spec.rb @@ -4,7 +4,6 @@ module Spree module Stock module Splitter describe Base, type: :model do - let(:packer) { build(:stock_packer) } let(:stock_location) { mock_model(Spree::StockLocation) } it 'continues to splitter chain' do @@ -15,12 +14,6 @@ module Splitter expect(splitter1).to receive(:split).with(packages) splitter2.split(packages) end - - it 'accepts a packer (deprecated)' do - splitter = Spree::Deprecation.silence { Base.new(packer) } - - expect(splitter.stock_location).to eq(packer.stock_location) - end end end end diff --git a/core/spec/models/spree/stock_quantities_spec.rb b/core/spec/models/spree/stock_quantities_spec.rb new file mode 100644 index 00000000000..c103b4e3469 --- /dev/null +++ b/core/spec/models/spree/stock_quantities_spec.rb @@ -0,0 +1,247 @@ +require 'rails_helper' + +describe Spree::StockQuantities, type: :model do + let(:variant1) { mock_model(Spree::Variant) } + let(:variant2) { mock_model(Spree::Variant) } + + subject do + described_class.new(quantities) + end + + describe "#each" do + def expect_each + expect { |b| subject.each(&b) } + end + + context "with no items" do + let(:quantities) { {} } + + it "doesn't yield" do + expect_each.not_to yield_control + end + end + + context "with one item" do + let(:quantities) { { variant1 => 2 } } + + it "yields values" do + expect_each.to yield_with_args([variant1, 2]) + end + end + + context "with two items" do + let(:quantities) { { variant1 => 2, variant2 => 3 } } + + it "yields values" do + expect_each.to yield_successive_args([variant1, 2], [variant2, 3]) + end + end + end + + describe "#variants" do + context "with one item" do + let(:quantities) { { variant1 => 2 } } + + it "returns variant" do + expect(subject.variants).to eq [variant1] + end + end + + context "with two items" do + let(:quantities) { { variant1 => 2, variant2 => 3 } } + + it "returns both variants" do + expect(subject.variants).to eq [variant1, variant2] + end + end + end + + describe "#empty?" do + context "no variants" do + let(:quantities) { {} } + it { is_expected.to be_empty } + end + + context "only quantity 0" do + let(:quantities) { { variant1 => 0 } } + it { is_expected.to be_empty } + end + + context "positive quantity" do + let(:quantities) { { variant1 => 1 } } + it { is_expected.not_to be_empty } + end + + context "one variant positive one zero" do + let(:quantities) { { variant1 => 1, variant2 => 0 } } + it { is_expected.not_to be_empty } + end + + context "negative quantity" do + # empty? doesn't make a whole lot of sense in this case, but returning + # false is probably more accurate. + let(:quantities) { { variant1 => -1 } } + it { is_expected.not_to be_empty } + end + end + + describe "==" do + subject do + described_class.new(quantity1) == described_class.new(quantity2) + end + + context "when both empty" do + let(:quantity1) { {} } + let(:quantity2) { {} } + it { is_expected.to be true } + end + + context "when both equal" do + let(:quantity1) { { variant1 => 1 } } + let(:quantity2) { { variant1 => 1 } } + it { is_expected.to be true } + end + + context "with different order" do + let(:quantity1) { { variant1 => 1, variant2 => 2 } } + let(:quantity2) { { variant2 => 2, variant1 => 1 } } + it { is_expected.to be true } + end + + context "with different variant" do + let(:quantity1) { { variant1 => 1 } } + let(:quantity2) { { variant2 => 1 } } + it { is_expected.to be false } + end + + context "with different quantities" do + let(:quantity1) { { variant1 => 1 } } + let(:quantity2) { { variant1 => 2 } } + it { is_expected.to be false } + end + + context "nil != 0" do + let(:quantity1) { { variant1 => 0 } } + let(:quantity2) { {} } + it { is_expected.to be false } + end + end + + describe "+" do + subject do + described_class.new(quantity1) + described_class.new(quantity2) + end + + context "same variant" do + let(:quantity1) { { variant1 => 20 } } + let(:quantity2) { { variant1 => 22 } } + + it { is_expected.to eq described_class.new({ variant1 => 42 }) } + end + + context "different variants" do + let(:quantity1) { { variant1 => 1 } } + let(:quantity2) { { variant2 => 2 } } + + it { is_expected.to eq described_class.new({ variant1 => 1, variant2 => 2 }) } + end + + context "0 quantities" do + let(:quantity1) { { variant1 => 0 } } + let(:quantity2) { { variant2 => 1 } } + + it { is_expected.to eq described_class.new({ variant1 => 0, variant2 => 1 }) } + end + + context "empty quantity" do + let(:quantity1) { { variant1 => 1 } } + let(:quantity2) { {} } + + it { is_expected.to eq described_class.new({ variant1 => 1 }) } + end + end + + describe "-" do + subject do + described_class.new(quantity1) - described_class.new(quantity2) + end + + context "same variant" do + let(:quantity1) { { variant1 => 22 } } + let(:quantity2) { { variant1 => 20 } } + + it { is_expected.to eq described_class.new({ variant1 => 2 }) } + end + + context "different variants" do + let(:quantity1) { { variant1 => 1 } } + let(:quantity2) { { variant2 => 2 } } + + it { is_expected.to eq described_class.new({ variant1 => 1, variant2 => -2 }) } + end + + context "0 quantity" do + let(:quantity1) { { variant1 => 0 } } + let(:quantity2) { { variant1 => 1 } } + + it { is_expected.to eq described_class.new({ variant1 => -1 }) } + end + + context "empty quantity RHS" do + let(:quantity1) { { variant1 => 1 } } + let(:quantity2) { {} } + + it { is_expected.to eq described_class.new({ variant1 => 1 }) } + end + + context "empty quantity LHS" do + let(:quantity1) { {} } + let(:quantity2) { { variant1 => 1 } } + + it { is_expected.to eq described_class.new({ variant1 => -1 }) } + end + end + + # Common subset + describe "&" do + subject do + described_class.new(quantity1) & described_class.new(quantity2) + end + + context "same variant" do + let(:quantity1) { { variant1 => 20 } } + let(:quantity2) { { variant1 => 22 } } + + it { is_expected.to eq described_class.new({ variant1 => 20 }) } + end + + context "multiple variants" do + let(:quantity1) { { variant1 => 10, variant2 => 20 } } + let(:quantity2) { { variant1 => 12, variant2 => 14 } } + + it { is_expected.to eq described_class.new({ variant1 => 10, variant2 => 14 }) } + end + + context "different variants" do + let(:quantity1) { { variant1 => 1 } } + let(:quantity2) { { variant2 => 2 } } + + it { is_expected.to be_empty } + it { is_expected.to eq described_class.new({}) } + end + + context "0 quantities" do + let(:quantity1) { { variant1 => 0 } } + let(:quantity2) { { variant1 => 1 } } + + it { is_expected.to eq described_class.new({ variant1 => 0 }) } + end + + context "empty quantity" do + let(:quantity1) { { variant1 => 1 } } + let(:quantity2) { {} } + + it { is_expected.to eq described_class.new({}) } + end + end +end