Fully remove SKU on product level

[drummer83]

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

1. Add different SKUs to the product and it's variant.
2. Check out with that variant.
3. Run the Orders and Distributors report.
4. Run the Order Cycle Supplier Totals report.
5. 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 toggle admin_style_v3; see app/views/admin/products_v3) and
• update Orders and Distributors report to use variant SKU
• Disable column with ignored_columns

Step 2: Then once that change has been released and bedded down, we can perform a full cleanup and delete the database column in a separate PR.

[RachL]

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 ?

[kirstenalarsen]

I would definitely go for Option A - people have distinct SKUs on variants

[kirstenalarsen]

apologies for initial misread!

[RachL]

@openfoodfoundation/core-devs is option A a papercut?

[dacook]

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:

1. hide the field on the new products page and
2. update Orders and Distributors report to use variant SKU

Then I'd say it's a papercut.

[bouaik]

I would like to work on this.

[drummer83]

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!

[RachL]

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!

[zanetagebka]

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

[sigmundpetersen]

Yes you are welcome to @zanetagebka 👍 This is part of the feature #9069 , take a look for more background.

[dacook]

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).

[zanetagebka]

I need to get familiar with it and currently do not have too much spare time, but Im looking at the related ticket.

[zanetagebka]

#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?

[RachL]

Hi @zanetagebka yes we are in the process of putting product_v3 as the default dev environment (see #12627). So if you have the time it would be worth doing it on product_v3 as well.

[RachL]

HI @zanetagebka I've closed your PR because we are now using v3 as our default page. If you wish to work on it again ping me. In the meantime I'm unassigning you.

[murjax]

@RachL I'm interested in picking this up. Is product_v3 fully used in production or are we supporting both new and old pages for now? Also is the product SKU safe to remove from the database after removing from the UI?

[RachL]

@murjax yes product_v3 is fully in production and we are not supporting the old page anymore.

The product SKU can be removed from the database, but a migration might be needed if there are data on the product SKU level. @openfoodfoundation/core-devs do you confirm?

[dacook]

Thanks for picking this up.
We can delete the data, but it will need to be done in a separate PR to ensure a seamless transition. It also gives us the ability to 'rollback' in case any issues pop up.
I've added some additional notes to the "possible fix" section, noting that ignored_columns can be used in the first PR to ensure the column is unused.