Delete dead feature product distributions

[luisramos0]

What? Why?

Closes #3525
I think this is the perfect timing to do this, just before we get to v2.

Product Distributions were an alternative way to get products into the shop list and into the orders. Order Cycle is now mandatory for an order to go through the cart phase.

What should we test?

We need to carefully validate these areas of the app:

• create product
• bulk product edit page - changing product and variants details
• product edit page
  • there should be no product distributions sub menu now and all other sub menus should work normally
• not directly related to this PR but a question was raised about recalculating enterprise fees on orders where the products were removed from the order cycle: 1. create an order with OC fees in an OC, 2. remove product from OC, 3. edit order and recalculate fees.
• same as above but for closed order cycles: 1. create order with OC fees, 2. close the OC, 3- edit order and recalculate fees.
• reports that were slightly changed (removed the product distributions part of the queries) and should work correctly:
  • order_cycle_management, packing, sales_tax, bulk_coop, payments, xero_invoices, customers and products_and_inventory
• enterprise fees
  • add fee to order cycle and see the correct adjustment in the order
• shop front list of products with correct enterprise fees
• add variant to cart and checkout
  • we must test the case where user selects one distributor and order cycle, adds products to cart and then changes to another order cycle or distributor, checkout should still work

In the add to cart stage, there's this scenario where user adds a variant to the cart and then changes to another order cycle where this same variant is also available, from the code it seems this was possible in the past, I am not sure if this a valid user case right now. maybe we should check and decide.

Release notes

Changelog Category: Removed
Remove a feature that was not working: Product Distributions, the predecessor of order cycles.

How is this related to the Spree upgrade?

It's not related.

Dependencies

There will be a conflict with #3569.

[sauloperez]

This is massive @luisramos0 !! IMO is worth digging deeper on the test cases that must be tested. At least, the particular actions and methods that were refactored.

[sauloperez]

Have we checked if there any recently created product_distributions in production servers? in OFF I get

openfoodnetwork=> select * from product_distributions order by id desc limit 1;
 id | product_id | distributor_id |         created_at         |         updated_at         | enterprise_fee_id 
----+------------+----------------+----------------------------+----------------------------+-------------------
 44 |       4390 |            572 | 2019-02-07 10:19:57.291831 | 2019-02-07 10:19:57.291831 |                95

While from Katuma I get

 id | product_id | distributor_id |         created_at         |         updated_at         | enterprise_fee_id 
----+------------+----------------+----------------------------+----------------------------+-------------------
  5 |       1008 |            152 | 2019-01-29 17:14:23.109552 | 2019-01-29 17:14:23.109552 |                25

It could as well be that the feature is not really used by we still create these from callbacks without us noticing. Perhaps the client-side is still populating it through accepts_nested_attributes_for :product_distributions?

[luisramos0]

In terms of usage, I dont see any other explanation other than: users are using the product distributions page and selecting enterprises thinking it works, in some way.
The numbers in the databases are:
354 rows in UK, some entries from Oct 2018 from one single distributor id 200270.
173 entries in AUS, one entry from Nov 2018 and others before Mars 2018.
44 entries in FR, some recent entries from a specific user.

I think these are all entries created by users that think the page works. I am also saying this because I was one of theses users during a demo in Portugal... later on, looking at the code, I realized the page is useless...

[sauloperez]

That would be a feasible explanation. Just out of curiosity I'd check with Matomo what that user did in FR although we know the actions were useless.

[luisramos0]

I resolved conflicts and improved the "what to test" description as suggested by @sauloperez

[mkllnk]

This is such good work. Well done! It should speed up our tests and prevents us from wasting time with dead code. I got a couple of questions but I'm otherwise very happy with this. It will need some really thorough testing though.

Oh, and we should query all affected enterprises so that we can ask them if they are using it (and why).

[mkllnk]

I'm wondering if this method can go as well. Without product distributions, all items of an order should be in the order cycle of that order, right?

What else could happen? A product can be removed from an order cycle after it has been ordered. Would that cause any problem? Could it be that they disappear from a report?

[luisramos0]

yes, I think that is one case in which this is protecting against. Products removed from OCs.

I am not sure how the system behaves in terms of modifying orders with products that are no longer in the order cycle, or even modifying orders where the OC is already closed...
I'll add that to the test scenarios of this PR.

[mkllnk]

Did you run this against production data? Is there any dependent data destroyed?

[luisramos0]

I tried it now, it just works.

[luisramos0]

I tried with AUS data.

[luisramos0]

In terms of enterprises, in AUS live data there's only one enterprise using it and the entries are from 2013, I think we will be ok :-)
In UK and FR there are more recent entries.

• there are 15 enterprises in the UK. I have the list of enterprises and emails. @lin-d-hop would you be the right person to contact the users asking them if/why they use product distributions and alerting them it will be deleted from the system?
• in FR we have 11 enterprises, @RachL would you want to do this communication?
• in Katuma, there are 4 enterprises but only one of the enterprises with orders

[RachL]

@luisramos0 yes. Do you have the list?

[luisramos0]

yes, just sent you on slack.

[luisramos0]

rebased, build green, ready for testing!

[RachL]

@luisramos0 I've tested all the cases. It seems to me that enterprise fees are not working. Now please take into account that I'm kind of a fee newbie. I've asked Myriam for some detailed test cases but she didn't had the time to write them so far. So I'm sorry but we will have to look at this step by step :(
Note that so far I've only played with the "Flat percent" Calculator. However I've tried packing / transport / admin type of fees. I know that packing and transport should appear in the OC, while admin fees are only added at checkout. I've checked that I've added them both on incoming and outgoing... but still I could only see fees linked to payment method....

Do you see a step missing in my tests?

add variant to cart and checkout
we must test the case where user selects one distributor and order cycle, adds products to cart and then changes to another order cycle or distributor, checkout should still work

Also FYI I've added screenshots in my notes for this. The UI does not allow a user to shop from different shops or from different OC. Meaning 1 order = 1 OC = 1 distributor. It's a bit a pity if you think about it, but this can be dealt with as wishlist items. So you can 🔥 anything related to a different behaviour in the code.

Testing notes: https://docs.google.com/document/d/1m_nmQU4Xki5xlHAGeOf-gdbVqIMsTNLpkOT4Ual1zc8/edit#

[luisramos0]

Can you be a bit more specific in terms of fees not working?
I just went to uk staging, opened your test OC "manana in Rachel fee report test", added your test admin Fee to outgoing and I can see the prices changing in the shop front...

re 1 order = 1 OC = 1 distributor
👍

[luisramos0]

In terms of calculators, one important part is that "per order" fees are only seen in cart and checkout, not in the product list.

re " I know that packing and transport should appear in the OC, while admin fees are only added at checkout."
I scanned the code quickly to see how the different types of fees work (admin, transport, etc)...
I dont see any logic applied to these, are you sure about your statement?

and thanks a lot for testing this 🎉

[RachL]

@luisramos0

I scanned the code quickly to see how the different types of fees work (admin, transport, etc)...
I dont see any logic applied to these, are you sure about your statement?

Not at all. I saw this in an old testing scenario, but I can't find the PR, so let's say I was totally wrong on this.

Can you be a bit more specific in terms of fees not working? I just went to uk staging, opened your test OC "manana in Rachel fee report test", added your test admin Fee to outgoing and I can see the prices changing in the shop front...

😊 Could this be I forgot to add them in outgoing? 😱 If it is I'm deeply sorry.

However my fee is a flat percent per item set at 50. So in this following screenshot I should see an amount of 1.5 not 2, right?

[luisramos0]

you have the fee in both incoming and outgoing, that's 50% + 50%, right ?

[RachL]

@luisramos0 yes I just saw that. Nevermind, coffee was slow to kick in on this one. Ahem. I'm re-checking all of this right now.

[RachL]

Alright, I've combined all possible calculators. I didn't see something weird. So I think we are good to go! I've updated the notes.

[luisramos0]

great, thanks a lot @RachL :-D