feat(payment): PAYPAL-4051 Apple Pay throws an error in checkout due to attempting to load Braintree information #2469
Conversation
bc-nick
requested review from
serhii-tkachenko,
andriiVitvitskyi1990,
bc-dronov and
animesh1987
packages/apple-pay-integration/src/apple-pay-customer-strategy.ts
Outdated
Show resolved
Hide resolved
packages/apple-pay-integration/src/apple-pay-payment-strategy.ts
Outdated
Show resolved
Hide resolved
| storeConfig, | ||
| ); | ||
| } catch (_) { | ||
| return noop(); |
There was a problem hiding this comment.
It is not necessary to add noop here
There was a problem hiding this comment.
Added comment to prevent lint error for empty block
| storeConfig, | ||
| ); | ||
| } catch (_) { | ||
| return noop(); |
There was a problem hiding this comment.
It is not necessary to add noop here as well
|
|
||
| return data.deviceData; | ||
| if (braintreePaymentMethod?.clientToken) { | ||
| const data = await this._braintreeIntegrationService.getDataCollector(); |
There was a problem hiding this comment.
Is there a way we can avoid BT calls?
There was a problem hiding this comment.
We can not avid it because we need to initialize BT to collect device data based on merchant configuration
| ApplePayGatewayType.BRAINTREE, | ||
| ); | ||
| try { | ||
| await this._paymentIntegrationService.loadPaymentMethod(ApplePayGatewayType.BRAINTREE); |
There was a problem hiding this comment.
This seemed to be called in initialize path, is there a reason why we need to load braintree payment method.
There was a problem hiding this comment.
Because we need to get a BT client instance to get collect device data
There was a problem hiding this 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.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
That seems ok, can we please add a TODO with JIRA to address the issue above so we have a plan.
There was a problem hiding this comment.
@animesh1987 added TODO without JIRA. We need to create a ticket for backend developers with description. But they are aware of this problem
There was a problem hiding this comment.
Please create a Jira ticket for tracking purpose.
There was a problem hiding this comment.
@animesh1987 @bc-peng i have updated PR with adding JIRA ticket
| storeConfig, | ||
| ); | ||
| } catch (_) { | ||
| // we don't need to do anything in this block |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
@bc-peng This will mean that the braintree payment method cannot be initialized because we have enabled the appropriate feature in the CP
What?
Added
try {} catch {}to solve the applepay issue with braintree initialization process when credit card feature is disabledWhy?
To get the collect data the credit card feature must to be enabled otherwise the request for getting braintree data will be failed. In this case we can not be able to use braintree integration for collecting data because we do not have clientToken for correct initialization
This is a temporary solution for a quick fix
Testing / Proof
Manual testing
@bigcommerce/team-checkout @bigcommerce/team-payments