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-4051 Apple Pay throws an error in checkout due to attempting to load Braintree information #2469

Merged
merged 5 commits into from
May 15, 2024
Merged

feat(payment): PAYPAL-4051 Apple Pay throws an error in checkout due to attempting to load Braintree information #2469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fc4026f
feat(payment): PAYPAL-4051 fix braintree data collector issue
bc-nick Apr 18, 2024
7956856
feat(payment): PAYPAL-4051 refactoring
bc-nick Apr 19, 2024
fd7ed91
feat(payment): PAYPAL-4051 tests
bc-nick Apr 19, 2024
028bebe
feat(payment): PAYPAL-4051 add todo
bc-nick May 7, 2024
7a2ae82
feat(payment): PAYPAL-4051 added JIRA
bc-nick May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
96 changes: 81 additions & 15 deletions packages/apple-pay-integration/src/apple-pay-customer-strategy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,28 +590,34 @@ describe('ApplePayCustomerStrategy', () => {
});

describe('braintree gateway', () => {
const getPaymentMethodCallback = (methodId: string) => {
if (methodId === 'applepay') {
const applePayPaymentMethod = getApplePay();

applePayPaymentMethod.initializationData.gateway = 'braintree';

return applePayPaymentMethod;
}

if (methodId === 'braintree') {
return getBraintree();
}

return {};
};

beforeEach(() => {
jest.spyOn(paymentIntegrationService, 'loadPaymentMethod').mockReturnValue(
paymentIntegrationService.getState(),
);
jest.spyOn(
paymentIntegrationService.getState(),
'getPaymentMethodOrThrow',
).mockImplementation((methodId) => {
if (methodId === 'applepay') {
const applePayPaymentMethod = getApplePay();

applePayPaymentMethod.initializationData.gateway = 'braintree';

return applePayPaymentMethod;
}

if (methodId === 'braintree') {
return getBraintree();
}

return {};
});
).mockImplementation(getPaymentMethodCallback);
jest.spyOn(
paymentIntegrationService.getState(),
'getPaymentMethod',
).mockImplementation(getPaymentMethodCallback);

jest.spyOn(braintreeIntegrationService, 'getClient').mockImplementation(
() => 'token',
Expand Down Expand Up @@ -666,6 +672,66 @@ describe('ApplePayCustomerStrategy', () => {
}
}
});

it('sends a payment without deviceSessionId in case the request braintree fails', async () => {
jest.spyOn(paymentIntegrationService, 'loadPaymentMethod').mockImplementation(
(methodId) => {
if (methodId === 'braintree') {
return Promise.reject();
}

return paymentIntegrationService.getState();
},
);

jest.spyOn(
paymentIntegrationService.getState(),
'getPaymentMethod',
).mockReturnValue(undefined);

const authEvent = {
payment: {
billingContact: getContactAddress(),
shippingContact: getContactAddress(),
token: {
paymentData: {},
paymentMethod: {},
transactionIdentifier: {},
},
},
} as ApplePayJS.ApplePayPaymentAuthorizedEvent;

const customerInitializeOptions = getApplePayCustomerInitializationOptions();

await strategy.initialize(customerInitializeOptions);

if (customerInitializeOptions.applepay) {
const buttonContainer = document.getElementById(
customerInitializeOptions.applepay.container,
);
const button = buttonContainer?.firstChild as HTMLElement;

if (button) {
button.click();
await applePaySession.onpaymentauthorized(authEvent);

expect(paymentIntegrationService.loadPaymentMethod).toHaveBeenCalledWith(
'braintree',
);
expect(paymentIntegrationService.submitPayment).toHaveBeenCalledWith(
expect.objectContaining({
paymentData: expect.objectContaining({
deviceSessionId: undefined,
}),
}),
);
expect(applePaySession.completePayment).toHaveBeenCalled();
expect(
customerInitializeOptions.applepay.onPaymentAuthorize,
).toHaveBeenCalled();
}
}
});
});

