openfoodnetwork
openfoodnetwork copied to clipboard
Fully remove SKU on product level
Description
It is currently possible to set the SKU on product level (on page /products
) as well as on variant level (on pages /products
and /products/<Product_ID>/variants/<Variant_ID>/edit
. However it is unclear which one is used where.
This could be related to #10939 where master variants have been removed - not sure.
Assumption 1: Since it is not possible to edit the product's SKU on page /products/<Product_ID>/edit
, removing it from /products
page could have been overlooked. In this case always the variant's SKU should be used everywhere - but in some reports the product's SKU is still being used.
~~Assumption 2: It was intentional to keep the SKU field on product level. Then it should be clear where it is used insead of the variant's SKU.~~
Expected Behavior
☑️ Option A: Fully remove SKU on product level
- Remove the input field for SKU on page
/products
. - Search the code for places where the product's SKU is being used and replace it with the variant's SKU (e.g.
Orders and Distributors
report). - Disadvantage: It's unknown to me if and how much the product's SKU is being used by our users. A migration (maybe including communication to users could be required). But since in most customer facing cases the variant's SKU is already being used, the effect might be small.
- Disadvantage: Uncomfortable to add SKUs when it needs to be done for every single variant.
- Advantage: Clearly defined usage of SKU.
🆇 ~~Option B: Fully keep SKU on product level~~
- Add SKU input field on page
/products/<Product_ID>/edit
. - Clearly indicate where which SKU is being used, e.g. by naming them Product SKU vs. Variant SKU or similar.
🆇 ~~Option C: Keep the input field on product level but always use variant's SKU~~
- Search the code for places where the product's SKU is being used and replace it with the variant's SKU (e.g.
Orders and Distributors
report). - Proposal: Add logic to copy the product's SKU to all variants, when it's updated. Optional: Ask before overwriting existing variant's SKUs.
- Advantage: It's quick to add the same SKU or a SKU-prefix to all variants.
- Disadvantage: It's unknown to me if and how much the product's SKU is being used by our users. A migration (maybe including communication to users could be required). But since in most customer facing cases the variant's SKU is already being used, the effect might be small.
Actual Behaviour
SKU can be set for product and variant:
Product's SKU can't be set on the /products/<Product_ID>/edit
page:
Variant's SKU can be set on the /products/<Product_ID>/variants/<Variant_ID>/edit
page:
Variant's SKU is used in order confirmation email:
Variant's SKU is used in order cycle report email:
Product's SKU is used in Orders and Distributors report:
Variant's SKU is used in Order Cycle Supplier Totals report:
Other reports have not yet been checked.
Steps to Reproduce
- Add different SKUs to the product and it's variant.
- Check out with that variant.
- Run the Orders and Distributors report.
- Run the Order Cycle Supplier Totals report.
- See that the different SKUs are being used.
Animated Gif/Screenshot
See above.
Workaround
Severity
bug-s3: a feature is broken but there is a workaround OR bug-s5: we can live with it, only a few users impacted
Your Environment
- Version used:
- Browser name and version:
- Operating System and version (desktop or mobile):
Possible Fix (for option A)
Step 1:
- hide the field on the old products page (
/admin/products
) - hide the field on the new products page (
/admin/products
with feature toggleadmin_style_v3
; seeapp/views/admin/products_v3
) and - update Orders and Distributors report to use variant SKU
Step 2: Then once that change has been released and bedded down, we can perform a full cleanup and delete the database columns.
Thanks Konrad!
I don't see uses cases for option C. I would go for option A: what do you think @lin-d-hop @kirstenalarsen ?
I would definitely go for Option A - people have distinct SKUs on variants
apologies for initial misread!
@openfoodfoundation/core-devs is option A a papercut?
Option A: Fully remove SKU on product level
- Remove the input field for SKU on page
/products
.- Search the code for places where the product's SKU is being used and replace it with the variant's SKU (e.g.
Orders and Distributors
report).
With a quick search of the codebase, it appears that Konrad has already identified all usages.
Note that there's currently two /products
screens. I think it would save a little bit of time to ignore the old one.
But if we're removing the product SKU, I think we should also consider:
- Go ahead and remove it from the database entirely.
But if we're happy to start with simply:
- hide the field on the new products page and
- update Orders and Distributors report to use variant SKU
Then I'd say it's a papercut.
I would like to work on this.
Sure, thank you, @bouaik! I have assigned you.
Let's go for option A.
And please check David's comment above for some helpful information.
Thanks again!
Hello @bouaik ! I hope this message finds you well. Are you still planning to work on this issue? Let us know if you need more info!
Is anyone working on it? If not I could try to take a look and get more familiar. I do not want to be a only Rubocops person :D
Yes you are welcome to @zanetagebka 👍 This is part of the feature #9069 , take a look for more background.
Thanks Zaneta, I've updated the "possible fix" section in the description above to clarify what's expected. I realised we need to remove the field from the old and new product screens, to ensure we're able to proceed with deleting the data later. (I think the old screen will stay for a while).
I need to get familiar with it and currently do not have too much spare time, but Im looking at the related ticket.
https://github.com/openfoodfoundation/openfoodnetwork/pull/12650
I opened draft PR here. As far as I understood there is currently only need to "hide" this field. I found there is products_v3 as well, do we want to remove it from there as well?
Hi @zanetagebka yes we are in the process of putting product_v3 as the default dev environment (see https://github.com/openfoodfoundation/openfoodnetwork/issues/12627). So if you have the time it would be worth doing it on product_v3 as well.