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

Allow searching orders by customers' full name with comma and full name reversed #11891

Conversation

bmd08a1
Copy link
Contributor

@bmd08a1 bmd08a1 commented Dec 3, 2023

What? Why?

What should we test?

  • Visit /admin/orders/bulk_management page.
  • Try searching with Mistery John or John, Mistery (as displayed to the user) with built-in sample data
  • Show line items linked to orders made by that customer

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

Allow searching orders by customers' full name with comma and full name reversed

Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Thank you! This is great.

For completeness it would be great to also search for reversed name with comma. Would you mind adding that?

spec/system/admin/bulk_order_management_spec.rb Outdated Show resolved Hide resolved
Copy link
Collaborator

@rioug rioug left a comment

Choose a reason for hiding this comment

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

Thanks for your help !

@bmd08a1
Copy link
Contributor Author

bmd08a1 commented Dec 4, 2023

Thank you! This is great.
For completeness it would be great to also search for reversed name with comma. Would you mind adding that?

Ye, let me add it

Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Perfect, thank you!

@drummer83 drummer83 self-assigned this Dec 6, 2023
@drummer83 drummer83 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 labels Dec 6, 2023
@drummer83
Copy link
Contributor

drummer83 commented Dec 6, 2023

Hi @bmd08a1,
Thank you so much for this contribution! It's looking great!

I compared the results when searching for different combinations before and after your PR (the displayed name in the orders is 'Azra, Angel'):
grafik

Search string Before PR After PR
Angel Azra ✔️ ✔️
Angel Az ✔️ ✔️
Azra, Angel no results ❌ ✔️
az an no results ❌ no results ❌
Azra, an no results ❌ ✔️
an az no results ❌ no results ❌

Result

We have a great improvement here! 💪
Most of the use cases should now be covered. The remaining two are edge cases which we shouldn't worry too much about now.

I couldn't find any other issues with your solution, so this one is ready for merging! 🚀
Thanks again! 🥳

@drummer83 drummer83 merged commit 7efc6f3 into openfoodfoundation:master Dec 6, 2023
54 checks passed
@drummer83 drummer83 removed the pr-staged-fr staging.coopcircuits.fr label Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[BOM] Allow to search with Lastname Firstname order and with a comma
4 participants