-
Notifications
You must be signed in to change notification settings - Fork 206
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
Add split UPE APMs (eg GiroPay) with deferred intent for Block Checkouts #2820
Add split UPE APMs (eg GiroPay) with deferred intent for Block Checkouts #2820
Conversation
Note that the changes on this PR go some way to laying the foundation for APMs on the shortcode checkout too. However none of them appear to work correctly. I want to take a look into that to see if there's a simple thing I'm missing that will make them work, or whether it will need to be a larger set of changes that should be handled by a separate PR. |
@@ -30,7 +30,7 @@ abstract class WC_Stripe_UPE_Payment_Method { | |||
* | |||
* @var string | |||
*/ | |||
protected $title; |
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.
Note that when I changed the WC_Stripe_UPE_Payment_Method
class to extend WC_Payment_Gateway
I had to make all these properties public to be consistant with that class.
…_upe_available_payment_methods()
…pending on Stripe Account country
It is good to mention that the Stripe account needs to be configured for an EU country here (took me some minutes to find out). |
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.
Tested all the cases and LGTM! Left a (possible) minor comment
// If the payment intent requires action, respond with the pi and client secret so it can confirmed on checkout. | ||
if ( 'requires_action' === $payment_intent->status ) { | ||
// If the payment intent requires action, respond with redirect URL or the pi and client secret so it can confirmed on checkout. | ||
if ( 'requires_action' === $payment_intent->status || 'requires_confirmation' === $payment_intent->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.
Hey @james-allan I'm curious if you remember what this change was needed for? Is there a specific gateway that has 'requires_confirmation' as the payment intent status after it has already been confirmed inside create_and_confirm_payment_intent()
?
I had to do somethign similar to this change in my PR for Boleto and Oxxo but I'm wondering if there's other gateways that need this.
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 I was working on this PR I needed to change this section so payment methods that need to redirect the user offsite respond to client with the correct redirect URL. Eg this change:
$redirect = $payment_intent->next_action->redirect_to_url->url;
When I was looking into how WooPayments handled this I noticed they had this additional status. I don't recall ever confirming if I received a payment intent status with requires_confirmation
though.
You needed it for Boleto and Oxxo though?
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.
I noticed you're changes in #2823.
I guess whichever one we merge first, we need to condense what that logic needs to look like.
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.
Ive consolidated that logic in 259ab86
ef06684
to
b60906f
Compare
Fixes #2790
Changes proposed in this Pull Request:
This PR adds support for all APMs (eg GiroPay) with split UPE and deferred intents.
To achieve this I had to do 2 main things:
WC_Payment_Gateway
s. This is required so that all our UPE methods get returned byget_available_payment_gateways()
and are registrable as a block checkout payment method withregisterPaymentMethod()
.Testing instructions
AT611904300234573201
) *AT861904300235473202
) *Important
* You will need to test SEPA with webhooks enabled as the final status transition (processing or failure) is handled via webhook.
changelog.txt
andreadme.txt
(or does not apply)Post merge