Openfoodnetwork: Delete product will break the shopfront (during the current OC) of all users who have bought that item in the current OC

[RachL]

Description

When I log in with my producer account in Katuma's production I get a weird error and cannot do anything else.

Steps to Reproduce

Just log in with my account rachel.[email protected]

Yesterday I was testing stuff and when I was done I turned my enterprise as "not visible" anymore as I didn't want any real customer to see it. Maybe this is having an impact?

Animated Gif/Screenshot

Severity

bug-s2: a non-critical feature is broken, no workaround

Your Environment

• Version used: v.2.0.2
• Browser name and version: Firefox and Chromium
• Operating System and version (desktop or mobile): Desktop and mobile

[luisramos0]

can you reproduce @RachL ? can you make the shop visible again and see if the error remains? would be good to know if it is related to that.

the error message points to a problem with the line items in the cart.
for reference, the code line where we are assigning line_item:
https://github.com/openfoodfoundation/openfoodnetwork/blob/568e3003ba43c713ac4fb5a57fddfbca086f7b98/app/assets/javascripts/darkswarm/services/cart.js.coffee#L13

[RachL]

@luisramos0 yes I can reproduce @sauloperez as well. But the thing is I cannot do anything on the platform logged in with that user. A super admin needs to make the shop visible again. I can't.

[sauloperez]

Good @luisramos0! I didn't find that skimming through the code. We need to investigate in DB what's wrong with that order and see which one is its root cause.

[luisramos0]

the order will probably be the cart order, the only latest order of the user.

[luisramos0]

Got it. Nice one!
@RachL you have bought Carrots and then deleted Carrots. That's causing this issue in the cart when showing bought items.

If you delete a product that was already sold as part of the current Order Cycle. You will break the cart and therefore the full frontoffice of all users who have bought that item in the current Order Cycle. The problem is gone as soon as the OC is closed.

To replicate the issue:

• Add product to OC
• Checkout an order with that product
• Delete the product (while the OC is still open)
• Go back to shopfront

[luisramos0]

it's very unlikely a user deletes a product that was just sold. Can we downgrade this to S3?

In terms of fix I think we will need similar magic to this in the line_item.variant.product models.

[sauloperez]

Agree on both: better as an S3 and it immediately sounded like a soft-delete issue.

[luisramos0]

This is hard, I am not taking it now...

Even if we handle line_item.variant.product, for example, the current order serializer is loading the variants in the order which include variant.price which is not present in the DB after the product is deleted... there are many approaches to this, none is a few minutes job:

• remove variant serialization from current_order serialization
• make spree::price soft deletable
• make variant code handle the fact the price may not be there...

[luisramos0]

Another option to this issue is to prevent the user from deleting a product that is in an open order cycle (or was sold in one and then removed). We would need to prevent OCs with deleted products to be re-opened.

[sauloperez]

I haven't checked the code but the most reasonable solution seems to be

make spree::price soft deletable

It might make sense to delete a product from an OC in some situations.

[sauloperez]

This one is indeed very annoying, even in development :see_no_evil:

[lin-d-hop]

An issue related to this has popped up on UK production. The producer deleted a product that was sold in an OC. The OC was closed so fortunately we didn't get broken shopfronts. However the hub manager now can't delete the product from invoices etc. It is a different issue but related.

From my understanding of the Network Feature conversations, we've spoken of never being able to delete a product. Other major actors in this space follow that approach. How we handle product delete is also important for Delivery Note and legal Invoicing.

Maybe I am not understanding fully. Forgive me if so. But maybe soft-delete is too much. Maybe we need to be thinking about archiving products?

[luisramos0]

soft delete will do the job. its not actaully a delete. it just marks the
product with a timestamp on a column "deleted at".
i think you should open a separate issue and specify exactly what that
manager needs to do in invoices. ok?

On Fri, 30 Aug 2019 at 13:44, Lynne [e-mail] wrote:

An issue related to this has popped up on UK production. The producer
deleted a product that was sold in an OC. The OC was closed so fortunately
we didn't get broken shopfronts. However the hub manager now can't delete
the product from invoices etc. It is a different issue but related.

From my understanding of the Network Feature conversations, we've spoken
of never being able to delete a product. Other major actors in this space
follow that approach. How we handle product delete is also important for
Delivery Note and legal Invoicing.

Maybe I am not understanding fully. Forgive me if so. But maybe
soft-delete is too much. Maybe we need to be thinking about archiving
products?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openfoodfoundation/openfoodnetwork/issues/3907?email_source=notifications&email_token=AAMQPOSCYVY3V7IUAXIROXLQHEI3XA5CNFSM4HVWHRVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5RRMYA#issuecomment-526587488,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAMQPOT3L6I7WR27AEALXKLQHEI3XANCNFSM4HVWHRVA
.

[sigmundpetersen]

Ok I put this as test ready blocked by merge of #4204
Same as #4203, we can test after merge and see if it's fixed.

Please move it back to dev if I'm wrong @Matt-Yorkley

[RachL]

I can't reproduce. Closing :tada: