Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating a new variant is selecting the first producer and first category of the dropdown #12666

Closed
RachL opened this issue Jul 12, 2024 · 8 comments · Fixed by #12690
Closed
Assignees
Labels
bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround. good first issue

Comments

@RachL
Copy link
Contributor

RachL commented Jul 12, 2024

Use openfoodfoundation/wishlist#520 Flower Farms code in clokify

Description

Since we move the producer and the category to the variant, when creating a new variant in admin/products, the first producer of the user and the first category are selected by default. If the users act fast, they might create the variant with the wrong producer.

We need to leave the dropdown empty by default (which is the case on /admin/products/XXXX/variants/new)

Steps to Reproduce

  1. Go to admin/products with a user who has access to several producers
  2. Create a new variant and notice a producer is already selected, same for the product category

Animated Gif/Screenshot

image

Workaround

Select a different one.

Severity

bug-s3: a feature is broken but there is a workaround

Your Environment

  • Version used: 4.5
  • Browser name and version: Firefox and Chrome
  • Operating System and version (desktop or mobile): desktop
@RachL RachL added good first issue bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround. labels Jul 12, 2024
@github-project-automation github-project-automation bot moved this to To triage (By the maintainers) in Welcome New Developers! Jul 12, 2024
@github-project-automation github-project-automation bot moved this to All the things 💤 in OFN Delivery board Jul 12, 2024
@RachL RachL moved this from To triage (By the maintainers) to Frontend Easy in Welcome New Developers! Jul 12, 2024
@wandji20
Copy link
Contributor

Taking this one

@RachL RachL moved this from All the things 💤 to In Progress ⚙ in OFN Delivery board Jul 17, 2024
@RachL RachL moved this from Frontend Easy to In progress in Welcome New Developers! Jul 17, 2024
@wandji20
Copy link
Contributor

Hello
I would love some clarification on this one.

  1. When creating a product variant, we set the category to that of the last product variant from before_validation :assign_related_taxon, if: ->(v) { v.primary_taxon.blank? }. So I am on the fence as to how we will want to continue with handling this assignment.
  2. When we render SearchableDropdownComponent for the product variant Category and Producer fields, will using Select category and Select producer as respective component include_blank values make sense?

@RachL
Copy link
Contributor Author

RachL commented Jul 17, 2024

@wandji20 My apologies, I thought we would have set the new admin as default by the time someone pick this one up so I forgot one important info: to reproduce the problem, you need to turn the feature toggle admin_style_v3 on.

@wandji20
Copy link
Contributor

wandji20 commented Jul 18, 2024

Thanks 😃 @RachL I was able to turn on the admin_v3_style.

  • The logic we have at the moment for creating new variants from the admin products table will always set the variants' category to the same as the last variant of thesame product if the category is not selected.
    My question is how do we want to handle this logic?
    Should we choose to keep this logic, will it make sense to set thesame value as the default for new variants on the table? Since it will be set anyway if not selected.
  • And for the default text to be displayed on the producer and supplier dropdowns, can we have text like Select producer and Select category respectively?
    Screenshot
    Screenshot from 2024-07-18 16-50-50

@RachL
Copy link
Contributor Author

RachL commented Jul 18, 2024

Hi @wandji20 we've introduced a change recently and you can now select different categories and different producers for each variant under the same product.
So I don't know if it is really simple to do:

The logic we have at the moment for creating new variants from the admin products table will always set the variants' category to the same as the last variant of thesame product if the category is not selected.

Because we would need to introduce a check of the previous variants producers and / or categories, and IF they are the same, pre-select these producers and/or categories. This sounded a bit over-thought, that's why I proposed to leave the dropdown blank.

Since it will be set anyway if not selected.

Are you sure it isn't the first producer and the first category of the dropdown instead? That's what I see when I use a user with a lot of producers. 🤔

The logic we have at the moment for creating new variants from the admin products table will always set the variants' category to the same as the last variant of thesame product if the category is not selected.

Yes good idea!

@wandji20
Copy link
Contributor

Yes, you are right it will set the producer and category to the first element in the dropdown list.

This is because these dropdowns don't have an empty state and so will select the first item on the list.
After introducing changes to keep dropdowns empty, the logic to set variant category runs when no category is selected from the dropdown.
This line is responsible for that!
before_validation :assign_related_taxon, if: ->(v) { v.primary_taxon.blank? } in app/models/spree/variant.rb

My suggestion will be to disable this assignment and allow the user to always select a producer and category when creating a new variant

@RachL
Copy link
Contributor Author

RachL commented Jul 19, 2024

@wandji20 sounds good to me, many thanks!

@wandji20
Copy link
Contributor

Hi
I have opened PR for this one

@github-project-automation github-project-automation bot moved this from In Progress ⚙ to Done in OFN Delivery board Jul 31, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in Welcome New Developers! Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround. good first issue
Projects
Status: Done
2 participants