Reduce the surface area of Spree::Stock::Coordinator#950
Reduce the surface area of Spree::Stock::Coordinator#950jordan-brough wants to merge 2 commits intosolidusio:masterfrom
Conversation
Make 'shipments' the only public method of the Coordinator. It's the only method called on Coordinator outside of the specs. This was prompted by me looking at the Coordinator code while reviewing some of the taxes work and thinking it might be nice to refactor it a bit, which is easier with a reduced surface area.
| expect(packages.map(&:quantity).sum).to eq(5) | ||
| expect(packages.flat_map(&:contents).map(&:inventory_unit).uniq.size).to eq(5) | ||
| expect(shipments).not_to be_empty | ||
| inventory_units = shipments.flat_map { |s| s.inventory_units } |
There was a problem hiding this comment.
Pass &:inventory_units as an argument to flat_map instead of a block.
|
I like this. This would mean that the shipping rates estimation could be part of the |
|
I also like this but reckon it will need a changelog if it goes in, as we are technically immediately removing a method from the public API. I have such a poor understanding of what a "package" does I'd rather this be the only thing accessible by a co-ordinator, and agree it would give us more flexibility to (eventually) make some meaningful changes behind the scenes if we can get this in. |
|
I wouldn't mind 👍 |
|
Great! I added a changelog entry. |
…-area Reduce the surface area of Spree::Stock::Coordinator
|
Merged in f8b889a |
5 participants
Make 'shipments' the only public method of the Coordinator. It's the
only method called on Coordinator outside of the specs.
This was prompted by me looking at the Coordinator code while
reviewing some of the taxes work and thinking it might be nice to
refactor it a bit, which is easier with a reduced surface area.