[INTER-3851] Braintree Paypal buttons #35
Conversation
BoldCole
commented
Jun 18, 2024
BoldCole
commented
- Implement paypal buttons for braintree paypal
src/braintree/getBraintreeJsUrls.ts
Outdated
| } = braintreeConstants; | ||
| version ??= jsVersion; | ||
| const clientJsURL = `${base}/${version}/${clientJs}`; | ||
| const appleJsURL = `${base}/${version}/${appleJs}`; | ||
| const braintreeGoogleJsURL = `${base}/${version}/${googleJs}`; | ||
| const dataCollectorJsURL = `${base}/${version}/${dataCollectorJs}`; | ||
| const fastlaneJsURL = `${base}/${version}/${fastlaneJs}`; | ||
| const paypalCheckoutURL = `${base}/${version}/${paypalCheckoutUrl}`; |
There was a problem hiding this comment.
Having two variable names that only differ in their case, seems like asking for trouble. My two bits.
|
|
||
| interface IEstimateShippingLinesResponse { | ||
| // Yes this is the correct key | ||
| 'shipping options': IShippingLine[]; |
There was a problem hiding this comment.
Is this formatted key coming from checkout or the platform connector?
| return paypalCheckout.createPayment({ | ||
| currency: getCurrency().iso_code, | ||
| flow: 'checkout', | ||
| amount: totals.totalAmountDue / 100, |
There was a problem hiding this comment.
Should we have a currency helper here? Is this going to cause off by one cent issues?
| const shippingAddress = result.details.shippingAddress; | ||
| const billingAddress = result.details.billingAddress; | ||
| const shippingCountry = shippingAddress ? countries.find(c => c.iso_code === shippingAddress.countryCode) : null; | ||
| const billingCountry = countries.find(c => c.iso_code === billingAddress.countryCode); |
There was a problem hiding this comment.
Are we guarenteed to have a billing address here? Or should you add a null check for billing address like you did for shipping address?
There was a problem hiding this comment.
Yes, there MUST be a billing address
| const shippingOption = order.purchase_units[0].shipping?.options?.find(o => o.selected); | ||
|
|
||
| /* istanbul ignore next */ | ||
| if (!billingProvince || !billingCountry) { |
There was a problem hiding this comment.
what if shippingProvince or shippingCountry are undefined? Do we need to throw an error, or is it okay to continue with empty values?
There was a problem hiding this comment.
It's okay for those values to be empty in the case where shipping is not required for the cart.
| const paypalButtonDiv = document.createElement('div'); | ||
| const paypalButtonDivId = 'ppcp-paypal-express-payment-button'; | ||
| paypalButtonDiv.id = paypalButtonDivId; | ||
| paypalButtonDiv.dataset.testid = paypalButtonDivId; |
There was a problem hiding this comment.
Can you add the class name express-payment to both the paypal and paylater buttons so they match the existing express pay buttons. This was recently an issue with the ppcp buttons: #34
