refactor: ECE buttons and settings consistency #11182
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Express Checkout Elements (ECE) settings to improve consistency by migrating from a single payment_request setting on the card gateway to individual enabled settings for Apple Pay and Google Pay gateways. This change provides better separation of concerns and clearer settings management.
Key Changes
- Introduced a migration class to transition existing
payment_requestsettings to individual Apple Pay and Google Pay gateway enabled states - Updated
is_payment_request_enabled()method to check the enabled status of Apple Pay and Google Pay gateways instead of reading a single option - Modified Apple Pay domain registration to listen to Apple Pay gateway settings instead of card gateway settings
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| includes/migrations/class-migrate-payment-request-to-express-checkout-enabled.php | New migration class to convert legacy payment_request setting to individual gateway enabled settings |
| includes/class-wc-payment-gateway-wcpay.php | Updated is_payment_request_enabled() to check Apple Pay/Google Pay gateway enabled states |
| includes/class-wc-payments-apple-pay-registration.php | Refactored to listen to Apple Pay gateway settings and use constant for error option name |
| includes/admin/class-wc-rest-payments-settings-controller.php | Updated REST API to read/write Apple Pay and Google Pay gateway enabled settings |
| includes/class-wc-payments-onboarding-service.php | Modified to enable/disable Apple Pay and Google Pay gateways based on capabilities |
| includes/class-wc-payments-account.php | Updated to use new is_payment_request_enabled() method |
| includes/class-wc-payments-status.php | Updated status report to use new method |
| includes/class-duplicates-detection-service.php | Added method check for is_payment_request_enabled() |
| includes/express-checkout/class-wc-payments-express-checkout-button-handler.php | Updated to use new is_payment_request_enabled() method |
| includes/express-checkout/class-wc-payments-express-checkout-button-display-handler.php | Updated to use new is_payment_request_enabled() method |
| includes/class-wc-payments.php | Registered migration hook |
| includes/class-wc-payments-blocks-payment-method.php | Removed unnecessary blank line |
| tests/unit/test-class-wc-payments-apple-pay-registration.php | Updated tests for Apple Pay registration with new gateway-based checks and helper methods |
| tests/unit/test-class-wc-payment-gateway-wcpay.php | Added comprehensive tests for is_payment_request_enabled() method |
| tests/unit/migrations/test-class-migrate-payment-request-to-express-checkout-enabled.php | New test file for migration class |
| tests/unit/test-class-wc-payments-account.php | Updated test to use new is_payment_request_enabled() method mock |
| tests/unit/admin/test-class-wc-rest-payments-settings-controller.php | Added tests for REST API payment request enable/disable functionality |
| tests/unit/duplicate-detection/test-class-duplicates-detection-service.php | Updated test to use new property-based approach |
| tests/unit/duplicate-detection/class-test-gateway.php | Added is_payment_request_enabled() method to test gateway |
| changelog/refactor-google-pay-apple-pay-settings-storage | Added changelog entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
Size Change: 0 B Total Size: 926 kB ℹ️ View Unchanged
|
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
frosso
commented
Dec 5, 2025
| 'type' => 'title', | ||
| 'description' => '', | ||
| ], | ||
| 'payment_request' => [ |
Contributor
Author
There was a problem hiding this comment.
No longer needed - the payment_request setting got deleted in favor of the enabled flag on the Google Pay/Apple Pay payment method definitions
frosso
commented
Dec 5, 2025
Comment on lines
866
to
883
| // Update Google Pay and Apple Pay enabled settings to keep them in sync. | ||
| $google_pay_gateway = WC_Payments::get_payment_gateway_by_id( 'google_pay' ); | ||
| if ( $google_pay_gateway ) { | ||
| if ( $is_payment_request_enabled ) { | ||
| $google_pay_gateway->enable(); | ||
| } else { | ||
| $google_pay_gateway->disable(); | ||
| } | ||
| } | ||
|
|
||
| $apple_pay_gateway = WC_Payments::get_payment_gateway_by_id( 'apple_pay' ); | ||
| if ( $apple_pay_gateway ) { | ||
| if ( $is_payment_request_enabled ) { | ||
| $apple_pay_gateway->enable(); | ||
| } else { | ||
| $apple_pay_gateway->disable(); | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This is where things get "muddy": Google Pay and Apple Pay are two separate payment methods, implemented with two different payment method classes.
But in the settings page, only one checkbox controls both of them.
In other words:
- For Google Pay/Apple Pay: 1 checkbox controls both of them.
- For other payment methods: 1 checkbox controls one payment method.
Now, in order to have them appear "enabled" on the payment methods list at checkout (for the subsequent work), I need to ensure that they are both "enabled".
I considered adding an action somewhere that listened to changes to either the Google Pay or Apple Pay payment method settings and enabled/disabled the other automagically.
But I thought it wasn't very explicit. And since it would have been a filter somewhere else, I thought it wouldn't a "transparent" mechanism, which could have lead to bugs.
frosso
commented
Dec 5, 2025
| * | ||
| * @var string | ||
| */ | ||
| private $apple_pay_verify_notice; |
Contributor
Author
There was a problem hiding this comment.
In the previous implementation, the error message was stored as an attribute on the class.
The problem is that the settings updates happen over AJAX. And the attribute is not persisted across page refreshes.
So the exact error message was never displayed to the merchant.
I modified the implementation to store the error message in a new wcpay_apple_pay_domain_error option, so it can be persisted across page refreshes.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes WOOPMNT-5546
Changes proposed in this Pull Request
In #11144 we migrated Google Pay/Apple Pay to have their own payment method definition.
That change was iterative, and left some inconsistency: all the other payment methods have their own
enabledoption. But Google Pay/Apple Pay still relied on thepayment_requestoption on the main gateway class.With these changes, I'd like to bring consistency to that.
These changes are needed as part of the Dynamic "Place Order" Button project (larger PR here), which aims to having Google Pay and Apple Pay to be displayed as part of the main gateway list.
After this PR, there's another slight inconsistency: the
settingsendpoint isn't returninggoogle_payorapple_payin theenabled_payment_method_idsarray.That change would affect the Settings UI/JS. I am not implementing that change (yet) because it would blow this PR even further 😅
Testing instructions
1. Test the migration
SELECT * FROM wp_options WHERE option_name = 'woocommerce_woocommerce_payments_settings'payment_requestkey with valueyeswoocommerce_woocommerce_payments_versionoption to a previous version, like10.1.0woocommerce_woocommerce_payments_versionshould be updated to a newer versionwoocommerce_woocommerce_payments_settingsshould no longer contain thepayment_requestkeywoocommerce_woocommerce_payments_apple_pay_settingsshould exist withenabled=yes2.
apple_pay&google_paynot inenabled_payment_method_ids/wp-json/wc/v3/payments/settingsendpoint responseenabled_payment_method_idsarray should not containgoogle_payorapple_payavailable_payment_method_idsarray should containgoogle_payandapple_paywindow.wcpayConfig.paymentMethodsConfigshould not containgoogle_payorapple_pay3. test saving the settings
Tip
After updating the settings,
woocommerce_woocommerce_payments_apple_pay_settingsandwoocommerce_woocommerce_payments_google_pay_settingsmight include some additional keys, other thanenabled. That's normal.npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge