-
Notifications
You must be signed in to change notification settings - Fork 69
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
Disable incompatible payment methods when manual capture is enabled #9470
Conversation
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. |
Size Change: 0 B Total Size: 1.33 MB ℹ️ View Unchanged
|
…patibility status
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.
Works well, I added a few comments but those shouldn't be blockers 👍
@@ -603,6 +603,10 @@ private function update_enabled_payment_methods( WP_REST_Request $request ) { | |||
$payment_method_ids_to_enable = $request->get_param( 'enabled_payment_method_ids' ); | |||
$available_payment_methods = $this->wcpay_gateway->get_upe_available_payment_methods(); | |||
|
|||
if ( $request->has_param( 'is_manual_capture_enabled' ) && $request->get_param( 'is_manual_capture_enabled' ) ) { | |||
$payment_method_ids_to_enable = array_intersect( $payment_method_ids_to_enable, [ 'card', 'link' ] ); |
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.
A comment explaining why this is done and why are those payments methods selected would be helpful.
Or maybe just adding the array as a constant in Payment_Method
with a descriptive name could be enough.
$filtered_payment_methods = array_filter( | ||
$enabled_payment_methods, | ||
function ( $method ) { | ||
return in_array( $method, [ 'card', 'link' ], true ); |
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.
As mentioned in another file, should a constant be defined for this array ?
|
||
foreach ( $this->all_registered_gateways as $gateway ) { | ||
$stripe_id = $gateway->get_stripe_id(); | ||
if ( 'card' !== $stripe_id && 'link' !== $stripe_id ) { |
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.
Should the constants be used here?
Thanks for the suggestions @danielmx-dev. Rather adding a new constant just for manual capture payment methods, I've used used the constants already in place and added a comment. Let me know if that sounds alright with you. |
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 the changes, looks good!
Fixes #9363
Changes proposed in this Pull Request
The goal of this change is to prevent payment methods which are not displayed on checkout from causing incompatibility notices in the checkout block. A couple different things lead to this problem:
Auth + Capture
When manual capture is enabled, LPMs that were already enabled remain enabled even though they're hidden from checkout. This leads to notices about incompatibilities in the checkout block.
A couple solutions were considered:
registerPaymentMethod()
.I've went with the second solution. Since these payment methods shouldn't appear on checkout or in the editor, they should be disabled. To do this, I've tapped into the save settings call which updates the enabled payment methods and disables any incompatible payment methods which were previously enabled. The only payment methods compatible with manual capture are
card
andlink
.Inactive payment methods
Payment methods have a capability status, i.e.
active
,inactive
,pending
, orunrequested
. This status isn't taken into account when payment methods are being enabled. All non-active
status payment methods are hidden from checkout, yet the gateway is still enabled and the payment method id is added to the list of enabled payment methods -upe_enabled_payment_method_ids
. The solution here is to only enable payment methods that have theactive
status.Testing instructions
Auth + Capture
upe_enabled_payment_method_ids
option ( easy to view via Dev Tools ).Inactive payment methods
This requires getting a payment method into a non-active status. The best way I've found to do this is by onboarding a new site.
rejected
and therefore not added toupe_enabled_payment_method_ids
. Previously it would've been enabled.npm run changelog
to add a changelog file, choosepatch
to 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