Refactor stock related service objects to deal with inventory units.

Prior to this change, stock related service objects (coordinator, packer,
prioritizer, package, adjuster) were tightly coupled to orders and line
items, such that they would only create a set of shipments for the set
of line items for an order.

This change makes those objects work with inventory units, which enables
future enhancements that would create shipments for an order that are
not necessarily tied to the initial purchase (e.g. upcoming exchanges
work).

Fixes #5042

core/CHANGELOG.md

@@ -14,3 +14,9 @@
   relying on Spree's default behaviour. Fixes #5036
 
     Gregor MacDougall
+
+* Refactored Stock::Coordinator to optionally accept a list of inventory units 
+  for an order so that shipments can be created for an order that do not comprise
+  only of the order's line items.
+
+     Andrew Thal

core/app/models/spree/order.rb

@@ -489,13 +489,7 @@ def shipped?
     def create_proposed_shipments
       adjustments.shipping.delete_all
       shipments.destroy_all
-
-      packages = Spree::Stock::Coordinator.new(self).packages
-      packages.each do |package|
-        shipments << package.to_shipment
-      end
-
-      shipments
+      self.shipments = Spree::Stock::Coordinator.new(self).shipments
     end
 
     def apply_free_shipping_promotions
@@ -666,5 +660,6 @@ def create_token
           break random_token unless self.class.exists?(guest_token: random_token)
         end
       end
+
   end
 end

core/app/models/spree/shipment.rb

@@ -281,13 +281,9 @@ def tax_total
     end
 
     def to_package
-      package = Stock::Package.new(stock_location, order)
-      grouped_inventory_units = inventory_units.includes(:line_item).group_by do |iu|
-        [iu.line_item, iu.state_name]
-      end
-
-      grouped_inventory_units.each do |(line_item, state_name), inventory_units|
-        package.add line_item, inventory_units.count, state_name
+      package = Stock::Package.new(stock_location)
+      inventory_units.group_by(&:state).each do |state, state_inventory_units|
+        package.add_multiple state_inventory_units, state.to_sym
       end
       package
     end

core/app/models/spree/shipping_calculator.rb

@@ -15,7 +15,7 @@ def available?(package)
 
     private
     def total(content_items)
-      content_items.sum { |item| item.quantity * item.variant.price }
+      content_items.map(&:amount).sum
     end
   end
 end

core/app/models/spree/stock/adjuster.rb

@@ -3,25 +3,24 @@
 module Spree
   module Stock
     class Adjuster
-      attr_accessor :variant, :need, :status
+      attr_accessor :inventory_unit, :status, :fulfilled
 
-      def initialize(variant, quantity, status)
-        @variant = variant
-        @need = quantity
+      def initialize(inventory_unit, status)
+        @inventory_unit = inventory_unit
         @status = status
+        @fulfilled = false
       end
 
-      def adjust(item)
-        if item.quantity >= need
-          item.quantity = need
-          @need = 0
-        elsif item.quantity < need
-          @need -= item.quantity
+      def adjust(package)
+        if fulfilled?
+          package.remove(inventory_unit)
+        else
+          self.fulfilled = true
         end
       end
 
       def fulfilled?
-        @need == 0
+        fulfilled
       end
     end
   end

core/app/models/spree/stock/content_item.rb

@@ -0,0 +1,48 @@
+module Spree
+  module Stock
+    class ContentItem
+      attr_accessor :inventory_unit, :state
+
+      def initialize(inventory_unit, state = :on_hand)
+        @inventory_unit = inventory_unit
+        @state = state
+      end
+
+      def variant
+        inventory_unit.variant
+      end
+
+      def weight
+        variant.weight * quantity
+      end
+
+      def line_item
+        inventory_unit.line_item
+      end
+
+      def on_hand?
+        state.to_s == "on_hand"
+      end
+
+      def backordered?
+        state.to_s == "backordered"
+      end
+
+      def price
+        variant.price
+      end
+
+      def amount
+        price * quantity
+      end
+
+      def quantity
+        # Since inventory units don't have a quantity,
+        # make this always 1 for now, leaving ourselves
+        # open to a different possibility in the future,
+        # but this massively simplifies things for now
+        1
+      end
+    end
+  end
+end

core/app/models/spree/stock/coordinator.rb

@@ -1,10 +1,17 @@
 module Spree
   module Stock
     class Coordinator
-      attr_reader :order
+      attr_reader :order, :inventory_units
 
-      def initialize(order)
+      def initialize(order, inventory_units = nil)
         @order = order
+        @inventory_units = inventory_units || InventoryUnitBuilder.new(order).units
+      end
+
+      def shipments
+        packages.map do |package|
+          package.to_shipment.tap { |s| s.address = order.ship_address }
+        end
       end
 
       def packages
@@ -24,17 +31,17 @@ def packages
       # Returns an array of Package instances
       def build_packages(packages = Array.new)
         StockLocation.active.each do |stock_location|
-          next unless stock_location.stock_items.where(:variant_id => order.line_items.pluck(:variant_id)).exists?
+          next unless stock_location.stock_items.where(:variant_id => inventory_units.map(&:variant_id)).exists?
 
-          packer = build_packer(stock_location, order)
+          packer = build_packer(stock_location, inventory_units)
           packages += packer.packages
         end
         packages
       end
 
       private
       def prioritize_packages(packages)
-        prioritizer = Prioritizer.new(order, packages)
+        prioritizer = Prioritizer.new(inventory_units, packages)
         prioritizer.prioritized_packages
       end
 
@@ -46,8 +53,8 @@ def estimate_packages(packages)
         packages
       end
 
-      def build_packer(stock_location, order)
-        Packer.new(stock_location, order, splitters(stock_location))
+      def build_packer(stock_location, inventory_units)
+        Packer.new(stock_location, inventory_units, splitters(stock_location))
       end
 
       def splitters(stock_location)

core/app/models/spree/stock/estimator.rb

@@ -35,7 +35,7 @@ def calculate_shipping_rates(package)
               # If the rate's zone matches the order's zone, a positive adjustment will be applied.
               # If the rate is from the default tax zone, then a negative adjustment will be applied.
               # See the tests in shipping_rate_spec.rb for an example of this.d
-              rate.zone == package.order.tax_zone || rate.zone.default_tax?
+              rate.zone == order.tax_zone || rate.zone.default_tax?
             end
           end
 

core/app/models/spree/stock/inventory_unit_builder.rb

@@ -0,0 +1,21 @@
+module Spree
+  module Stock
+    class InventoryUnitBuilder
+      def initialize(order)
+        @order = order
+      end
+
+      def units
+        @order.line_items.flat_map do |line_item|
+          line_item.quantity.times.map do |i|
+            InventoryUnit.new(
+              pending: true,
+              variant: line_item.variant,
+              line_item: line_item
+            )
+          end
+        end
+      end
+    end
+  end
+end

core/app/models/spree/stock/package.rb

@@ -1,53 +1,50 @@
 module Spree
   module Stock
     class Package
-      ContentItem = Struct.new(:line_item, :variant, :quantity, :state)
-
-      attr_reader :stock_location, :order, :contents
+      attr_reader :stock_location, :contents
       attr_accessor :shipping_rates
 
-      def initialize(stock_location, order, contents=[])
+      def initialize(stock_location, contents=[])
         @stock_location = stock_location
-        @order = order
         @contents = contents
         @shipping_rates = Array.new
       end
 
-      def add(line_item, quantity, state = :on_hand, variant = nil)
-        contents << ContentItem.new(line_item, variant || line_item.variant, quantity, state)
+      def add(inventory_unit, state = :on_hand)
+        contents << ContentItem.new(inventory_unit, state) unless find_item(inventory_unit)
+      end
+
+      def add_multiple(inventory_units, state = :on_hand)
+        inventory_units.each { |inventory_unit| add(inventory_unit, state) }
+      end
+
+      def remove(inventory_unit)
+        item = find_item(inventory_unit)
+        @contents -= [item] if item
       end
 
       def weight
