Now that we managed to run the spec/models/spree/order_spec.rb on our spree 2.0 branch, it turns out that few changes in the Order model affect us.
shipping_method
Now orders have many shipments instead of a single shipping_method. However the shipping_method_id column hasn’t been removed from the spree_orders table, at least until Spree’s 3-0-stable. Check out spree/_order_details.html.erb at master · spree/spree · GitHub and Remove anything left of Order#ShippingMethod · spree/spree@1cc4680 · GitHub
This means that now the information of the shipment method lies in each of the particular shipments the order has.
shipping_address_from_distributor
#shipping_address_from_distributor makes use of ShippingMethod#require_ship_address. It turns out that is a column that we added to the spree_shipping_methods table 
20140213003443_add_require_ship_address_to_shipping_methods.rb.
Said column is populated when creating a new shipping method as an admin (choosing pick up sends it to false). When set, the shipping section of the checkout form shows the ship_address_same_as_billing and default_ship_address checkboxes.
Addressed in https://github.com/openfoodfoundation/openfoodnetwork/pull/1642
go_to_state :payment
Likewise, the checkout flow also checks whether the shipping method requires an address (using ShippingMethod#require_ship_address) and if it is set it creates a shipment in the order object.
Due to now having many shipments, spree removed Order#create_shipment!. It doesn’t make sense anymore. Checkout the commit for more details.
NOTE: why do we access the payment state (haven’t checked the others) 8 times? 
Tasks
These are the links to specific threads, GitHub issues and PRs required for the upgrade of the Order model:
1 Like
I found something a bit weird today. I placed two orders using the same address for shipment and billing and the spree_addresses table contains my address 6 times. When looking at the order record it contains different ids for bill_address_id and ship_address_id and these associated records have the same created_at (where created at almost the same time).
I’ve been digging through the ChecksoutController and the Order but I haven’t found anything.
Any clues @oeoeaio @RohanM ?
@sauloperez wow, that is crazy! No ideas from me. As you know, the Order model comes from Spree. It is a bit of a ‘black box’ for me, I don’t really understand all of the magic that happens when you create an order. I do know that creation of an order always takes a very long time, and there is a lot of repetition of callbacks, etc. My understanding is that the process of updating an order is changed to be a bit more sane somewhere between Spree 2.0 and 2.3ish, so that is another reason why I think the upgrade is very valuable.
@enricostn @sauloperez I love this approach of doing ‘model’ at a time and understanding the logic. I think it is going to really help us think through the implications of changes
just an observation from feature perspective on what you’ve said above. If a shipping method is now broken into shipments, then each order can have multiple shipments - which then happen on different days and times. We could then allow a customer to have an order that sits over multiple ‘order cycles’ from the same hub e.g. they just order once and their order is managed as ‘shipments’ allocated to an order cycle, rather than one order per order cycle. I don’t even know if we need / want this, and it touches on a bunch of other things. Just an observation 
Now we partially get it. Together with all the other order data, we send bill_address_attributes and ship_address_attributes although with exact same attributes. Then, ActiveRecord creates two different address records in the database as it can’t infer they both represent the same address.
With this, we wil now investigate how many times and why the checkout flow’s callbacks are executed.
Something we’ll need to consider in the future is how we will display in the UI that an order has multiple shipments.
While we can overcome in this in most of the views by iterating on the list of shipments, places like reports are not that straightforward. Do we show a visual mark in the table cell to note there are more than one? can this list be potentially large? do we list them separated by commas?
See an example: https://github.com/openfoodfoundation/openfoodnetwork/pull/1659/files#diff-0f6e8bba8a9b524117cc8bf10bbac623R97
It is not a blocker as we don’t really support multiple shipments as of today in OFN but it’ll be something to consider once we finish the step 7.
thoughts @oeoeaio @RohanM @Kirsten @Matt-Yorkley ?
1 Like
The only way an order can have multiple shipments currently is if an enterprise user creates shipments for an order in the backend. Customers cannot do anything that results in an order having multiple shipments. As a short term measure we could a) change the admin interface to prevent it from happening, or b) warn the user when they go to create multiple shipments for an order that they may not display in reports…
Or we could just fix the reports.
I am now archiving this old upgrade to spree 2 thread.
