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 fatal when default_locale is empty #142

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

msaggiorato
Copy link
Contributor

@msaggiorato msaggiorato commented Jul 29, 2024

All Submissions:

Changes proposed in this Pull Request:

Fixes a fatal error when trying to generate orders. Some countries may have an empty "default_locale" property. In which case, this would throw a fatal (at least in PHP 8).

There's actually, only one case of this here, so I guess I was "lucky" to hit this. This issue should be fixed separately in Woo itself, but checking here to avoid a fatal wouldn't be bad either IMO.

How to test the changes in this Pull Request:

  1. Check out trunk
  2. Run wp eval "var_dump( WC\SmoothGenerator\Generator\CustomerInfo::generate_person('MV') );"
  3. See fatal
  4. Check out this branch
  5. Run wp eval "var_dump( WC\SmoothGenerator\Generator\CustomerInfo::generate_person('MV') );"
  6. See success

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Fix fatal when generating a large amount of orders, which increases the chances of hitting the empty locale issue.

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

Copy link
Contributor

@coreymckrill coreymckrill left a comment

Choose a reason for hiding this comment

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

@msaggiorato thanks for the PR! Wow, you were indeed lucky to encounter this error. Tested and confirmed that this fixes the issue.

@coreymckrill coreymckrill merged commit 4431276 into woocommerce:trunk Aug 2, 2024
@coreymckrill
Copy link
Contributor

Opened a separate issue to fix the default_locale value for MV:
woocommerce/woocommerce#50322

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants