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

Fix: Correct number formatting by removing unnecessary price separators #888

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

ZZiane
Copy link
Contributor

@ZZiane ZZiane commented Aug 22, 2024

Your checklist for this pull request

Thanks for sending a pull request! Please make sure you click the link above to view the contribution guidelines, then fill out the blanks below.

🚨Please review the guidelines for contributing to this repository.

  • Make sure you are making a pull request against the develop branch (left side). Also you should start your branch off our develop.
  • Make sure you are requesting to pull request from a topic/feature/bugfix/devops branch (right side). Don't pull request from your master!
  • Have you ensured/updated that CLI tests to extend coverage to any new logic. Learn how to modify the tests here.

What does this implement/fix? Explain your changes.

This pull request addresses an issue with number formatting in the maxPrice and minPrice filters for product queries in WPGraphQL WooCommerce. Previously, when users entered values greater than 999, the query failed to return the correct products due to improper formatting.

Specifically, when values over 999 were used in the maxPrice or minPrice filters, the query did not return the expected products, as demonstrated in the screenshots below.

The new commit resolves this issue by removing all number separators , and . The previous implementation used str_replace to remove periods . but it did not account for commas , which are also used as separators in large numbers. This has now been corrected.

Does this close any currently open issues?

Any relevant logs, error output, GraphiQL screenshots, etc?

Before:
error
After:
solution

Any other comments?

...

Where has this been tested?

WooGraphQL Version: 0.21.0
WPGraphQL Version: 1.28.0
WordPress Version: 6.6.1
WooCommerce Version: 7.8.1

@kidunot89 kidunot89 self-requested a review August 29, 2024 16:00
Copy link
Member

@kidunot89 kidunot89 left a comment

Choose a reason for hiding this comment

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

@ZZiane Thanks for this. Will merge if it passes CI.

@ZZiane ZZiane requested a review from kidunot89 August 30, 2024 09:10
@kidunot89 kidunot89 merged commit 5f924bb into wp-graphql:develop Oct 2, 2024
1 check passed
@kidunot89 kidunot89 added the bugfix Implements bugfix label Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Implements bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants