Skip to content

Commit

Permalink
Pass options to coordinator dependencies
Browse files Browse the repository at this point in the history
It's very useful that the simple_coordinator can have so many of
it's internal classes configured. However, they can only be configured
for one set of hardcoded behavior, based on the two input values, an
order and inventory_units

If you want the coordinator to behave differently in different scenarios
e.g, the admin and the frontend, then you have to start getting creative.
The simple_coordinator (and all it's configured classes) in their current
state can only react to the state of the order and inventory_units argument,
or they can react to globally set state (which is not a great pattern).

Currently, the simple_coordinator is only called in two places in Solidus:
during exchanges, and during creating proprosed shipments. However, it is
reasonable for a complicated store to want to build shipments in other scenarios.

One workaround to getting the coordinator to behave differently in these different
locations is to include any arguments that you want to pass to the coordinator on
the order as an attribute or database column, and then read the order attribute
in your configured custom class. However, this isn't even a perfect workaround,
because not every configurable class is passed the order (e.g. the
location_sorter_class). To truly have the coordinator behave differently in different
locations you need to do minor to extensive monkey patching

This solution addresses the problem by allowing generic options to be passed to the
simple coordinator, which are then passed down to each configurable class. This means
that any caller of the simple_coordinator can customize the behavior it wants through
these options and overriding the configurable inner classes.

This for example, allows for customizations like shipment building behavior that is
specific to the admin, where a admin user could be allowed to rebuild shipments with
a stock location that is not normally available to users.

This is however a **breaking change** for certain consumers of Solidus. Since this
change adds an argument to the constructor of the following classes, if a consumer of
solidus has a.) configured their own custom class and b.) overrode the constructor of their
own custom class then their custom class could break. However, this error will cause shipment
building to fail, so it should be very obvious to spot and correct. Additionally, there
were few reasons to override the constructor of these configurable classes unless you had also
overridden the simple_coordinator, as you did not have control of the arguments passed to these
configurable classes.

`inventory_unit_builder_class`
`location_filter_class`
`location_sorter_class`
`allocator_class`
`estimator_class`

e.g. a custom configured stock location filter like this would be broken

class StockLocationFilter < Spree::Stock::LocationFilter::Base
  def initialize(stock_locations, order)
    super(stock_locations, order)
    @some_variable = 'Some initializer'
  end

  ...
end

However, initializers like this will be fine, as they implicitly
pass the original arguments:

class StockLocationFilter < Spree::Stock::LocationFilter::Base
  def initialize(_stock_locations, order)
    super
    @my_variable = 'Some Value'
  end
  ...
end

Co-authored-by: Benjamin Willems <[email protected]>
  • Loading branch information
Noah-Silvera and benjaminwil committed Oct 16, 2024
1 parent c3fea3e commit cd5b4be
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 21 deletions.
5 changes: 3 additions & 2 deletions core/app/models/spree/stock/allocator/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ module Spree
module Stock
module Allocator
class Base
attr_reader :availability
attr_reader :availability, :coordinator_options

def initialize(availability)
def initialize(availability, coordinator_options: {})
@availability = availability
@coordinator_options = coordinator_options
end

def allocate_inventory(_desired)
Expand Down
4 changes: 4 additions & 0 deletions core/app/models/spree/stock/estimator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ class Estimator
class ShipmentRequired < StandardError; end
class OrderRequired < StandardError; end

def initialize(coordinator_options: {})
@coordinator_options = coordinator_options
end

# Estimate the shipping rates for a package.
#
# @param package [Spree::Stock::Package] the package to be shipped
Expand Down
3 changes: 2 additions & 1 deletion core/app/models/spree/stock/inventory_unit_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
module Spree
module Stock
class InventoryUnitBuilder
def initialize(order)
def initialize(order, coordinator_options: {})
@order = order
@coordinator_options = coordinator_options
end

def units
Expand Down
7 changes: 5 additions & 2 deletions core/app/models/spree/stock/location_filter/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Base
# @!attribute [r] stock_locations
# @return [Enumerable<Spree::StockLocation>]
# a collection of locations to sort
attr_reader :stock_locations
attr_reader :stock_locations, :coordinator_options

# @!attribute [r] order
# @return <Spree::Order>
Expand All @@ -25,9 +25,12 @@ class Base
# a collection of locations to sort
# @param order <Spree::Order>
# the order we are creating the shipment for
def initialize(stock_locations, order)
# @param coordinator_options [Hash]
# a set of options passed from the stock_coordinator
def initialize(stock_locations, order, coordinator_options: {})
@stock_locations = stock_locations
@order = order
@coordinator_options = coordinator_options
end

# Filter the stock locations.
Expand Down
7 changes: 5 additions & 2 deletions core/app/models/spree/stock/location_sorter/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@ class Base
# @!attribute [r] stock_locations
# @return [Enumerable<Spree::StockLocation>]
# a collection of locations to sort
attr_reader :stock_locations
attr_reader :stock_locations, :coordinator_options

# Initializes the stock location sorter.
#
# @param stock_locations [Enumerable<Spree::StockLocation>]
# a collection of locations to sort
def initialize(stock_locations)
# @param coordinator_options [Hash]
# a set of options passed from the stock_coordinator
def initialize(stock_locations, coordinator_options: {})
@stock_locations = stock_locations
@coordinator_options = coordinator_options
end

# Sorts the stock locations.
Expand Down
15 changes: 8 additions & 7 deletions core/app/models/spree/stock/simple_coordinator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,17 @@ class SimpleCoordinator
# @api private
attr_reader :inventory_units, :splitters, :stock_locations,
:filtered_stock_locations, :inventory_units_by_variant, :desired,
:availability, :allocator, :packages
:availability, :allocator, :packages, :coordinator_options

def initialize(order, inventory_units = nil)
def initialize(order, inventory_units = nil, coordinator_options: {})
@order = order
@coordinator_options = coordinator_options
@inventory_units =
inventory_units || Spree::Config.stock.inventory_unit_builder_class.new(order).units
inventory_units || Spree::Config.stock.inventory_unit_builder_class.new(order, coordinator_options:).units
@splitters = Spree::Config.environment.stock_splitters

@filtered_stock_locations = Spree::Config.stock.location_filter_class.new(load_stock_locations, order).filter
sorted_stock_locations = Spree::Config.stock.location_sorter_class.new(filtered_stock_locations).sort
@filtered_stock_locations = Spree::Config.stock.location_filter_class.new(load_stock_locations, order, coordinator_options:).filter
sorted_stock_locations = Spree::Config.stock.location_sorter_class.new(filtered_stock_locations, coordinator_options:).sort
@stock_locations = sorted_stock_locations

@inventory_units_by_variant = @inventory_units.group_by(&:variant)
Expand All @@ -44,7 +45,7 @@ def initialize(order, inventory_units = nil)
stock_locations: stock_locations
)

@allocator = Spree::Config.stock.allocator_class.new(availability)
@allocator = Spree::Config.stock.allocator_class.new(availability, coordinator_options:)
end

def shipments
Expand All @@ -69,7 +70,7 @@ def build_shipments
# 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.shipping_rates = Spree::Config.stock.estimator_class.new(coordinator_options:).shipping_rates(package)
shipment
end
end
Expand Down
16 changes: 9 additions & 7 deletions core/spec/models/spree/stock/simple_coordinator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,26 @@ module Stock

describe "#shipments" do
it 'uses the pluggable estimator class' do
expect(Spree::Config.stock).to receive(:estimator_class).and_call_original
expect(Spree::Config.stock.estimator_class).to receive(:new).with(coordinator_options: {}).and_call_original

subject.shipments
end

it 'uses the configured stock location filter' do
expect(Spree::Config.stock).to receive(:location_filter_class).and_call_original
expect(Spree::Config.stock.location_filter_class).to receive(:new).with(anything, anything, coordinator_options: {}).and_call_original

subject.shipments
end

it 'uses the configured stock location sorter' do
expect(Spree::Config.stock).to receive(:location_sorter_class).and_call_original
expect(Spree::Config.stock.location_sorter_class).to receive(:new).with(anything, coordinator_options: {}).and_call_original

subject.shipments
end

it 'uses the pluggable allocator class' do
expect(Spree::Config.stock).to receive(:allocator_class).and_call_original
expect(Spree::Config.stock.allocator_class).to receive(:new).with(anything, coordinator_options: {}).and_call_original

subject.shipments
end

Expand All @@ -39,9 +43,7 @@ module Stock
end

it 'uses the pluggable inventory unit builder class' do
expect(Spree::Config.stock)
.to receive(:inventory_unit_builder_class)
.and_call_original
expect(Spree::Config.stock.inventory_unit_builder_class).to receive(:new).with(anything, coordinator_options: {}).and_call_original

subject.shipments
end
Expand Down

0 comments on commit cd5b4be

Please sign in to comment.