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
reset state and postcode if it's not valid for the current country #47369
Conversation
Hi @nielslange, @opr, Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
/** | ||
* There's a bug in WooCommerce core in which not having a state ("") would result in us validating against the store's state. | ||
* This resets the state to an empty string if it doesn't match the country. | ||
* | ||
* @todo Removing this handling once we fix the issue with the state value always being the store one. | ||
*/ | ||
if ( ! $validation_util->validate_state( $billing_state, $billing_country ) ) { | ||
$billing_state = ''; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is no longer needed now that we reset at sanization level, I also tested this PR steps and they're still working
$carry[ $key ] = $validation_util->validate_state( $address['state'], $address['country'] ) ? $validation_util->format_state( sanitize_text_field( wp_unslash( $address[ $key ] ) ), $address['country'] ) : ''; | ||
break; | ||
case 'postcode': | ||
$carry[ $key ] = $address['postcode'] ? wc_format_postcode( sanitize_text_field( wp_unslash( $address['postcode'] ) ), $address['country'] ) : ''; | ||
$carry[ $key ] = \WC_Validation::is_postcode( $address['postcode'], $address['country'] ) ? wc_format_postcode( sanitize_text_field( wp_unslash( $address['postcode'] ) ), $address['country'] ) : ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to reset both postcode and state
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, @senadir. The fix works as expected. I just closed and re-opened the PR so that the CI checks can run. Let's ensure that all CI checks are passing before merging this PR.
Some tests are failing and they seem to be valid, will look into them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When testing this using Store API, I was able to place an order with a JP
address with an invalid postcode and an invalid state.
I think this is because we're unsetting invalid values in the AbstractAddressSchema
sanitize_callback
and this also happens on checkout. If there's a way we can limit that logic to run only on the cart/update-customer
route that would be better.
I will look into that, it's strange because I didn't remove any validation (not counting the one in get_address_x), I added more validation, I will look into this. I knew this issue would be tricky. |
e2f18c0
to
3cdc448
Compare
I changed the approach in the PR, would love a second review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is working nicely for me now, thanks Nadir!
When switching countries, Checkout would sometimes clear out state and zipcode, there are some cases in which those fields are not cleared correctly, and the server end up validating the current country against the old state and postcode. This PR resets them when changing country, and make sure to reset them as well if you're changing country, this is because state and postcode would always be validated client side first.
Initially I tried fixing this at server side, but it caused issues when placing an order, this feels like a good middleground.
Closes #46270
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Changelog entry
Significance
Type
Message
Correctly clear out state and postcode when switching countries.
Comment