Commit 94b7875
Pass options to coordinator dependencies
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]>1 parent e8c9675 commit 94b7875
File tree
7 files changed
+36
-21
lines changed- core
- app/models/spree/stock
- allocator
- location_filter
- location_sorter
- spec/models/spree/stock
7 files changed
+36
-21
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
| 7 | + | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
9 | 13 | | |
10 | 14 | | |
11 | 15 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
| 6 | + | |
7 | 7 | | |
| 8 | + | |
8 | 9 | | |
9 | 10 | | |
10 | 11 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
15 | | - | |
| 15 | + | |
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
28 | | - | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
29 | 31 | | |
30 | 32 | | |
| 33 | + | |
31 | 34 | | |
32 | 35 | | |
33 | 36 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
18 | | - | |
| 18 | + | |
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
25 | 27 | | |
| 28 | + | |
26 | 29 | | |
27 | 30 | | |
28 | 31 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
28 | | - | |
| 28 | + | |
29 | 29 | | |
30 | | - | |
| 30 | + | |
31 | 31 | | |
| 32 | + | |
32 | 33 | | |
33 | | - | |
| 34 | + | |
34 | 35 | | |
35 | 36 | | |
36 | | - | |
37 | | - | |
| 37 | + | |
| 38 | + | |
38 | 39 | | |
39 | 40 | | |
40 | 41 | | |
| |||
44 | 45 | | |
45 | 46 | | |
46 | 47 | | |
47 | | - | |
| 48 | + | |
48 | 49 | | |
49 | 50 | | |
50 | 51 | | |
| |||
69 | 70 | | |
70 | 71 | | |
71 | 72 | | |
72 | | - | |
| 73 | + | |
73 | 74 | | |
74 | 75 | | |
75 | 76 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
14 | | - | |
| 14 | + | |
| 15 | + | |
15 | 16 | | |
16 | 17 | | |
17 | 18 | | |
18 | 19 | | |
19 | | - | |
| 20 | + | |
| 21 | + | |
20 | 22 | | |
21 | 23 | | |
22 | 24 | | |
23 | 25 | | |
24 | | - | |
| 26 | + | |
| 27 | + | |
25 | 28 | | |
26 | 29 | | |
27 | 30 | | |
28 | 31 | | |
29 | | - | |
| 32 | + | |
| 33 | + | |
30 | 34 | | |
31 | 35 | | |
32 | 36 | | |
| |||
39 | 43 | | |
40 | 44 | | |
41 | 45 | | |
42 | | - | |
43 | | - | |
44 | | - | |
| 46 | + | |
45 | 47 | | |
46 | 48 | | |
47 | 49 | | |
| |||
0 commit comments