@W-20508471: Implement PWA Adyen integration for Express#3640
@W-20508471: Implement PWA Adyen integration for Express#3640amittapalli merged 7 commits intot/team404/sfp-on-pwafrom
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
|
||
| const cardCaptureAutomatic = useAutomaticCapture() | ||
| const zoneId = useShopperConfiguration('zoneId') | ||
| const zoneId = paymentConfig?.zoneId |
There was a problem hiding this comment.
The payment sheet code has const zoneId = useShopperConfiguration('zoneId') which you removed. I don't know which one is right but I think they should match.
There was a problem hiding this comment.
I noticed useShopperConfiguration was always coming back undefined for me and I know that the payment config always has a value. So, I assumed its better to use the one that is bringing down the right value
| * @param {string} orderNo - The order number to fail | ||
| * @returns {Promise<boolean>} - true if failOrder succeeded and basket was reopened | ||
| */ | ||
| const attemptFailOrder = async (orderNo) => { |
There was a problem hiding this comment.
Could we move this to a shared place so payment sheet can call it too?
There was a problem hiding this comment.
Ya, we can. once we go back and start refactoring things a bit more. There is another WI to go over code we want to refactor since both the components are getting to a point where its not as readable. If I do it now, then any local refs that I am using would need to also get added to payment sheet and then we need to do regression on that too as part of the express work item.
| const createOrderAndUpdatePayment = async ( | ||
| basketId, | ||
| paymentType, | ||
| zoneIdValue, |
There was a problem hiding this comment.
Do we need to pass in a zone id if it's a const above?
There was a problem hiding this comment.
Ah, I will clean that up, don't think we need to pass that as a param.
| }, | ||
| ORDER_RECOVERY_FAILED: { | ||
| defaultMessage: | ||
| 'Order recovery failed. Please try again or select a different payment method.', |
There was a problem hiding this comment.
Will the basket have been reopened in this case? Not sure they can try again.
There was a problem hiding this comment.
That basket is gone when fail order fails looks like. So you are in this odd state where there is no basket and hence you see that skeleton. I didn't see a good way to reopen a basket from PWA without recreating a basket from scratch which seemed kind of risky if done from the client side. I mean if we go for this retry approach, then we need a way to recreate baskets that are just gone once an order is created. Another reason I also added basketIdRef in sf-payments-express, basically to prevent unmounting when basket becomes null during the failure scenarios
| } | ||
|
|
||
| const onCancel = async () => { | ||
| isPaymentInProgress.current = false |
There was a problem hiding this comment.
In past work on express it was hard but I was able to eliminate the need for a flag like this. The basket was the key to it all, which led to the annoying startConfirming/endConfirming calls. Is there something about Adyen express specifically which makes us need this flag?
There was a problem hiding this comment.
The way we had was working for Stripe and when I started to integrate Adyen, things started to unmount before I can even complete the process, especially when we are also trying to mitigate the fail order use-cases. startConfirming/endConfirming seem to stabilize the basket. But the main useEffect in Express has other dependencies that can change and perhaps changed for Adyen. If that effect reruns, it will cleanup and unmount things. Because we don't know what causes React to go down that route for different use-cases, maybe having the guard can help. I think we need to further test and see which of the culprits in the dependencies might be triggering the effect to rerun.
| } | ||
| updatedPaymentInstrument = getSFPaymentsInstrument(expressBasket.current) | ||
| } else { | ||
| // For Adyen: Update addresses from paymentData before creating order |
There was a problem hiding this comment.
I don't think we did this in SFRA for Adyen, is that a bug?
There was a problem hiding this comment.
Not sure of SFRA but Stripe also does it but it was running all that logic in the onPayerApprove action which I didn't see for Adyen in the SDK. The Order API validates that the basket has the following set already before it creates the order.
-Shipping address
-Billing address
-Payment instrument
| const paymentMethodSet = { | ||
| paymentMethods: paymentConfig.paymentMethods, | ||
| paymentMethodSetAccounts: paymentConfig.paymentMethodSetAccounts | ||
| // inject country code into payment method set accounts (Adyen seems to need it but Stripe works with/without it) |
There was a problem hiding this comment.
This is supposed to come from the Adyen account. It needs to be in the Adyen account config info coming back from ecom. Is it not there?
There was a problem hiding this comment.
Hmm, this is what I see in the SCAPI response for paymentMethodSetAccounts and not seeing the country the way the SDK expects. Are we missing something?
const initialTransactionInfo = {
countryCode: vendorAccount.config.country.toUpperCase(),
"paymentMethodSetAccounts": [ { "accountId": "SFAccount880ECOM", "config": { "key": "test_DJR5I4B4RJFEPMEO76L4AGNH6IXDCQH4", "paymentMethods": { "paymentMethods": [ { "brands": [ "amex", "cup", "diners", "discover", "mc", "visa" ], "name": "Cards", "type": "scheme" }, { "configuration": { "merchantId": "50", "gatewayMerchantId": "SFAccount880ECOM" }, "name": "Google Pay", "type": "googlepay" }, { "name": "Pay later with Klarna.", "type": "klarna" }, { "name": "Pay over time with Klarna.", "type": "klarna_account" } ] } }, "live": false, "vendor": "Adyen" }, { "accountId": "HZ74QTLCWMBBL", "config": { "bnCode": "SalesforceCommerceCloud_PPCP", "key": "AVzUrnC6x3Zb4l4PIZSP3yOJaO7SLm9q6zRsJ_2gL8fmzxK-UvJ4sFPi3f0x82U5TMuB4i_EYRRZ5woQ" }, "live": false, "vendor": "Paypal" } ],
| // this flag ensures data remains available even if the refetch completes before the cache update. | ||
| const {data: basket} = useCurrentBasket() | ||
|
|
||
| // Preserves the basketId in a ref so that the component stays mounted even after the basket is consumed. |
There was a problem hiding this comment.
Unless there's something specific for Adyen express we shouldn't need this.
There was a problem hiding this comment.
I will try to remove that
There was a problem hiding this comment.
Done. I realized that startConfirming() was not getting triggered in the right place for the Adyen flow. So fixed and removed the use of basketRef which was also essentially trying to keep the wrapper component from unmounting due to basket becoming null and the start/endconfirming handle it as you mentioned.
I am still keeping isPaymentInProgress around for a bit longer since that is being used to guard the useEffect in the buttons component from re-running
Support processing adyen payments and handle 409 errors for failure use-cases
Support processing adyen payments and handle 409 errors for failure use-cases
Support processing adyen payments and handle 409 errors for failure use-cases
3 participants
Description
Updated commerce-sdk-react with the latest isomorphic changes so we know we are testing newer updates. Note: some of these updates have already been pushed to develop branch and we can rebase it again once we pull from develop
Update Express Buttons component to process Payments for Adyen as well. Adyen does not confirm the payment client-side via the SDK. It only calls createPaymentIntent action
In addition Adyen does not call the pre approve action as Stripe does. So, changes include ensuring that the shipping, billing address, and basket creation happen from the createPaymentIntent itself just for Adyen.
Some of the Adyen updates caused some unnecessarily remounting of the component, which broke the payment process. So added some extra guards around that
Also addressed the Fail Order itself failing scenario. This can happen if a paymentError is dispatched because createPaymentIntent did not receive a response OR if the patch payment instrument on order API fails and fail order has to be called. Sometimes a race condition can cause the order to move from created -> new too soon. If that happens Fail Order itself fails. Once it fails, the basket is not recoverable. And the user just sees the checkout skeleton. Fix is to show an error and navigate the user away from the skeleton. If in checkout, take user to cart and if in pdp, no need to do anything
Note: There might be some changes to ssr.js and default.js etc which will not be pushed to develop
Types of Changes
Changes
How to Test-Drive This PR
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization