openfoodfoundation / openfoodnetwork

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

Product Stock Levels not Updating for Producer Shop - if product was added and then hidden from own inventory #4355

Closed lbwright22 closed 4 years ago

lbwright22 commented 5 years ago

Description

Burnt Edge Farm Produce in UK added a new product to their order cycle but it is not showing on the shop front even though it is in stock. The enterprise has no products in their inventory and so shop stock levels are taken from master list. The enterprise does not require products to be in the inventory to be listed on the shop front. Similar issues have occurred in the recent past with Sole of Discretion (another UK producer shop) #4143 and #3961 and were thought to be due to cache update problems.

Expected Behavior

Stock level of product > 0 and product is listed in an order cycle. Product appears for sale on shop front.

Actual Behaviour

Stock level of Breakfast Pack = 2. Product is listed in active order cycle but does not appear on shop front for customers to purchase.

Steps to Reproduce

1. 2. 3. 4.

Animated Gif/Screenshot

image image

Workaround

Severity

S1 This is a critical feature (product which is in stock not available for customer to purchase). No work around. This has occurred in the past too.

Your Environment

Possible Fix

lin-d-hop commented 5 years ago

An additional note. We've experienced this in the UK on two different shopfronts. They are BOTH single producer shopfronts.

This seems relevant as we don't have many active single producer shops in the UK so it is notable that we have two with issues.

luisramos0 commented 5 years ago

there's high probability this is related to cache... can we wait for the PR that removes the cache #4345 and see if this issue remains? it's being tested now, it will probably be live in one week.

daniellemoorhead commented 5 years ago

Downgrading this to a severity 2 as it's only 1 customer experiencing this on 1 product. Should be fixed by the pagination, and this is going live on Monday.

@lin-d-hop and @lbwright22 feel free to dispute this if it doesn't seem right 🙂

luisramos0 commented 5 years ago

:+1: probably "going live on Monday" if testing shopfront pagination goes well today/tomorrow.

Matt-Yorkley commented 5 years ago

Awaiting retest after #4345

lin-d-hop commented 5 years ago

@lbwright22 can you keep an eye on this and test after the next release? There will be a notification of the release in #instance-managers or #deployment channel

lbwright22 commented 5 years ago

@Matt-Yorkley @lin-d-hop @luisramos0 Tested this today, after the shop front pagination release and no change- the product still won't appear in stock for customers to purchase even though stock levels >0

sigmundpetersen commented 5 years ago

Yeah shopfront pagination is in the next release, it will probably be deployed on Tuesday the coming week

daniellemoorhead commented 5 years ago

@lbwright22 time to test, the correct release is up on the UK server now :)

sigmundpetersen commented 5 years ago

Blocked by deployment of pagination fix #4417

RachL commented 5 years ago

@lbwright22 hello! 2.6.1 is now on UK you can retest this :-)

lbwright22 commented 5 years ago

@RachL @sigmundpetersen tested this morning. No change post release of 2.6.1

1) Add Breakfast pack to test order cycle for Burnt Edge Farm Produce 2) stock level of breakfast pack > 0, enterprise does not use inventory 3) breakfast pack does not appear on shop front.

luisramos0 commented 5 years ago

I can see it live. It's a producer shop. I'll try to understand what's going on now.

EDITED: removed my vegan rant...

luisramos0 commented 5 years ago

Looks like this is related to products that are or have been added to own inventory. I think this is replicable:

impact in UK: this is affecting 897 products in 31 different enterprises.

this is how the variants that have been in the inventory are removed from the shop: see the code here

RachL commented 5 years ago

@luisramos0 is this something we can create an automated test for or a test we should add manually in release testing?

luisramos0 commented 5 years ago

downgrading to S2, at least, as I concluded this is only happening to 83 products in uk live and 64 of those are from Burnt Edge Farm Produce. It only happens if you add your product to your own inventory and then remove it (hide) from inventory. If you add that product to an OC, it will not show in the shopfront. This is not related to the latest v2.6.1.

supplier_id | count -------------+------- 200141 | 1 200261 | 1 200275 | 1 200281 | 64 - this is Burnt Edge Farm Produce 200285 | 2 200371 | 4 200436 | 2 200637 | 2 200664 | 4 201198 | 1 201207 | 1

kirstenalarsen commented 5 years ago

is there a workaround to add the products back into their inventory? if they are not using it to manage the order cycle it won't make a difference to the order cycle, and then they would be effectively removing the 'hide' attribute? @lbwright22 ?

luisramos0 commented 5 years ago

The workaround is to go to the inventory and re-add the product to the inventory: Screenshot 2019-11-06 at 12 35 36

So that you see the product in the inventory like this (no overrides are needed): Screenshot 2019-11-06 at 12 42 37

the breakfast pack is back to the shopfront: Screenshot 2019-11-06 at 12 43 29

lin-d-hop commented 5 years ago

64 products missing from a shop front is pretty critical. Is there a workaround we can fix this with? Otherwise this is at least an S2....

lin-d-hop commented 5 years ago

Ha sorry, on phone. Got the updates. Ignore my last

luisramos0 commented 5 years ago

see my workaround @lin-d-hop

I think this is now an S3.

these are the other 19 products with this problem: enterprise_name | enterprise_id | product_name | variant_id ---------------------------------------------+---------------+-----------------------------------------+------------ Noshi Vegan Food | 200261 | Baked Vegan Donut - cookies & cream | 208440 Bury St Edmunds Food Coop | 200371 | Split Green Peas | 208855 Bury St Edmunds Food Coop | 200371 | Liquid Hand Soap - Ecover | 209116 Bury St Edmunds Food Coop | 200371 | Multi-surface Cleaner - Ecover | 209112 Bury St Edmunds Food Coop | 200371 | Peanuts | 209106 Wee WM Country Markets | 200436 | Rye Sourdough Bread NAD, NAE, VFR | 210214 Wee WM Country Markets | 200436 | Pickled Green Chillies | 209611 CROP | 200141 | Basil - green | 205481 Sole of Discretion | 200275 | Smoked Tuna | 208786 Woodside Arran CIC - The Real Food Shop | 200637 | Blood Oranges | 215898 Woodside Arran CIC - The Real Food Shop | 200637 | Peppers | 215936 Burscough Community Farm | 200285 | Courgettes - Wholesale | 217544 Burscough Community Farm | 200285 | Cucumber | 217316 Banc Organics | 200664 | lemongrass | 214677 Banc Organics | 200664 | Mint | 214681 Bought in from Watson and Pratt wholesalers | 201198 | Apple | 226454 Banc Organics | 200664 | Rosemary | 214685 Banc Organics | 200664 | 2. Fresh Coriander | 214679 The Fresh Fish Shop | 201207 | Smoked Halibut | 227079

I think it's better we fix this manually because when unhiding the variant in the inventory, we may want to clear the possibly existing overrides to stock and price.

SQL: select e.name enterprise_name, e.id enterprise_id, p.name product_name, v.id variant_id from enterprises e, spree_products p, inventory_items ii, spree_variants v where e.id = ii.enterprise_id and ii.variant_id = v.id and ii.visible = 'f' and v.product_id = p.id and p.supplier_id = ii.enterprise_id and ii.enterprise_id != 200281;

GDPR doubt: it's ok we publish names of enterprises and names of products in Github right? user data, orders, etc would be very different. Correct me if I am wrong.

luisramos0 commented 5 years ago

I have downgraded to S3 and moved to bug backlog. please feel free to change if you think this bug needs to be solved now.

luisramos0 commented 5 years ago

@RachL re your question: when we fix the bug we should add an automated test.

tschumilas commented 5 years ago

Just checking here - I don't think this is new - has always been that a 'hidden' product is effectively a product not in inventory, therefore it doesn't show in a shop. I think we want it this way - don't we? I think farms and hubs use this to 'hide' things that are seasonal... that won't be back in inventory for a months. This way they don't have to scroll through a ridiculously long inventory to manage stock.

luisramos0 commented 5 years ago

Hi @tschumilas That makes sense although this is the case where producer adds their own products to their inventory. It should be hidden in inventory but it should show as a normal catalog product. The product shows up in the OC page and is selectable...