it('returns an error if autorize payment fails', async () => {
Expand Down
45 changes: 29 additions & 16 deletions packages/apple-pay-integration/src/apple-pay-customer-strategy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,29 +543,42 @@ export default class ApplePayCustomerStrategy implements CustomerStrategy {
}

private async _getBraintreeDeviceData() {
const data = await this._braintreeIntegrationService.getDataCollector();
const braintreePaymentMethod = this._paymentIntegrationService
.getState()
.getPaymentMethod(ApplePayGatewayType.BRAINTREE);

return data.deviceData;
if (braintreePaymentMethod?.clientToken) {
const data = await this._braintreeIntegrationService.getDataCollector();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way we can avoid BT calls?

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 can not avid it because we need to initialize BT to collect device data based on merchant configuration


return data.deviceData;
}
}

private async _initializeBraintreeIntegrationService() {
const state = await this._paymentIntegrationService.loadPaymentMethod(
ApplePayGatewayType.BRAINTREE,
);
try {
// TODO: temporary solution to initialize the braintree payment method to get a clientToken to create a braintree client instance
// TODO: this approach should be removed in the future
// TODO: Jira ticket for tracking purpose: https://bigcommercecloud.atlassian.net/browse/PAYPAL-4122
await this._paymentIntegrationService.loadPaymentMethod(ApplePayGatewayType.BRAINTREE);
Copy link
Contributor

Choose a reason for hiding this comment

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

This seemed to be called in initialize path, is there a reason why we need to load braintree payment method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we need to get a BT client instance to get collect device data

Copy link
Contributor

Choose a reason for hiding this comment

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

#2469 (comment)

Is this the same as here? Since we might be doing the same thing twice? Also maybe add a TODO with a JIRA for plans to fix this i.e. avoid making this call and maybe pass the data as part of apple pay config load.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@animesh1987 it's not the same. This is the best way to fix the issue at this point. It's a quick fix. I've already talked to our team about it, and it's going to take some time to fix it on the backend side. When another way will be found, the Braintree initialization process will be removed. At the moment we have some issue with generating clientToken on the backend side to pass it to the applepay configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

That seems ok, can we please add a TODO with JIRA to address the issue above so we have a plan.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@animesh1987 added TODO without JIRA. We need to create a ticket for backend developers with description. But they are aware of this problem

Copy link
Contributor

Choose a reason for hiding this comment

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

Please create a Jira ticket for tracking purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@animesh1987 @bc-peng i have updated PR with adding JIRA ticket


const storeConfig = state.getStoreConfigOrThrow();
const state = this._paymentIntegrationService.getState();

const braintreePaymentMethod: PaymentMethod = state.getPaymentMethodOrThrow(
ApplePayGatewayType.BRAINTREE,
);
const storeConfig = state.getStoreConfigOrThrow();

if (!braintreePaymentMethod.clientToken || !braintreePaymentMethod.initializationData) {
throw new MissingDataError(MissingDataErrorType.MissingPaymentMethod);
}
const braintreePaymentMethod: PaymentMethod = state.getPaymentMethodOrThrow(
ApplePayGatewayType.BRAINTREE,
);

this._braintreeIntegrationService.initialize(
braintreePaymentMethod.clientToken,
storeConfig,
);
if (!braintreePaymentMethod.clientToken || !braintreePaymentMethod.initializationData) {
throw new MissingDataError(MissingDataErrorType.MissingPaymentMethod);
}

this._braintreeIntegrationService.initialize(
braintreePaymentMethod.clientToken,
storeConfig,
);
} catch (_) {
// we don't need to do anything in this block
Copy link
Contributor

Choose a reason for hiding this comment

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

I see // we don't need to do anything in this block appears twice in the PR.
Could you please explain what would happen if an error occurs in the middle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bc-peng This will mean that the braintree payment method cannot be initialized because we have enabled the appropriate feature in the CP

}
}
}
83 changes: 69 additions & 14 deletions packages/apple-pay-integration/src/apple-pay-payment-strategy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,25 +185,32 @@ describe('ApplePayPaymentStrategy', () => {
});

describe('braintree gateway', () => {
const getPaymentMethodCallback = (methodId: string) => {
if (methodId === 'applepay') {
const applePayPaymentMethod = getApplePay();

applePayPaymentMethod.initializationData.gateway = 'braintree';

return applePayPaymentMethod;
}

if (methodId === 'braintree') {
return getBraintree();
}

return {};
};

beforeEach(() => {
jest.spyOn(
paymentIntegrationService.getState(),
'getPaymentMethodOrThrow',
).mockImplementation((methodId) => {
if (methodId === 'applepay') {
const applePayPaymentMethod = getApplePay();

applePayPaymentMethod.initializationData.gateway = 'braintree';

return applePayPaymentMethod;
}
).mockImplementation(getPaymentMethodCallback);

if (methodId === 'braintree') {
return getBraintree();
}

return {};
});
jest.spyOn(
paymentIntegrationService.getState(),
'getPaymentMethod',
).mockImplementation(getPaymentMethodCallback);

jest.spyOn(braintreeIntegrationService, 'getClient').mockImplementation(
() => 'token',
Expand Down Expand Up @@ -246,6 +253,54 @@ describe('ApplePayPaymentStrategy', () => {
}),
);
});

it('sends a payment without deviceSessionId in case the request braintree fails', async () => {
jest.spyOn(paymentIntegrationService, 'loadPaymentMethod').mockImplementation(
(methodId) => {
if (methodId === 'braintree') {
return Promise.reject();
}

return paymentIntegrationService.getState();
},
);

jest.spyOn(
paymentIntegrationService.getState(),
'getPaymentMethod',
).mockReturnValue(undefined);

await strategy.initialize({ methodId: 'applepay' });

const payload = merge({}, getOrderRequestBody(), {
payment: { methodId: paymentMethod.id },
});
const authEvent = {
payment: {
token: {
paymentData: {},
paymentMethod: {},
transactionIdentifier: {},
},
},
} as ApplePayJS.ApplePayPaymentAuthorizedEvent;

strategy.execute(payload);
await new Promise((resolve) => process.nextTick(resolve));
await applePaySession.onpaymentauthorized(authEvent);

expect(paymentIntegrationService.loadPaymentMethod).toHaveBeenCalledWith(
ApplePayGatewayType.BRAINTREE,
);

expect(paymentIntegrationService.submitPayment).toHaveBeenCalledWith(
expect.objectContaining({
paymentData: expect.objectContaining({
deviceSessionId: undefined,
}),
}),
);
});
});
});

Expand Down
42 changes: 26 additions & 16 deletions packages/apple-pay-integration/src/apple-pay-payment-strategy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,29 +267,39 @@ export default class ApplePayPaymentStrategy implements PaymentStrategy {
}

private async _getBraintreeDeviceData() {
const data = await this._braintreeIntegrationService.getDataCollector();
const braintreePaymentMethod = this._paymentIntegrationService
.getState()
.getPaymentMethod(ApplePayGatewayType.BRAINTREE);

return data.deviceData;
if (braintreePaymentMethod?.clientToken) {
const data = await this._braintreeIntegrationService.getDataCollector();

return data.deviceData;
}
}

private async _initializeBraintreeIntegrationService() {
const state = await this._paymentIntegrationService.loadPaymentMethod(
ApplePayGatewayType.BRAINTREE,
);
try {
await this._paymentIntegrationService.loadPaymentMethod(ApplePayGatewayType.BRAINTREE);

const storeConfig = state.getStoreConfigOrThrow();
const state = this._paymentIntegrationService.getState();

const braintreePaymentMethod: PaymentMethod = state.getPaymentMethodOrThrow(
ApplePayGatewayType.BRAINTREE,
);
const storeConfig = state.getStoreConfigOrThrow();

if (!braintreePaymentMethod.clientToken || !braintreePaymentMethod.initializationData) {
return;
}
const braintreePaymentMethod: PaymentMethod = state.getPaymentMethodOrThrow(
ApplePayGatewayType.BRAINTREE,
);

this._braintreeIntegrationService.initialize(
braintreePaymentMethod.clientToken,
storeConfig,
);
if (!braintreePaymentMethod.clientToken || !braintreePaymentMethod.initializationData) {
return;
}

this._braintreeIntegrationService.initialize(
braintreePaymentMethod.clientToken,
storeConfig,
);
} catch (_) {
// we don't need to do anything in this block
}
}
}