reset state and postcode if it's not valid for the current country #47369
Conversation
senadir
added
type: bug
github-actions
bot
added
the
plugin: woocommerce
woocommercebot
requested review from
a team and
nielslange
and removed request for
a team
|
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.
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.
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.
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.
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. |
senadir
force-pushed
the
fix/state-flickring-when-switching-countries
branch
from
e2f18c0
to
3cdc448
Compare
|
I changed the approach in the PR, would love a second review. |
github-actions
bot
added
the
needs: analysis
rodelgc
added
status: analysis complete
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