Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(payment): PAYPAL-3520 Max capture amount failure #2485

Merged
merged 3 commits into from
Jun 4, 2024
Merged

feat(payment): PAYPAL-3520 Max capture amount failure #2485

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
07745bd
feat(payment): PAYPAL-3520 instrument_declined error handling
bc-nick Apr 25, 2024
eea19f1
feat(payment): PAYPAL-3520 added paypal button widget
bc-nick May 31, 2024
9bd99af
feat(payment): PAYPAL-3520 updates after review
bc-nick Jun 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/braintree-integration/.eslintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"@typescript-eslint/consistent-type-assertions": "off",
"jest/no-conditional-expect": "off",
"@typescript-eslint/no-unsafe-call": "off",
"@typescript-eslint/no-empty-function": "off"
"@typescript-eslint/no-empty-function": "off",
"@typescript-eslint/no-throw-literal": "off"
}
}
]
Expand Down
5 changes: 5 additions & 0 deletions ...braintree-integration/src/braintree-paypal/braintree-paypal-payment-initialize-options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ export interface BraintreePaypalPaymentInitializeOptions {
* The location to insert the Pay Later Messages.
*/
bannerContainerId?: string;

/**
* A callback for displaying error popup. This callback requires error object as parameter.
*/
onError?(error: unknown): void;
}

export interface WithBraintreePaypalPaymentInitializeOptions {
Expand Down
55 changes: 55 additions & 0 deletions ...ages/braintree-integration/src/braintree-paypal/braintree-paypal-payment-strategy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
} from '@bigcommerce/checkout-sdk/payment-integration-api';
import {
getCart,
getConfig,
getOrderRequestBody,
getShippingAddress,
PaymentIntegrationServiceMock,
Expand All @@ -53,6 +54,19 @@ describe('BraintreePaypalPaymentStrategy', () => {
let paypalSdkMock: PaypalSDK;
let paypalMessageElement: HTMLDivElement;

const storeConfig = getConfig().storeConfig;

const storeConfigWithFeaturesOn = {
...storeConfig,
checkoutSettings: {
...storeConfig.checkoutSettings,
features: {
...storeConfig.checkoutSettings.features,
'PAYPAL-3521.handling_declined_error_braintree': true,
},
},
};

beforeEach(() => {
paypalMessageElement = document.createElement('div');
paypalMessageElement.id = 'banner-container-id';
Expand All @@ -77,6 +91,7 @@ describe('BraintreePaypalPaymentStrategy', () => {
paymentIntegrationService.getState(),
'getOutstandingBalance',
).mockImplementation((useStoreCredit) => (useStoreCredit ? 150 : 190));
jest.spyOn(paymentIntegrationService, 'submitPayment').mockReturnValue(undefined);

braintreeScriptLoader = new BraintreeScriptLoader(getScriptLoader(), window);
jest.spyOn(braintreeScriptLoader, 'initialize').mockReturnValue(undefined);
Expand Down Expand Up @@ -416,6 +431,46 @@ describe('BraintreePaypalPaymentStrategy', () => {
}
});

it('throws specific error if receive INSTRUMENT_DECLINED error and experiment is on', async () => {
const providerError = {
errors: [
{
code: 'transaction_declined',
message: 'Payment was declined. Please try again.',
provider_error: {
code: '2046',
},
},
],
};

jest.spyOn(
paymentIntegrationService.getState(),
'getStoreConfigOrThrow',
).mockReturnValue(storeConfigWithFeaturesOn);

jest.spyOn(paymentIntegrationService, 'submitPayment').mockImplementation(() => {
throw providerError;
});

const braintreeOptions = {
...options,
braintree: {
onError: jest.fn(),
},
};

await strategy.initialize(braintreeOptions);

try {
await strategy.execute(orderRequestBody, options);
} catch (error) {
expect(braintreeOptions.braintree.onError).toHaveBeenCalledWith(
new Error('INSTRUMENT_DECLINED'),
);
}
});

describe('when paying with a vaulted instrument', () => {
beforeEach(() => {
orderRequestBody = {
Expand Down
32 changes: 32 additions & 0 deletions packages/braintree-integration/src/braintree-paypal/braintree-paypal-payment-strategy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
PaymentStrategy,
PaypalInstrument,
} from '@bigcommerce/checkout-sdk/payment-integration-api';
import { isPaypalCommerceProviderError } from '@bigcommerce/checkout-sdk/paypal-commerce-utils';

import isBraintreeError from '../is-braintree-error';
import mapToBraintreeShippingAddressOverride from '../map-to-braintree-shipping-address-override';
Expand All @@ -40,6 +41,7 @@ import {
export default class BraintreePaypalPaymentStrategy implements PaymentStrategy {
private paymentMethod?: PaymentMethod;
private braintreeHostWindow: BraintreeHostWindow = window;
private braintree?: BraintreePaypalPaymentInitializeOptions;

constructor(
private paymentIntegrationService: PaymentIntegrationService,
Expand All @@ -51,6 +53,8 @@ export default class BraintreePaypalPaymentStrategy implements PaymentStrategy {
) {
const { braintree: braintreeOptions, methodId } = options;

this.braintree = braintreeOptions;

if (!this.paymentMethod || !this.paymentMethod.nonce) {
this.paymentMethod = this.paymentIntegrationService
.getState()
Expand Down Expand Up @@ -85,6 +89,12 @@ export default class BraintreePaypalPaymentStrategy implements PaymentStrategy {
async execute(orderRequest: OrderRequestBody, options?: PaymentRequestOptions): Promise<void> {
const { payment, ...order } = orderRequest;

const { onError } = this.braintree || {};
const state = this.paymentIntegrationService.getState();
const features = state.getStoreConfigOrThrow().checkoutSettings.features;
const shouldHandleInstrumentDeclinedError =
features && features['PAYPAL-3521.handling_declined_error_braintree'];

if (!payment) {
throw new PaymentArgumentInvalidError(['payment']);
}
Expand All @@ -95,6 +105,18 @@ export default class BraintreePaypalPaymentStrategy implements PaymentStrategy {
await this.paymentIntegrationService.submitOrder(order, options);
await this.paymentIntegrationService.submitPayment(paymentData);
} catch (error) {
if (this.isProviderError(error) && shouldHandleInstrumentDeclinedError) {
await this.loadPaypal();

await new Promise((_resolve, reject) => {
if (onError && typeof onError === 'function') {
onError(new Error('INSTRUMENT_DECLINED'));
}

reject();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to reject promise in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to do this to avoid the execution code freeze. Code will not execute correctly without reject calling

});
Comment on lines +125 to +131
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really see why do we need this promise.. To avoid code freeze we can simply remove the promise, since there is no reason to wait for something.

Suggested change
await new Promise((_resolve, reject) => {
if (onError && typeof onError === 'function') {
onError(new Error('INSTRUMENT_DECLINED'));
}
reject();
});
if (onError && typeof onError === 'function') {
onError(new Error('INSTRUMENT_DECLINED'));
}

}

this.handleError(error);
}
}
Expand Down Expand Up @@ -316,4 +338,14 @@ export default class BraintreePaypalPaymentStrategy implements PaymentStrategy {

throw new PaymentMethodFailedError(error.message);
}

private isProviderError(error: unknown): boolean {
if (isPaypalCommerceProviderError(error)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not use paypal commerce related code inside brainteree strategy. Please create another typeguard for that.

const paypalProviderError = error?.errors?.filter((e: any) => e.provider_error) || [];

return paypalProviderError[0].provider_error?.code === '2046';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better to create a separate constant for '2046' error? How do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late response. I think it is not necessary, we use this code only here and nowhere else

}

return false;
}
}
5 changes: 5 additions & 0 deletions packages/core/src/payment/strategies/braintree/braintree-payment-options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ export interface BraintreePaymentInitializeOptions {
* The location to insert the Pay Later Messages.
*/
bannerContainerId?: string;

/**
* A callback for displaying error popup. This callback requires error object as parameter.
*/
onError?(error: unknown): void;
}

/**
Expand Down