From 61ae351f301496b299fbe60d3a8111e0c2fe437f Mon Sep 17 00:00:00 2001 From: Daniel Guerra <15204776+danielmx-dev@users.noreply.github.com> Date: Wed, 14 Aug 2024 18:43:24 +0300 Subject: [PATCH] Fix: Handle payment methods errors in backend (#9226) Co-authored-by: Francesco --- ...77-handle-payment-method-errors-in-backend | 4 + client/checkout/blocks/payment-processor.js | 23 ++- .../blocks/test/payment-processor.test.js | 67 +++++++- client/checkout/classic/payment-processing.js | 39 +++-- .../classic/test/payment-processing.test.js | 147 +++++++++++++++++- client/checkout/constants.js | 1 + client/checkout/utils/upe.js | 19 +++ includes/class-payment-information.php | 45 +++++- includes/class-wc-payment-gateway-wcpay.php | 7 + tests/e2e-pw/playwright.config.ts | 4 +- tests/unit/test-class-payment-information.php | 38 +++++ .../test-class-wc-payment-gateway-wcpay.php | 12 ++ 12 files changed, 381 insertions(+), 25 deletions(-) create mode 100644 changelog/fix-9177-handle-payment-method-errors-in-backend diff --git a/changelog/fix-9177-handle-payment-method-errors-in-backend b/changelog/fix-9177-handle-payment-method-errors-in-backend new file mode 100644 index 00000000000..d69f8cab70b --- /dev/null +++ b/changelog/fix-9177-handle-payment-method-errors-in-backend @@ -0,0 +1,4 @@ +Significance: minor +Type: fix + +If a payment method fails to be created in the frontend during checkout, forward the errors to the server so it can be recorded in an order. diff --git a/client/checkout/blocks/payment-processor.js b/client/checkout/blocks/payment-processor.js index 1779c4b7b2c..88b72393524 100644 --- a/client/checkout/blocks/payment-processor.js +++ b/client/checkout/blocks/payment-processor.js @@ -27,6 +27,7 @@ import { useCustomerData } from './utils'; import enableStripeLinkPaymentMethod from 'wcpay/checkout/stripe-link'; import { getUPEConfig } from 'wcpay/utils/checkout'; import { validateElements } from 'wcpay/checkout/classic/payment-processing'; +import { PAYMENT_METHOD_ERROR } from 'wcpay/checkout/constants'; const getBillingDetails = ( billingData ) => { return { @@ -193,8 +194,26 @@ const PaymentProcessor = ( { if ( result.error ) { return { - type: 'error', - message: result.error.message, + // We return a `success` type even when there's an error since we want the checkout request to go + // through, so we can have this attempt recorded in an Order. + type: 'success', + meta: { + paymentMethodData: { + payment_method: + upeMethods[ paymentMethodId ], + 'wcpay-payment-method': PAYMENT_METHOD_ERROR, + 'wcpay-payment-method-error-code': + result.error.code, + 'wcpay-payment-method-error-decline-code': + result.error.decline_code, + 'wcpay-payment-method-error-message': + result.error.message, + 'wcpay-payment-method-error-type': + result.error.type, + 'wcpay-fraud-prevention-token': getFraudPreventionToken(), + 'wcpay-fingerprint': fingerprint, + }, + }, }; } diff --git a/client/checkout/blocks/test/payment-processor.test.js b/client/checkout/blocks/test/payment-processor.test.js index 2fad681cd40..c3ac18b4895 100644 --- a/client/checkout/blocks/test/payment-processor.test.js +++ b/client/checkout/blocks/test/payment-processor.test.js @@ -8,6 +8,7 @@ import { useEffect } from 'react'; */ import PaymentProcessor from '../payment-processor'; import { PaymentElement } from '@stripe/react-stripe-js'; +import { PAYMENT_METHOD_ERROR } from 'wcpay/checkout/constants'; jest.mock( 'wcpay/checkout/classic/payment-processing', () => ( { validateElements: jest.fn().mockResolvedValue(), @@ -153,10 +154,15 @@ describe( 'PaymentProcessor', () => { expect( mockCreatePaymentMethod ).not.toHaveBeenCalled(); } ); - it( 'should return an error when createPaymentMethod fails', async () => { + it( 'should return success with the error data when createPaymentMethod fails', async () => { let onPaymentSetupCallback; mockCreatePaymentMethod = jest.fn().mockResolvedValue( { - error: { message: 'Error creating payment method' }, + error: { + code: 'code', + decline_code: 'decline_code', + message: 'Error creating payment method', + type: 'card_error', + }, } ); act( () => { @@ -179,8 +185,61 @@ describe( 'PaymentProcessor', () => { expect( mockCreatePaymentMethod ).not.toHaveBeenCalled(); expect( await onPaymentSetupCallback() ).toEqual( { - type: 'error', - message: 'Error creating payment method', + type: 'success', + meta: { + paymentMethodData: { + payment_method: 'woocommerce_payments', + 'wcpay-payment-method': PAYMENT_METHOD_ERROR, + 'wcpay-payment-method-error-code': 'code', + 'wcpay-payment-method-error-decline-code': 'decline_code', + 'wcpay-payment-method-error-message': + 'Error creating payment method', + 'wcpay-payment-method-error-type': 'card_error', + 'wcpay-fraud-prevention-token': '', + 'wcpay-fingerprint': '', + }, + }, + } ); + expect( mockCreatePaymentMethod ).toHaveBeenCalled(); + } ); + + it( 'should return success when there are no failures', async () => { + let onPaymentSetupCallback; + mockCreatePaymentMethod = jest.fn().mockResolvedValue( { + paymentMethod: { + id: 'paymentMethodId', + }, + } ); + + act( () => { + render( + + ( onPaymentSetupCallback = callback ), + } } + fingerprint="" + shouldSavePayment={ false } + upeMethods={ { card: 'woocommerce_payments' } } + /> + ); + } ); + + expect( mockCreatePaymentMethod ).not.toHaveBeenCalled(); + expect( await onPaymentSetupCallback() ).toEqual( { + type: 'success', + meta: { + paymentMethodData: { + payment_method: 'woocommerce_payments', + 'wcpay-payment-method': 'paymentMethodId', + 'wcpay-fraud-prevention-token': '', + 'wcpay-fingerprint': '', + }, + }, } ); expect( mockCreatePaymentMethod ).toHaveBeenCalled(); } ); diff --git a/client/checkout/classic/payment-processing.js b/client/checkout/classic/payment-processing.js index ab10e4b5e6c..cfbd116b5d4 100644 --- a/client/checkout/classic/payment-processing.js +++ b/client/checkout/classic/payment-processing.js @@ -17,6 +17,7 @@ import { import { appendFraudPreventionTokenInputToForm, appendPaymentMethodIdToForm, + appendPaymentMethodErrorDataToForm, getPaymentMethodTypes, getSelectedUPEGatewayPaymentMethod, getTerms, @@ -29,7 +30,8 @@ import enableStripeLinkPaymentMethod, { import { SHORTCODE_SHIPPING_ADDRESS_FIELDS, SHORTCODE_BILLING_ADDRESS_FIELDS, -} from '../constants'; + PAYMENT_METHOD_ERROR, +} from 'wcpay/checkout/constants'; // It looks like on file import there are some side effects. Should probably be fixed. const gatewayUPEComponents = {}; @@ -192,14 +194,7 @@ function createStripePaymentMethod( return api .getStripeForUPE( paymentMethodType ) - .createPaymentMethod( { elements, params: params } ) - .then( ( paymentMethod ) => { - if ( paymentMethod.error ) { - throw paymentMethod.error; - } - - return paymentMethod; - } ); + .createPaymentMethod( { elements, params: params } ); } /** @@ -563,11 +558,20 @@ export const processPayment = ( $form, paymentMethodType ); + + if ( paymentMethodObject.error ) { + appendPaymentMethodIdToForm( $form, PAYMENT_METHOD_ERROR ); + appendPaymentMethodErrorDataToForm( + $form, + paymentMethodObject.error + ); + } else { + appendPaymentMethodIdToForm( + $form, + paymentMethodObject.paymentMethod.id + ); + } appendFingerprintInputToForm( $form, fingerprint ); - appendPaymentMethodIdToForm( - $form, - paymentMethodObject.paymentMethod.id - ); appendFraudPreventionTokenInputToForm( $form ); await additionalActionsHandler( paymentMethodObject.paymentMethod, @@ -587,6 +591,15 @@ export const processPayment = ( return false; }; +/** + * Used only for testing, resets the hasCheckoutCompleted value. + * + * @return {void} + */ +export function __resetHasCheckoutCompleted() { + hasCheckoutCompleted = false; +} + /** * Used only for testing, resets the gatewayUPEComponents internal cache of elements for a given property. * diff --git a/client/checkout/classic/test/payment-processing.test.js b/client/checkout/classic/test/payment-processing.test.js index 44a120fda50..7230e8790ff 100644 --- a/client/checkout/classic/test/payment-processing.test.js +++ b/client/checkout/classic/test/payment-processing.test.js @@ -7,13 +7,22 @@ import { processPayment, renderTerms, __resetGatewayUPEComponentsElement, + __resetHasCheckoutCompleted, } from '../payment-processing'; import { getAppearance } from '../../upe-styles'; import { getUPEConfig } from 'wcpay/utils/checkout'; -import { getFingerprint } from 'wcpay/checkout/utils/fingerprint'; +import { + getFingerprint, + appendFingerprintInputToForm, +} from 'wcpay/checkout/utils/fingerprint'; import showErrorCheckout from 'wcpay/checkout/utils/show-error-checkout'; import { waitFor } from '@testing-library/react'; -import { getSelectedUPEGatewayPaymentMethod } from 'wcpay/checkout/utils/upe'; +import { + appendPaymentMethodErrorDataToForm, + appendPaymentMethodIdToForm, + getSelectedUPEGatewayPaymentMethod, +} from 'wcpay/checkout/utils/upe'; +import { PAYMENT_METHOD_ERROR } from 'wcpay/checkout/constants'; jest.mock( '../../upe-styles' ); @@ -58,6 +67,7 @@ jest.mock( 'wcpay/utils/checkout', () => ( { jest.mock( 'wcpay/checkout/utils/fingerprint', () => ( { getFingerprint: jest.fn(), + appendFingerprintInputToForm: jest.fn(), } ) ); jest.mock( 'wcpay/checkout/utils/show-error-checkout', () => jest.fn() ); @@ -87,7 +97,11 @@ const mockElements = jest.fn( () => ( { submit: mockSubmit, } ) ); -const mockCreatePaymentMethod = jest.fn().mockResolvedValue( {} ); +const mockCreatePaymentMethod = jest.fn().mockResolvedValue( { + paymentMethod: { + id: 'paymentMethodId', + }, +} ); const apiMock = { saveUPEAppearance: jest.fn().mockResolvedValue( {} ), @@ -386,6 +400,7 @@ describe( 'Payment processing', () => { document.body.removeChild( element ); } ); + __resetHasCheckoutCompleted(); jest.clearAllMocks(); } ); @@ -619,6 +634,132 @@ describe( 'Payment processing', () => { } ); } ); + test( 'Payment processing adds the payment information to the form', async () => { + setupBillingDetailsFields(); + getFingerprint.mockImplementation( () => { + return { visitorId: 'fingerprint' }; + } ); + + const mockDomElement = document.createElement( 'div' ); + mockDomElement.dataset.paymentMethodType = 'card'; + + await mountStripePaymentElement( apiMock, mockDomElement ); + + const checkoutForm = { + submit: jest.fn(), + addClass: jest.fn( () => ( { + block: jest.fn(), + } ) ), + removeClass: jest.fn( () => ( { + unblock: jest.fn(), + submit: checkoutForm.submit, + } ) ), + attr: jest.fn().mockReturnValue( 'checkout' ), + }; + + mockCreatePaymentMethod.mockReturnValue( { + paymentMethod: { + id: 'paymentMethodId', + }, + } ); + + await processPayment( apiMock, checkoutForm, 'card' ); + // Wait for promises to resolve. + await new Promise( ( resolve ) => setImmediate( resolve ) ); + + expect( mockCreatePaymentMethod ).toHaveBeenCalledWith( { + elements: expect.any( Object ), + params: { + billing_details: expect.objectContaining( { + name: 'John Doe', + email: 'john.doe@example.com', + phone: '555-1234', + address: expect.any( Object ), + } ), + }, + } ); + + expect( appendPaymentMethodIdToForm ).toHaveBeenCalledWith( + checkoutForm, + 'paymentMethodId' + ); + + expect( appendFingerprintInputToForm ).toHaveBeenCalledWith( + checkoutForm, + 'fingerprint' + ); + + expect( checkoutForm.submit ).toHaveBeenCalled(); + } ); + + test( 'Payment processing adds the error information if payment method fails to be created', async () => { + setupBillingDetailsFields(); + getFingerprint.mockImplementation( () => { + return { visitorId: 'fingerprint' }; + } ); + + const mockDomElement = document.createElement( 'div' ); + mockDomElement.dataset.paymentMethodType = 'card'; + + await mountStripePaymentElement( apiMock, mockDomElement ); + + const checkoutForm = { + submit: jest.fn(), + addClass: jest.fn( () => ( { + block: jest.fn(), + } ) ), + removeClass: jest.fn( () => ( { + unblock: jest.fn(), + submit: checkoutForm.submit, + } ) ), + attr: jest.fn().mockReturnValue( 'checkout' ), + }; + + const errorData = { + code: 'code', + decline_code: 'decline_code', + message: 'message', + type: 'type', + }; + + mockCreatePaymentMethod.mockReturnValue( { + error: errorData, + } ); + + await processPayment( apiMock, checkoutForm, 'card' ); + // Wait for promises to resolve. + await new Promise( ( resolve ) => setImmediate( resolve ) ); + + expect( mockCreatePaymentMethod ).toHaveBeenCalledWith( { + elements: expect.any( Object ), + params: { + billing_details: expect.objectContaining( { + name: 'John Doe', + email: 'john.doe@example.com', + phone: '555-1234', + address: expect.any( Object ), + } ), + }, + } ); + + expect( appendPaymentMethodIdToForm ).toHaveBeenCalledWith( + checkoutForm, + PAYMENT_METHOD_ERROR + ); + + expect( appendPaymentMethodErrorDataToForm ).toHaveBeenCalledWith( + checkoutForm, + errorData + ); + + expect( appendFingerprintInputToForm ).toHaveBeenCalledWith( + checkoutForm, + 'fingerprint' + ); + + expect( checkoutForm.submit ).toHaveBeenCalled(); + } ); + function setupBillingDetailsFields() { // Create DOM elements for the test const firstNameInput = document.createElement( 'input' ); diff --git a/client/checkout/constants.js b/client/checkout/constants.js index 59bebcbac12..7da1460c913 100644 --- a/client/checkout/constants.js +++ b/client/checkout/constants.js @@ -17,6 +17,7 @@ export const PAYMENT_METHOD_NAME_EXPRESS_CHECKOUT_ELEMENT = 'woocommerce_payments_express_checkout'; export const PAYMENT_METHOD_NAME_WOOPAY_EXPRESS_CHECKOUT = 'woocommerce_payments_woopay_express_checkout'; +export const PAYMENT_METHOD_ERROR = 'woocommerce_payments_payment_method_error'; export const WC_STORE_CART = 'wc/store/cart'; export function getPaymentMethodsConstants() { diff --git a/client/checkout/utils/upe.js b/client/checkout/utils/upe.js index aa85be44436..c0ce9209cab 100644 --- a/client/checkout/utils/upe.js +++ b/client/checkout/utils/upe.js @@ -152,6 +152,25 @@ export const appendPaymentMethodIdToForm = ( $form, paymentMethodId ) => { ); }; +export const appendPaymentMethodErrorDataToForm = ( + $form, + paymentMethodError +) => { + [ + [ 'wcpay-payment-method-error-code', paymentMethodError.code ], + [ + 'wcpay-payment-method-error-decline-code', + paymentMethodError.decline_code, + ], + [ 'wcpay-payment-method-error-message', paymentMethodError.message ], + [ 'wcpay-payment-method-error-type', paymentMethodError.type ], + ].forEach( ( [ fieldName, value ] ) => { + $form.append( + `` + ); + } ); +}; + export const appendFraudPreventionTokenInputToForm = ( $form ) => { const fraudPreventionToken = window.wcpayFraudPreventionToken ?? ''; $form.append( diff --git a/includes/class-payment-information.php b/includes/class-payment-information.php index 2fd39f78739..f3cc00d85ef 100644 --- a/includes/class-payment-information.php +++ b/includes/class-payment-information.php @@ -21,6 +21,13 @@ * Mostly a wrapper containing information on a single payment. */ class Payment_Information { + /** + * Key used to indicate that an error occurred during the payment method creation in the client. + * + * @type string + */ + const PAYMENT_METHOD_ERROR = 'woocommerce_payments_payment_method_error'; + /** * The ID of the payment method used for this payment. * @@ -112,6 +119,13 @@ class Payment_Information { */ private $customer_id; + /** + * Will be set if there was an error during setup. + * + * @var ?\WP_Error + */ + private $error = null; + /** * Payment information constructor. * @@ -260,7 +274,16 @@ public static function from_payment_request( $order->add_meta_data( 'is_woopay', true, true ); $order->save_meta_data(); } - return new Payment_Information( $payment_method, $order, $payment_type, $token, $payment_initiated_by, $manual_capture, $cvc_confirmation, $fingerprint, $payment_method_stripe_id ); + $payment_information = new Payment_Information( $payment_method, $order, $payment_type, $token, $payment_initiated_by, $manual_capture, $cvc_confirmation, $fingerprint, $payment_method_stripe_id ); + + if ( self::PAYMENT_METHOD_ERROR === $payment_method ) { + $error_message = $request['wcpay-payment-method-error-message'] ?? __( "We're not able to process this payment. Please try again later.", 'woocommerce-payments' ); + $error_code = $request['wcpay-payment-method-error-code'] ?? 'unknown-error'; + $error = new \WP_Error( $error_code, $error_message ); + $payment_information->set_error( $error ); + } + + return $payment_information; } /** @@ -455,4 +478,24 @@ public function get_payment_method_stripe_id() { public function get_customer_id() { return $this->customer_id; } + + + /** + * Sets the error data. + * + * @param \WP_Error $error The error to be set. + * @return void + */ + public function set_error( \WP_Error $error ) { + $this->error = $error; + } + + /** + * Returns the error data. + * + * @return ?\WP_Error + */ + public function get_error() { + return $this->error; + } } diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index 7652281c860..24dc0686c0f 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -1495,6 +1495,13 @@ public function process_payment_for_order( $cart, $payment_information, $schedul $this->order_service->set_customer_id_for_order( $order, $customer_id ); $order->update_meta_data( WC_Payments_Order_Service::WCPAY_MODE_META_KEY, WC_Payments::mode()->is_test() ? Order_Mode::TEST : Order_Mode::PRODUCTION ); + // If an error happened during the payment setup in the client it will be saved in the payment information so we can throw + // the error here and follow the standard failed order flow. + $error = $payment_information->get_error(); + if ( ! is_null( $error ) ) { + throw new \Exception( $error->get_error_message() ); + } + // In case amount is 0 and we're not saving the payment method, we won't be using intents and can confirm the order payment. if ( apply_filters( 'wcpay_confirm_without_payment_intent', ! $payment_needed && ! $save_payment_method_to_store ) ) { $order->payment_complete(); diff --git a/tests/e2e-pw/playwright.config.ts b/tests/e2e-pw/playwright.config.ts index 74ab435eb9a..cece169c973 100644 --- a/tests/e2e-pw/playwright.config.ts +++ b/tests/e2e-pw/playwright.config.ts @@ -36,11 +36,11 @@ export default defineConfig( { video: 'on-first-retry', viewport: { width: 1280, height: 720 }, }, - timeout: 60 * 1000, // Default is 30s, somteimes it is not enough for local tests due to long setup. + timeout: 120 * 1000, // Default is 30s, somteimes it is not enough for local tests due to long setup. expect: { toHaveScreenshot: { maxDiffPixelRatio: 0.025 }, //=* Increase expect timeout to 10 seconds. See https://playwright.dev/docs/test-timeouts#set-expect-timeout-in-the-config.*/ - timeout: 10 * 1000, + timeout: 20 * 1000, }, snapshotPathTemplate: '{testDir}/__snapshots__/{testFilePath}/{arg}{ext}', diff --git a/tests/unit/test-class-payment-information.php b/tests/unit/test-class-payment-information.php index 3067e1c13b2..219eedac6f9 100644 --- a/tests/unit/test-class-payment-information.php +++ b/tests/unit/test-class-payment-information.php @@ -203,6 +203,7 @@ public function test_from_payment_request_with_token() { $this->assertTrue( $payment_information->is_using_saved_payment_method() ); $this->assertEquals( $this->card_token, $payment_information->get_payment_token() ); $this->assertTrue( $payment_information->is_merchant_initiated() ); + $this->assertNull( $payment_information->get_error() ); } public function test_from_payment_request_without_token() { @@ -212,6 +213,43 @@ public function test_from_payment_request_without_token() { $this->assertEquals( self::PAYMENT_METHOD, $payment_information->get_payment_method() ); $this->assertFalse( $payment_information->is_using_saved_payment_method() ); $this->assertFalse( $payment_information->is_merchant_initiated() ); + $this->assertNull( $payment_information->get_error() ); + } + + public function test_from_payment_request_with_error() { + $payment_information = Payment_Information::from_payment_request( + [ + 'payment_method' => CC_Payment_Gateway::GATEWAY_ID, + self::PAYMENT_METHOD_REQUEST_KEY => Payment_Information::PAYMENT_METHOD_ERROR, + 'wcpay-payment-method-error-message' => 'Invalid Card', + 'wcpay-payment-method-error-code' => 'invalid_card', + ] + ); + $this->assertEquals( Payment_Information::PAYMENT_METHOD_ERROR, $payment_information->get_payment_method() ); + $this->assertFalse( $payment_information->is_using_saved_payment_method() ); + $this->assertFalse( $payment_information->is_merchant_initiated() ); + + $error = $payment_information->get_error(); + $this->assertInstanceOf( \WP_Error::class, $error ); + $this->assertEquals( 'Invalid Card', $error->get_error_message() ); + $this->assertEquals( 'invalid_card', $error->get_error_code() ); + } + + public function test_from_payment_request_with_error_no_details() { + $payment_information = Payment_Information::from_payment_request( + [ + 'payment_method' => CC_Payment_Gateway::GATEWAY_ID, + self::PAYMENT_METHOD_REQUEST_KEY => Payment_Information::PAYMENT_METHOD_ERROR, + ] + ); + $this->assertEquals( Payment_Information::PAYMENT_METHOD_ERROR, $payment_information->get_payment_method() ); + $this->assertFalse( $payment_information->is_using_saved_payment_method() ); + $this->assertFalse( $payment_information->is_merchant_initiated() ); + + $error = $payment_information->get_error(); + $this->assertInstanceOf( \WP_Error::class, $error ); + $this->assertEquals( "We're not able to process this payment. Please try again later.", $error->get_error_message() ); + $this->assertEquals( 'unknown-error', $error->get_error_code() ); } public function test_get_cvc_confirmation_from_request_returns_null_if_payment_method_is_empty() { diff --git a/tests/unit/test-class-wc-payment-gateway-wcpay.php b/tests/unit/test-class-wc-payment-gateway-wcpay.php index dbc0e81876b..1c61bfc5046 100644 --- a/tests/unit/test-class-wc-payment-gateway-wcpay.php +++ b/tests/unit/test-class-wc-payment-gateway-wcpay.php @@ -2572,6 +2572,18 @@ public function test_process_payment_for_order_rejects_with_cached_minimum_amoun $this->card_gateway->process_payment_for_order( WC()->cart, $pi ); } + public function test_process_payment_for_order_rejects_if_the_payment_information_has_an_error() { + set_transient( 'wcpay_minimum_amount_usd', '50', DAY_IN_SECONDS ); + + $order = WC_Helper_Order::create_order(); + $pi = new Payment_Information( 'pm_test', $order, null, null, null, null, null, '', 'card' ); + $pi->set_error( new \WP_Error( 'invalid_card', 'Invalid Card' ) ); + + $this->expectException( \Exception::class ); + $this->expectExceptionMessage( 'Invalid Card' ); + $this->card_gateway->process_payment_for_order( WC()->cart, $pi ); + } + public function test_process_payment_for_order_rejects_with_order_id_mismatch() { $order = WC_Helper_Order::create_order(); $intent_meta_order_id = 0;