-
Notifications
You must be signed in to change notification settings - Fork 210
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
ACSS: Handle errors and edge cases #3870
base: develop
Are you sure you want to change the base?
Conversation
…merce/woocommerce-gateway-stripe into fix/3804-refactor-deferred-intent
Co-authored-by: César Costa <[email protected]>
Co-authored-by: César Costa <[email protected]>
…merce/woocommerce-gateway-stripe into fix/3804-refactor-deferred-intent
const paymentMethodTitle = | ||
getBlocksConfiguration()?.paymentMethodsConfig?.[ | ||
paymentMethodId | ||
]?.title ?? ''; | ||
setErrorMessage( | ||
error?.message ?? | ||
sprintf( | ||
// translators: %s is the payment method title. | ||
__( | ||
'Failed to load %s payment method. Please refresh the page and try again.', | ||
'woocommerce-gateway-stripe' | ||
), | ||
paymentMethodTitle | ||
) | ||
); |
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.
This is specific to non-deferred intent payment methods (ACSS and BLIK).
{ paymentProcessorLoadErrorMessage?.error?.message && ( | ||
<div className="wc-block-components-notices"> | ||
<StoreNotice status="error" isDismissible={ false }> | ||
{ paymentProcessorLoadErrorMessage.error.message } | ||
</StoreNotice> | ||
</div> | ||
) } |
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.
This approach is sourced from WooPayments (src).
This fixes any rendering issues due to minimum order amount or any other possible load error.
if ( hasLoadErrorRef.current ) { | ||
return { | ||
type: 'error', | ||
message: __( | ||
'Invalid or missing payment details. Please ensure the provided payment method is correctly entered.', | ||
'woocommerce-gateway-stripe' | ||
), | ||
}; | ||
} |
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.
This is also sourced from WooPayments (Automattic/woocommerce-payments#8908), and it fixes any potential form submissions in case the PE has failed to load.
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.
Is this related? #3991
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 didn't try to reproduce in this PR, but at first glance it doesn't seem related. This would only get triggered for Payment Element load errors, but the error from the issue seems like it's generated from the backend.
if ( | ||
isset( $error->code ) && | ||
( | ||
'payment_intent_mandate_invalid' === $error->code || // Don't retry payments when a 3DS mandate is invalid. | ||
'charge_exceeds_transaction_limit' === $error->code || // Don't retry payments when the charge exceeds the transaction limit. | ||
'amount_too_small' === $error->code | ||
) | ||
) { |
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.
FYI @asumaran @rafaelzaleski – In case you find any issues with retrials due to ACH/BACS specific errors, you can check and ignore the error in is_retryable_error
.
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.
LGTM and tests well! I went through all tests against shortcode and Block-based checkout and everything worked as expected!
- Test 1: Failing to load the Payment Element ✅
- Test 2: Minimum order amount error ✅
- Test 3: Failing account numbers ✅
const $container = jQuery( containerSelector ).first(); | ||
|
||
if ( ! $container.length ) { | ||
// If the container doesn't exist, do nothing. | ||
return; | ||
} |
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'd suggest moving this check to the beginning of the function as the entire logic depend on this container existing.
Fixes #3830
Changes proposed in this Pull Request:
showErrorPaymentMethod
) when the create intent request fails, or when there's aloaderror
in the payment element.hasLoadError
to classic and blocks to prevent form submissions if the payment method fails to load.showErrorCheckout
.charge_exceeds_transaction_limit
error from retrials.Setup
Testing Instructions
👉 Please test on both classic checkout and shortcode checkout.
Test 1: Failing to load the Payment Element
WC_Stripe_API::request( $request, 'test' );
or some corrupt request endpoint to test an error.Test 2: Minimum order amount error
Test 3: Failing account numbers
000
11000
000222222227
000
11000
900222222227
000
11000
000666666661
000
11000
000777777771
000
11000
000888888881
Test Steps:
{any_prefix}+test_email@{any_domain}
to receive the micro-deposit verification email.32
and45
.changelog.txt
andreadme.txt
(or does not apply)Post merge