[Bye bye Spree] Bring models order, line_item and other related from spree_core

[luisramos0]

What? Why?

Also included in this PR models: order_contents, inventory_unit, order_inventory, return_authorization, state_changes and tokenized_permission.
The best part of this PR is the test coverage, the order model is actually the best covered model in the Spree codebase 👍

Continues #4826
Here we move the following files to OFN:

• core/app/models/spree/inventory_unit.rb COPY
• core/app/models/spree/line_item.rb MERGE WITH DECORATOR
• core/app/models/spree/order_contents.rb COPY
• core/app/models/spree/order_inventory.rb COPY
• core/app/models/spree/order.rb MERGE WITH DECORATOR
• core/app/models/spree/return_authorization.rb COPY
• core/app/models/spree/state_change.rb COPY
• core/app/models/spree/tokenized_permission.rb COPY
• spec/models/spree/order_contents_spec.rb COPY
• spec/models/spree/order_inventory_spec.rb COPY
• spec/models/spree/order_spec.rb MERGE WITH OFN VERSION
• spec/models/spree/inventory_unit_spec.rb COPY
• spec/models/spree/line_item_spec.rb MERGE WITH OFN VERSION
• spec/models/spree/return_authorization_spec.rb COPY
• core/spec/models/spree/order/address_spec.rb MAYBE COPY
• core/spec/models/spree/order/adjustments_spec.rb MAYBE COPY
• core/spec/models/spree/order/callbacks_spec.rb COPY
• core/spec/models/spree/order/payment_spec.rb MAYBE COPY
• core/spec/models/spree/order/state_machine_spec.rb MAYBE COPY
• core/spec/models/spree/order/tax_spec.rb MAYBE COPY
• core/spec/models/spree/order/updating_spec.rb MAYBE COPY

No code changes were made, here we only move the code from spree, adapt the specs if needed and improve the code a little with rubocop and transpec.

What should we test?

We need to verify if everything related to orders and line_items (yes, everything basically) in OFN is working well.
This is a little heart surgery to OFN. We do need to trust the build here and do some good verifications:

Release notes

Changelog Category: Changed
Brought code needed in OFN from Spree so that we can make OFN independent of Spree.

[sauloperez]

O M G. I'd like to go again through the merge of the decorators because there's a lot going on but it's looking good. There's so much 💩 🙈

[luisramos0]

yes 👍 I basically copy pasted things here but this commit does deserve a detailed review by someone going through the original decorator and spree file and check every change here.

Basically, this: https://github.com/openfoodfoundation/openfoodnetwork/blob/master/app/models/spree/order_decorator.rb
on top of this: https://github.com/openfoodfoundation/spree/blob/2-1-0-stable/core/app/models/spree/order.rb

There are some other good candidates but this may be the best commit in OFN history.

[luisramos0]

rebased to resolve conflicts.

[sauloperez]

The only bit from the decorator I couldn't find is

openfoodnetwork/app/models/spree/order_decorator.rb

Lines 32 to 40 in 427d1aa

	# The EmailValidator introduced in Spree 2.1 is not working
	# So here we remove it and re-introduce the regexp validation rule from Spree 2.0
	_validate_callbacks.each do |callback|
	if callback.raw_filter.respond_to? :attributes
	callback.raw_filter.attributes.delete :email
	end
	end
	validates :email, presence: true, format: /\A([\w\.%\+\-']+)@([\w\-]+\.)+([\w]{2,})\z/i,
	if: :require_email

. Where did that go?

[luisramos0]

That email related code was a workaround to disable the "new" default spree email validator. We no longer get it from spree and so we dont need it. We just need our simple validator 👍

[sauloperez]

👏 I'd be happy to approve it on Code Climate too. There are a few styling related issues in there that can wait IMO. It'd be too much for this PR.

[sauloperez]

I made the call and approved it in Code Climate so we merge this ASAP.

[filipefurtad0]

Allright, let's test the "OFN heart surgery" :-)
I'm happy for any advice, so before I start here I'd just ask if there is something specific to test - it looks like you were about to add something on the "what should we test" section above @luisramos0? Thank you!

[luisramos0]

Hello Filipe. There's nothing specific about this PR. It's changing the files (although the code should be exactly the same) of the orders so it is really touching everything related to orders and stock levels.

I can mention a few things:

• order stock at cart level - add and remove items and see stock limits are respected. This can be done with and without overrides.
• verify stock levels are respected while editing orders in the backoffice - one edge case we can verify is to cancel an order and verify the stock is returned
• return authorizations - we should verify that these are working as before (to be honest I don't know if this is working properly before this PR)
• testing a fee with a weight calculator would be good and verify the final weight volume in the BOM works as expected
• we need some sanity checks in reports
  These are just some ideas. I am sure you will find other good test cases to run here 👍

[filipefurtad0]

Hey @luisramos0,

As a first approach and as discussed, I followed what you suggested for test cases. I staged your PR in staging-ES and master branch on staging-UK: I found this to be the best way to compare my observations on each test case. So, if not mentioned otherwise, everything discussed below was confirmed as well in the latest release.

To summarize: so far, I found no differences whatsoever in behavior, between the two builds 🎉

Let's see the test cases in more detail

Stock Levels

1. order stock at cart level - add and remove items and see stock limits are respected. This can be done with and without overrides.

Proceeded as follows:

i) admin: set a limited stock level for the supplier

customer:
ii) attempt to add more items to the cart than available -> modal displays stock limit
ii) added items to cart (below or equal to stock) -> stock levels do not change
iii) proceeded to checkout -> stock levels do not change
iv) checks out -> stock levels are reduced as expected

Repeated this after overriding the variant (i.e., after adding item to inventory and overriding stock level settings). Works as well.

Stock Levels / BO
2) verify stock levels are respected while editing orders in the backoffice - one edge case we can verify is to cancel an order and verify the stock is returned

Cancelling orders in the backoffice restores stock, to the inventory stock (if a variant override is set) or to the stock set by the producer (if no override is set).

While doing this, bumped into this issue of being able to place orders with stock levels higher than available. I'm pretty certain to have observed this bug before but I can't seem to find the issue. #5784 is related. Anyway, I found out, that the BO allows for values above the stock-set limit, which in turn may lead to these these scenarios - see below - in which the invoice displays a different number of items and price charged than the actual order:

Basically, to reproduce this, one needs to set the max number of items available in the order, and then, edit the backoffice-cart, one by one. I guess this deserves an issue. Updated: this is the issue #5989.

Return Authorizations
3) we should verify that these are working as before

Tested return authorizations in the current v.3.2.10 - and it works! Also checked it on this PR, and found the same behavior: returning X items, gives back X items back to the stock.

However, I think I may have found a pre-existing bug here: if overrides are set for the returned product, then, the returned items go to the producer stock, and not to the inventory. This is not the case for cancelled orders, nor for backoffice order adjustments, in which the returned stock (be it due to cancellation or order adjustment) gets added back to the producer stock only if the variant is not overridden. Is this the expected behavior? I found nothing on this topic in the user guide. This might be something to as tomorrow at instance-managers.

Fees and BOM
4) testing a fee with a weight calculator would be good and verify the final weight volume in the BOM works as expected

This is a really interesting test case. To approach this, I placed an order with a previously set enterprise fee (per weight), and reduced the weight of the order (instead of lowering the number of items) in the bulk order management settings. This seems to work well. Here is the result, if one edits the order again after this:

Reports
5) we need some sanity checks in reports

I verified all reports are displayed/rendered correctly. I took a particular look at order related ones, and found no issues. I took particular attention to whether newly placed orders are appearing correctly.

Subscriptions
6) this is a particular type of order, and as such, I felt is should be checked, at least a basic sanity check. Since we had troubles with the sync/proxy orders lately, I opted for checking this manually instead of triggering the proxy order in the rails console.

Found no issues. The order was placed, the payment was processed, all (4) expected emails arrived. All good here.

Order-Related S3 Issues on the Pipe - only checked after staging this PR
7)

openfoodfoundation/wishlist#335 - reproduced
#4203 - could not reproduce as it is currently not possible to delete products from product list #5947 (reproduced)
#5505 - reproduced
#4186 - could not reproduce, due to the workaround introduced by PR#5253
#4220 - could not reproduce this
#4406 - could not reproduce this, seems to work fine? Will need to have a second look at this one.
#4582 - reproduced
#5236 - production test, did not reproduce
#5367 - reproduced
#5500 - did not test this one
#5708 - reproduced
#6029 - reproduced, related to #5367. I think the title may be need to be changed to "deactivates" instead of "deletes". Deleting (used) shipping methods is currently not possible #6135 (reproduced)
#6101 - reproduced

So, in summary, I found no difference in behavior between this PR and the current master. I think this heart surgery went very well :-D Looks ready to go!!

[luisramos0]

Amazing work Filipe!! Specially the part where you try to reproduce all related bugs!

[sauloperez]

This is definitely happening! if the order model is in our repo we're nearly there 🎉 !