fix(blocks): fix express checkout address validation in modal checkout#597
fix(blocks): fix express checkout address validation in modal checkout#597rbcorrales wants to merge 7 commits into
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates newspack-blocks’ modal checkout integration to ensure billing fields configured off in Audience → Payment are unregistered anywhere WooCommerce renders/validates checkout fields (not just during modal checkout), preventing express wallet checkouts from failing validation on hidden fields (NPPM-2937).
Changes:
- Expand
Modal_Checkout::woocommerce_checkout_fields()so configured-off billing fields are removed for standard checkout contexts too, while preserving the existing My Account and shippable-cart exclusions (plus a null-cart guard). - Add unit tests covering standard checkout behavior, no-config behavior, shipping-required carts, cart-unavailable contexts, and My Account exclusions.
- Introduce minimal test mocks for
Newspack\WooCommerce_My_Accountand WooCommerce globals needed to exercise the updated behavior in thenewspack-blockstest environment.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
plugins/newspack-blocks/includes/class-modal-checkout.php |
Widens checkout field removal beyond modal requests; adds My Account + null-cart/shipping guards. |
plugins/newspack-blocks/tests/test-modal-checkout.php |
Adds mocks and new unit tests validating field removal across contexts. |
plugins/newspack-blocks/tests/mocks/newspack-plugin-mocks.php |
Adds a minimal WooCommerce_My_Account mock for test environments without newspack-plugin. |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dkoo
left a comment
There was a problem hiding this comment.
Tested end-to-end on a dev site with Apple Pay enabled. All testing steps pass as described—the invalid address fields are scrubbed on an express checkout request during a modal checkout where those fields are disabled, allowing the checkout to complete, while default validation prevails when address fields are required.
Approved with comments. This is a careful, well-tested hotfix. The two new Store-API hooks are correctly wired (right rest_pre_dispatch signature and null-return contract, set_param writes to the bucket the Store API reads, cart/shipping guards in place), and the scrub only ever drops values that would fail WooCommerce validation anyway — valid wallet values pass through and are stored. I verified the core logic end-to-end against a live WooCommerce env (invalid state dropped, valid code kept, configured-on field untouched, required relaxed only under a modal referer) and confirmed no regression in standard checkout. The referer-trust surface is benign: a forged referer can only loosen required on two optional fields for the caller's own non-shipping cart — no control is bypassed and there's no cross-user effect. All prior Copilot comments are addressed.
One thing worth fixing before merge, plus two minor notes below. Not blocking.
| $states = \WC()->countries->get_states( $country ); | ||
|
|
||
| if ( is_array( $states ) && ! empty( $states ) ) { | ||
| $code_match = array_key_exists( strtoupper( $address['state'] ), array_change_key_case( $states, CASE_UPPER ) ); |
There was a problem hiding this comment.
Suggestion: The scrub runs strtoupper() / strtolower() / WC_Validation::is_postcode() on the raw address value here, on rest_pre_dispatch — before the Store API schema sanitizes or rejects the request. If a modal checkout request sends billing_address.state (or .postcode) as a JSON array instead of a string, these throw a PHP TypeError and the endpoint returns HTTP 500 instead of the clean 400 the schema would have returned. I reproduced this on PHP 8.3 (strtoupper(): Argument #1 ($string) must be of type string, array given). Only reachable for modal-originated requests on sites with the field configured off, and the request was going to fail either way — but a one-line is_string() guard on the value before transforming turns the 500 back into the intended 400.
| return $result; | ||
| } | ||
|
|
||
| $address = $request->get_param( 'billing_address' ); |
There was a problem hiding this comment.
Suggestion: The scrub only touches billing_address and bails entirely when the cart needs shipping. WooCommerce's CheckoutSchema registers a validate_callback on the shipping_address argument itself, which runs whenever a shipping_address payload is submitted regardless of cart shipping state — so a wallet that attaches a shipping_address with an invalid state on a virtual/donation cart (shipping not needed) could still hard-fail the same validation this fix targets, unscrubbed. Likelihood is low (wallets generally return only billing contact when shipping isn't requested), so this is a consider-scrubbing-symmetrically note rather than a blocker — or document why shipping_address is intentionally out of scope.
| * | ||
| * @return bool | ||
| */ | ||
| private static function is_modal_checkout_referer() { |
There was a problem hiding this comment.
Nit: is_modal_checkout_referer() matches with a raw false !== strpos( $referer, 'modal_checkout=1' ) against the whole referer URL, whereas this class already parses the referer properly a few lines down in is_express_checkout() via wp_parse_url( …, PHP_URL_QUERY ) + wp_parse_str. The strpos form also matches modal_checkout=1 appearing in a path segment, fragment, or another param's value. It's functionally fine for the real iframe URL and the security impact is nil (see the review summary), but reusing the existing wp_parse_url/wp_parse_str idiom would keep the two modal-detection paths consistent and slightly harder to trip accidentally.
All Submissions:
Changes proposed in this Pull Request:
Express checkout wallets (Apple Pay, Google Pay) in the modal checkout submit to the Store API (
wc/store/v1/checkout), whose address validation rejects any state value not in the country's list. Wallets can supply values the buyer never sees and cannot correct (for example a locality in the state slot), hard-failing the payment on a field the site is configured not to collect.The fix is scoped strictly to the modal checkout, so standard Woo checkout flows keep stock behavior (per review discussion, the modal-only scoping of the billing fields setting is deliberate):
rest_pre_dispatchscrub drops configured-off state/postcode values from modal-originatedwc/store/v1/checkoutrequests, and only when the value would fail validation anyway; valid wallet values pass through untouched and are stored on the order. Modal origin is detected via the referer (the modal iframe URL carriesmodal_checkout=1), since express submissions carry no request params.woocommerce_get_country_localefilter relaxes the required flag for configured-off state/postcode on modal-originated requests, so the scrubbed value's absence is accepted. It never applies when the cart needs a shipping address.Everything outside the modal is byte-for-byte stock: the standard Woo checkout page, the blocks checkout, express buttons on product/cart pages, My Account flows, and any cart needing a shipping address.
How to test the changes in this Pull Request:
woocommerce_statesto replace the state list for your wallet's country). Confirm checkout completes with the invalid state dropped, instead of failing with "The provided state (...) is not valid".Other information: