Openfoodnetwork: [Spree Upgrade] Spree 2.0 removes on_demand property from Product and Variant

[Matt-Yorkley]

Description

on_demand is removed from Product and Variant in these 2 PRs for Spree 2.0:

https://github.com/spree/spree/commit/40665ab244277138801ee278f663fd2f6da7da22#diff-c1dfa6db7daf5dcbf0bf6caad4024294
https://github.com/spree/spree/commit/14a398cd3625a1ef7173c1661d7013d51ea92446#diff-c1dfa6db7daf5dcbf0bf6caad4024294

There are 81 mentions of on_demand throughout our app folder.

I think our users regularly use this option for product stock.

Expected Behavior

Products can be set to on_demand.

Actual Behavior

It's gone in 2.0!

Animated Gif/Screenshot

Context

Spree Upgrade.

[Matt-Yorkley]

This will need a bit of a spike, but after some discussions there was a mention of a solution where we allow backorders on items. This will probably need to include looking at our current use of the allow_backorders config setting and the previous issues relating to it. I can't remember if it is just a large number of specs that depend on it being turned off, or whether or not some of our customisations make it unusable.

https://github.com/openfoodfoundation/openfoodnetwork/search?q=allow+backorders&type=Issues

[daniellemoorhead]

I assume this will be handled as part of the Spree Upgrade work @Matt-Yorkley?

[Matt-Yorkley]

Thanks @daniellemoorhead, this was just here to document it as a reminder. Yes it can be filed under "all the Spree upgrade things". I think it will be covered in the new plans to keep the existing API in terms of models/methods.

[luisramos0]

Useful conversation: https://github.com/spree/spree/commit/1ca45b826934d345c0e39dfebe9a707f649ea2ab
From what I have seen, it has not come back since then.
From this conversation here looks like we need to use backorders instead:
https://groups.google.com/forum/#!topic/spree-user/kUHJljlN2hQ

[myriamboure]

I don't really care about the use of backlorders in replacement of on_demand, but what I care for is the UX, as most users actually do check the box "on demand" on product as they don't manage stock levels within OFN.
My concern is that today backorder is setup at instance level, whereas every hub, for every product, should be able to decide if their customer can order more than stocked / order as much as they want without specified stock info. So the "allow backorder" info should be at product level. Is it the case?

[sauloperez]

that is now at stock item level, which is related to variants. In that case, it meets your expectations @myriamboure .

I agree we need to stick to our current UX as much as possible. Basically, keeping an on_demand checkbox that disables stock management for that product. Am I right?

[myriamboure]

Ok that sounds pretty good actually, pretty aligned with the work we have done on product chain model as well. I summed up how it works, change, and potential solution here https://community.openfoodnetwork.org/t/what-to-do-with-backorders-and-on-demand-after-spree-got-rid-of-on-demand/1445 let me know if that is logical and if we see other options.

[sauloperez]

For the record, Ux-wise when user sets product to on_demand, product master variant sets its stock item backordable to true #headache

[myriamboure]

No that's not a headache @sauloperez it's super consistent to me ! See my post on Discourse :-) All good I think, it's a pretty good move on Spree side IMO. Simplify how the app work. And consistent with where we want to move.

[luisramos0]

Currently "undefined method `allow_backorders" is the most common error in our 2-0-stable build, so it looks like this epic is the top priority in the Spree Upgrade ;-)

We need to go through the code and replace all appearances of Spree::Config.allow_backorders with some replacement like VariantStock.backorderable.... it looks like an epic, it sounds like an epic, it smells like epic.... it's an epic! :-)

[luisramos0]

I did some further tech analysis on this one, here's the result:

Spree 1 had two concepts: on_demand (at variant and product level) and allow_backorders (at instance level). Both disappeared in Spree 2.
In Spree 2 there’s only the concept of backorderable at stock_item level. Because in OFN2 we are forcing one variant to have only one stock_item, we can say that a variant will be backorderable or not according to its stock_item.backorderable value.
So, to do this, in OFN v2, we keep the concept ON_DEMAND in the code and implement a wrapper variant.on_demand that will depend on variant.stock_item.backorderable. This is already done here.
We keep on_demand in OFN2 code just to avoid having to replace all occurrences of on_demand (there are quite a few) with something like variant.backorderable?. It’s just a name and on_demand is good enough to represent the concept backorderable.

The only missing part is really what do we do with product.on_demand? Do we implement an adapter product.on_demand that looks at variant.on_demand or do we change the code that needs product.on_demand to use variant.on_demand? I am not clear about what’s the process to calculate stock on product and why a product becomes out of stock. Is a single variant on_demand enough to make a product never go out of stock? This challenge will be addressed in #2870

[luisramos0]

the disappearance of allow_backorders will use these wrappers but will be addressed in a separate epic: #2887

[luisramos0]

This has been addressed. Closing the issue.
We will review this in detail when we get to manual testing v2. Very soon :-D