-        contents.sum { |item| item.variant.weight * item.quantity }
+        contents.sum(&:weight)
       end
 
       def on_hand
-        contents.select { |item| item.state == :on_hand }
+        contents.select(&:on_hand?)
       end
 
       def backordered
-        contents.select { |item| item.state == :backordered }
+        contents.select(&:backordered?)
       end
 
-      # Consider extensions and applications might create a inventory unit
-      # where the variant and the line_item might not refer to the same product
-      def find_item(variant, state = :on_hand, line_item = nil)
-        contents.select do |item|
-          item.variant == variant &&
-          item.state == state &&
-          (line_item.nil? || line_item == item.line_item)
-        end.first
+      def find_item(inventory_unit, state = nil)
+        contents.detect do |item|
+          item.inventory_unit == inventory_unit &&
+            (!state || item.state.to_s == state.to_s)
+        end
       end
 
-      def quantity(state=nil)
-        case state
-        when :on_hand
-          on_hand.sum { |item| item.quantity }
-        when :backordered
-          backordered.sum { |item| item.quantity }
-        else
-          contents.sum { |item| item.quantity }
-        end
+      def quantity(state = nil)
+        matched_contents = state.nil? ? contents : contents.select { |c| c.state.to_s == state.to_s }
+        matched_contents.map(&:quantity).sum
       end
 
       def empty?
@@ -67,32 +64,22 @@ def shipping_methods
       end
 
       def inspect
-        out = "#{order} - "
-        out << contents.map do |content_item|
-          "#{content_item.variant.name} #{content_item.quantity} #{content_item.state}"
-        end.join('/')
-        out
+        contents.map do |content_item|
+          "#{content_item.variant.name} #{content_item.state}"
+        end.join(' / ')
       end
 
       def to_shipment
-        shipment = Spree::Shipment.new
-        shipment.address = order.ship_address
-        shipment.order = order
-        shipment.stock_location = stock_location
-        shipment.shipping_rates = shipping_rates
-
-        contents.each do |item|
-          item.quantity.times do |n|
-            unit = shipment.inventory_units.build
-            unit.pending = true
-            unit.order = order
-            unit.variant = item.variant
-            unit.line_item = item.line_item
-            unit.state = item.state.to_s
-          end
-        end
-
-        shipment
+        # At this point we should only have one content item per inventory unit
+        # across the entire set of inventory units to be shipped, which has been
+        # taken care of by the Prioritizer
+        contents.each { |content_item| content_item.inventory_unit.state = content_item.state.to_s }
+
+        Spree::Shipment.new(
+          stock_location: stock_location,
+          shipping_rates: shipping_rates,
+          inventory_units: contents.map(&:inventory_unit)
+        )
       end
     end
   end

core/app/models/spree/stock/packer.rb

@@ -1,11 +1,11 @@
 module Spree
   module Stock
     class Packer
-      attr_reader :stock_location, :order, :splitters
+      attr_reader :stock_location, :inventory_units, :splitters
 
-      def initialize(stock_location, order, splitters=[Splitter::Base])
+      def initialize(stock_location, inventory_units, splitters=[Splitter::Base])
         @stock_location = stock_location
-        @order = order
+        @inventory_units = inventory_units
         @splitters = splitters
       end
 
@@ -18,17 +18,19 @@ def packages
       end
 
       def default_package
-        package = Package.new(stock_location, order)
-        order.line_items.each do |line_item|
-          if line_item.should_track_inventory?
-            next unless stock_location.stock_item(line_item.variant)
+        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?
+            next unless stock_location.stock_item(variant)
 
-            on_hand, backordered = stock_location.fill_status(line_item.variant, line_item.quantity)
-            package.add line_item, on_hand, :on_hand if on_hand > 0
-            package.add line_item, backordered, :backordered if backordered > 0
+            on_hand, backordered = stock_location.fill_status(variant, 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 line_item, line_item.quantity, :on_hand
+            package.add_multiple units
           end
+
         end
         package
       end