W-21372336: Addressing comments

Author: rasbhat

packages/template-retail-react-app/app/pages/checkout/payment-processing.jsx

@@ -90,7 +90,6 @@ const PaymentProcessing = () => {
 
     const isError = !isValidReturnUrl()
     const isHandled = useRef(false)
-    const failOrderCalledRef = useRef(false)
 
     async function handleAdyenRedirect() {
         // Find SF Payments payment instrument in order
@@ -134,7 +133,7 @@ const PaymentProcessing = () => {
      * @returns {Promise<void>}
      */
     async function attemptFailOrderForPayment() {
-        if (!orderNo || failOrderCalledRef.current) {
+        if (!orderNo) {
             return
         }
 
@@ -156,10 +155,11 @@ const PaymentProcessing = () => {
                     }
                 })
             }
-        } catch {
-            // Order may already be failed by webhook; avoid hanging
+        } catch (error) {
+            // Swallow so flow continues (invalidate, navigate). Causes: (1) Race: getOrder
+            // returned 'created' but webhook already failed the order, so failOrder fails. (2) getOrder,
+            // getTokenWhenReady, or failOrder threw error. Behavior for all: don't hang.
         } finally {
-            failOrderCalledRef.current = true
             queryClient.invalidateQueries()
         }
     }
@@ -210,11 +210,11 @@ const PaymentProcessing = () => {
                     duration: 30000
                 })
 
-                // Attempt to fail the order (no-op if already failed by webhook
+                // Attempt to fail the order (no-op if already failed by webhook)
                 await attemptFailOrderForPayment()
 
-                // Redirect to cart when payment fails
-                navigate('/cart')
+                // Navigate back to the checkout page to try again
+                navigate('/checkout')
             })()
         }
     }, [sfp, order])

packages/template-retail-react-app/app/pages/checkout/payment-processing.test.js

@@ -407,13 +407,13 @@ describe('PaymentProcessing', () => {
                 })
             })
 
-            test('navigates to cart on failed payment', async () => {
+            test('navigates back to checkout on failed payment', async () => {
                 mockHandleRedirect.mockResolvedValue({responseCode: 1})
 
                 renderWithProviders(<PaymentProcessing />)
 
                 await waitFor(() => {
-                    expect(mockNavigate).toHaveBeenCalledWith('/cart')
+                    expect(mockNavigate).toHaveBeenCalledWith('/checkout')
                 })
             })
 
@@ -440,24 +440,41 @@ describe('PaymentProcessing', () => {
                     })
                 })
 
-                expect(mockNavigate).toHaveBeenCalledWith('/cart')
+                expect(mockNavigate).toHaveBeenCalledWith('/checkout')
             })
 
-            test('does not call failOrder when order already failed by webhook (e.g. 3DS declined)', async () => {
+            test('does not call failOrder when order already failed by webhook', async () => {
                 mockHandleRedirect.mockResolvedValue({responseCode: 1})
                 mockGetOrder.mockResolvedValue({status: 'failed'})
 
                 renderWithProviders(<PaymentProcessing />)
 
                 await waitFor(() => {
                     expect(mockToast).toHaveBeenCalled()
-                    expect(mockNavigate).toHaveBeenCalledWith('/cart')
+                    expect(mockNavigate).toHaveBeenCalledWith('/checkout')
                 })
 
                 expect(mockGetOrder).toHaveBeenCalled()
                 expect(mockFailOrder).not.toHaveBeenCalled()
             })
 
+            test('shows toast and navigates to checkout when failOrder fails', async () => {
+                mockHandleRedirect.mockResolvedValue({responseCode: 1})
+                mockGetOrder.mockResolvedValue({status: 'created'})
+                mockFailOrder.mockRejectedValue(new Error('Order already failed'))
+
+                renderWithProviders(<PaymentProcessing />)
+
+                await waitFor(() => {
+                    expect(mockToast).toHaveBeenCalled()
+                    expect(mockNavigate).toHaveBeenCalledWith('/checkout')
+                })
+
+                expect(mockGetOrder).toHaveBeenCalled()
+                expect(mockFailOrder).toHaveBeenCalledTimes(1)
+                expect(mockInvalidateQueries).toHaveBeenCalled()
+            })
+
             test('handles different error response codes', async () => {
                 const errorCodes = [1, 2, -1, 999]
 
@@ -471,7 +488,7 @@ describe('PaymentProcessing', () => {
 
                     await waitFor(() => {
                         expect(mockToast).toHaveBeenCalled()
-                        expect(mockNavigate).toHaveBeenCalledWith('/cart')
+                        expect(mockNavigate).toHaveBeenCalledWith('/checkout')
                     })
                 }
             })
@@ -581,11 +598,11 @@ describe('PaymentProcessing', () => {
                 })
             })
 
-            test('navigates to cart on failed payment', async () => {
+            test('navigates back to checkout on failed payment', async () => {
                 renderWithProviders(<PaymentProcessing />)
 
                 await waitFor(() => {
-                    expect(mockNavigate).toHaveBeenCalledWith('/cart')
+                    expect(mockNavigate).toHaveBeenCalledWith('/checkout')
                 })
             })
 
@@ -610,7 +627,7 @@ describe('PaymentProcessing', () => {
                     })
                 })
 
-                expect(mockNavigate).toHaveBeenCalledWith('/cart')
+                expect(mockNavigate).toHaveBeenCalledWith('/checkout')
             })
         })
     })