openfoodfoundation / openfoodnetwork

Connect suppliers, distributors and consumers to trade local produce.
https://www.openfoodnetwork.org
GNU Affero General Public License v3.0
1.1k stars 714 forks source link

Subscriptions can be edited to have line items with quantity = 0 #9092

Open filipefurtad0 opened 2 years ago

filipefurtad0 commented 2 years ago

Description

Follow-up from https://github.com/openfoodfoundation/openfoodnetwork/pull/9011.

Subscriptions can be edited to have line items with quantity = 0. After that, triggering the sub and generating the orders will display that line item (with quantity zero) on emails and invoices.

Expected Behavior

Line items with quantity = 0 should not appear on subscription orders. (The exception is of course if stock restrictions apply.)

Actual Behaviour

Line items with quantity = 0 appear on subscription orders. (even though these are not resulting from stock restrictions)

Steps to Reproduce

  1. Edit a subscription and add two items. Hit save.
  2. Reduce the quantity of one of them down to zero. Hit save.
  3. Notice the line item is still kept on that subscription.
  4. Run the subscription job.
  5. Take a look at order confirmation emails/invoices -> notice that zero-quantity line item is still appearing there.

Animated Gif/Screenshot

Peek 2022-04-13 11-24

Workaround

Edit the subscription and remove the line item (click the trashbin icon).

Severity

Your Environment

Possible Fix

Julien-Lopez commented 2 years ago

Hi everyone!

I'm very interested in the project, if you don't mind I would like to try to tackle this issue to get familiar with the code. :slightly_smiling_face:

jibees commented 2 years ago

hi @Julien-Lopez 👋 Welcome here!

I'll assign you to the issue ; thanks a lot. feel free to ask question, and join the slack: https://openfoodnetwork.org/slack-invite

RachL commented 2 years ago

@Julien-Lopez welcome! Just FYI subscriptions can be very tough code to start with. If you feel overwhelmed, don't feel bad and move to another issue. We have some issues labeled good-first-issues and we have this board also we we try to triage by type of skills: https://github.com/orgs/openfoodfoundation/projects/2

Julien-Lopez commented 2 years ago

Thank you @jibees and @RachL ! I'll work on this in my spare time and switch to a simpler issue if I'm stuck. I tried a patch on the controller that failed probably because of a fail-safe on the model, so I'll investigate there next.

mkllnk commented 1 year ago

@filipefurtad0 I just came across this issue and I'm wondering if we actually want to solve it. When products become unavailable then the line items quantities are marked as zero. That's intended behaviour. Now imagine a customer calling up and saying that hey want tomatoes in their order. As admin, you add the tomatoes but then realise they are unavailable. Doesn't it make sense to still show this on the invoice? Otherwise the customer thought you forgot to add the tomatoes.

I think, as admins we should be able to reproduce the same data that the subscription logic generates. And in the subscription logic, a line items with quantity 0 is a valid way to say: "you wanted this but we don't have it"

What do you think?