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

[inventory] Fix product sorting #12680

Conversation

rioug
Copy link
Collaborator

@rioug rioug commented Jul 17, 2024

What? Why?

The product sorting in the inventory got broken after the product refactor, where the supplier got moved to the variant, this PR re add the sorting. Product are now sorted by producer alphabetically and product alphabetically.

What should we test?

  • Go to Inventory
  • Observe that products are displayed in alphabetical order by producer
  • Observe that products are displayed in alphabetical order by product within the same producer

This may help with #12679, but it might be easiest to wait and test the replication case given in production.

Release notes

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes
  • API changes (V0, V1, DFC or Webhook)
  • Technical changes only
  • Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

It looks like the ordering by producer got lost in some rebase.
@rioug rioug added the user facing changes Thes pull requests affect the user experience label Jul 17, 2024
Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks for adding the spec to help protect us in the future 💯

permissions_list: [:create_variant_overrides])

result = query.products_for_producers
expect(result).to eq([product_b3, product_b1, product_b2, product3, product4])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first glance, it's not easy to see if this is correct or not. We could name the variables the same as the record names to make it clearer.

Not that I'm complaining, at least we have a spec now!!

@dacook dacook added the bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround. label Jul 17, 2024
@filipefurtad0 filipefurtad0 self-assigned this Jul 17, 2024
@filipefurtad0 filipefurtad0 added no-staging-FR A tag which does not trigger deployments, indicating a server is being used pr-staged-fr staging.coopcircuits.fr and removed no-staging-FR A tag which does not trigger deployments, indicating a server is being used pr-staged-fr staging.coopcircuits.fr labels Jul 17, 2024
@filipefurtad0
Copy link
Contributor

filipefurtad0 commented Jul 17, 2024

Hey @rioug,

This looks good. Before staging the PR, one can see that both products and variants are not sorted alphabetically; this changes after staging the PR:

image

We can see that first, it is ordered by producer (I've changed the product name tempura to carrots, to test this case):

image

And that both products and variants are ordered within each producer / product.

Merging!

@filipefurtad0 filipefurtad0 merged commit 3bd5ae2 into openfoodfoundation:master Jul 17, 2024
60 of 61 checks passed
@filipefurtad0 filipefurtad0 removed the pr-staged-fr staging.coopcircuits.fr label Jul 17, 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. user facing changes Thes pull requests affect the user experience
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Inventory] Products duplicated and not sorted alphabetically
3 participants