tschumilas commented 5 years ago

So are you saying a 'hidden' product SHOULD show up in an OC dropdown, or should NOT show up? (regardless of the inventory settings they've chosen on their profile?)

luisramos0 commented 5 years ago

if OC is looking at inventory only, the product should not show. if the OC is supposed to look for products in the catalog, the product should show because it's a product in the catalog of that producer (and in this case it doesnt matter what's in the producer's inventory). From the slack conversation, I think we all agree :-)

luisramos0 commented 4 years ago

Looks like this got prioritized. I am clearing the assignee for now as it may be picked up by another dev now.

Matt-Yorkley commented 4 years ago

I'll take a little look at this now.

It's not clear from the discussion whether this is working as intended or not, or whether some users want one thing and some want another, so I think we need to proceed carefully here...

Related Slack conversation for reference: https://openfoodnetwork.slack.com/archives/CDLKH9MM0/p1573041910137500

luisramos0 commented 4 years ago

Matt, I think it's clear:

Matt-Yorkley commented 4 years ago

Ok, so for OCs that use inventory only, we don't need any change, right?

The default is to use the regular catalogue, and that also includes scoping variants with overrides. The issue is that we need to exclude hidden overrides from the scoping process when fetching products to show in the shop, if the OC has product_selection_from_coordinator_inventory_only: false. And that scoping is happening in app/services/products_renderer.rb...

Matt-Yorkley commented 4 years ago

The explanatory note related to hidden products (in the inventory UI) says: "These products have been hidden from your inventory and will not be available to add to your shop. You can click 'Add' to add a product to you inventory."

Screenshot from 2020-02-18 10-25-51

luisramos0 commented 4 years ago

Now I see what you mean. Using the catalog "also includes scoping the variants". It feels like this could be a feature, not a bug... Anyway, what is being requested is that when the "producer" override their own catalog, they are still able to add the product to OCs that use the "All Available Products" setting. You are right, we would need to skip the scoping process for these products. We need to scope variants all the time except if overrides are from the producer and are hidden!?

WAIT, why do we want to do this? there's one alternative that does not involve this weird code cchange: if the producer wants to add their own overriden products to the OC they need to unhide the products in their inventory.

Product wizards? does this make sense?

Matt-Yorkley commented 4 years ago

Yeah, it's not super clear here.

I'm still slightly concerned that a change to the core logic for showing/hiding products could potentially be an unwelcome surprise for some other users who are not expecting it, if they are depending on the logic we have currently. And we can't know if that's the case or not until we change it in production and they complain...

Matt-Yorkley commented 4 years ago

Another perspective: the problem from the user's point of view is that these variants with hidden overrides can be added to the OC from the OC edit page, but they don't then show up in the shop. The discrepancy between what they expect and what happens is the cause of the annoyance. So an alternate approach would be to make it clearer in the UI of the OC edit page that those variants have been hidden via overrides, and will not appear in the shopfront. That way it's clearer what is happening, and the user will not be surprised/annoyed by unmet expectations.

Does that make sense? I'm not sure this is the optimal solution, just throwing it out there.

luisramos0 commented 4 years ago

I wonder if that is a problem with all hidden variants (can be added to OC but dont show up in shopfront) or just for overridden own variants?

Matt-Yorkley commented 4 years ago

Otherwise the solution is something like:

If: the coordinator of an OC (which is set to use the catalogue) is also a producer and is selling their own products, and some of those products have been overridden, but the overrides are set to hidden... Then: when the variants for that enterprise are fetched for the shopfront, they all need to be scoped to their available overrides, except the ones that are hidden, which need to remain unscoped.

Is that right? I think the required code changes will be horrible...

Matt-Yorkley commented 4 years ago

I wonder if that is a problem with all hidden variants (can be added to OC but dont show up in shopfront) or just for overridden own variants?

I'll take a look...

Matt-Yorkley commented 4 years ago

Here's the OC edit UI for a regular enterprise using hidden overrides (not own products). In this example Pork (Organic) is an override, and Sausage Meat (Organic) is a hidden override:

The incoming products page shows both variants, and they can both be selected: oc-incoming

The outgoing products page shows slightly odd counts, and shows the product for the "hidden" variant, but it can't be selected (no checkbox): oc-outgoing

There's no indication to the user that the variant is "hidden" by an override or that it won't show in the shopfront, but it looks like it can't be added to the OC (which I guess is working as intended).

Matt-Yorkley commented 4 years ago

In the above example (sorry it's not vegan, @luisramos0) I guess that the Sausage Meat (Oraganic) product could have multiple variants, and for example some could be hidden and some could be visible, in which case it would be reasonable to display it like this.

Matt-Yorkley commented 4 years ago

Take 2; Chicken Thighs 550g variant is a hidden override, 540g is a non-hidden override.

Incoming products: oc2-incoming Outgoing products: oc2-outgoing

Apart from the slightly confusing "4 of 2 Variants Loaded" message, I think this is ok.

Matt-Yorkley commented 4 years ago

Another thing here is in the inventory UI: after you've added an override for a variant, you have the option to hide it, but you don't have an option to remove the override, eg "stop" overriding it...

luisramos0 commented 4 years ago

great investigation Matt! is take 2 with a owned override? I guess the issue is that if it is an owned hidden override you will be able to add it on the outgoing exchange, right?

on this last comment: you cant stop overriding but you can unhide it and define the override to use the catalog values (it's the current workaround for this issue).

Matt-Yorkley commented 4 years ago

I'm leaning towards some conclusions here:

There are 3 distinct issues here that need to be worked on:

  1. the order cycle UI doesn't indicate to the user that a variant is overridden and hidden, and it probably should. This might need some inception / product input
  2. we probably need an option in the inventory page to "stop" overriding a variant, and that can be a small separate issue
  3. the counts in the OC edit pages should not include hidden overrides, and that can be a small separate issue
Matt-Yorkley commented 4 years ago

I can confirm that it works exactly the same for overrides for an enterprise's "own" inventory. The variant shows in the incoming products page (and is selectable), but doesn't have a checkbox to select it in the outgoing products page. If it's overridden and hidden, it's overridden and hidden.

luisramos0 commented 4 years ago

nice! the main issue is that the user managed to add the variant to the OC. So if the hidden override is not showing on the outgoing exchange to be added, the user that experienced this issue must have used the simple OC edit page? the other option is that the recent work on the OC page fixed this issue :-)

" If it's overridden and hidden, it's overridden and hidden." If it's overridden, it's overridden, but if it is hidden the usder can unhide and she will be able to add it to the OC, right?

Matt-Yorkley commented 4 years ago

the other option is that the recent work on the OC page fixed this issue :-)

I suspect that's the case...

if it is hidden the user can unhide and she will be able to add it to the OC, right?

I think this requires some inception around how we will enable this in the UI. The user can unhide it now (from the inventory page) and go back to the OC and add it. Do we want to add an unhide button in the OC edit UI somehow? Or just a clear indication that the variant is hidden via inventory? I think these questions need some product input...

luisramos0 commented 4 years ago

I agree this needs product input. The listed products without checkbox are there to represent variants in the OC that this user in particular does not have permissions to add or remove from the OC. Not exactly the same as hidden overrides...

If the original issue "user can add hidden variant to OC (and variant doesnt show on shopfront)" is not applicable any longer I think we should close this issue and open other issues for different problems.

tschumilas commented 4 years ago

I've been following this issue because its happened to simple farm shops here multiple times. I'm so glad you are onto it!!! Users do not realize that they have a variant 'hidden' in their inventory. They can't figure out why the product won't show in the shop. So from a support perspective - it would be great to have some kind of message to the user setting up an OC with hidden products. ie - you have products hidden in your inventory and need to unhide these before they will show in your shopfront. I'm not sure it matter if they can do it right in the OC or if they need to go to their inventory. (In a way - I think its good for them to go to inventory because it will show them that its there, and that inventory does not equal product list.)

Matt-Yorkley commented 4 years ago

Ok, if there's no objections I'll split this into three new issues that are easier to follow and more clearly scoped, and close this one.