Skip to content

W-21372336: Fail Order for redirect based payments #3707

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

Merged
rasbhat merged 3 commits into from
Mar 3, 2026
Merged

W-21372336: Fail Order for redirect based payments #3707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
80ca151
W-21372336: Fail Order for redirect based payments
rasbhat Mar 3, 2026
4349bc7
W-21372336: Addressing comments
rasbhat Mar 3, 2026
5f0e22f
W-21372336: Addressing comments
rasbhat Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 36 additions & 13 deletions packages/template-retail-react-app/app/pages/checkout/payment-processing.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {Heading, Stack, Text} from '@salesforce/retail-react-app/app/components/
import Link from '@salesforce/retail-react-app/app/components/link'

import {useOrder, useShopperOrdersMutation} from '@salesforce/commerce-sdk-react'
import {useQueryClient} from '@tanstack/react-query'
import useNavigation from '@salesforce/retail-react-app/app/hooks/use-navigation'
import {useSFPayments, STATUS_SUCCESS} from '@salesforce/retail-react-app/app/hooks/use-sf-payments'
import {getSFPaymentsInstrument} from '@salesforce/retail-react-app/app/utils/sf-payments-utils'
Expand Down Expand Up @@ -42,6 +43,7 @@ const PaymentProcessing = () => {
const navigate = useNavigation()
const {sfp} = useSFPayments()
const toast = useToast()
const queryClient = useQueryClient()

const {mutateAsync: updatePaymentInstrumentForOrder} = useShopperOrdersMutation(
'updatePaymentInstrumentForOrder'
Expand All @@ -51,7 +53,7 @@ const PaymentProcessing = () => {
const params = new URLSearchParams(location.search)
const vendor = params.get('vendor')
const orderNo = params.get('orderNo')
const {data: order} = useOrder(
const {data: order, refetch} = useOrder(
{
parameters: {orderNo}
},
Expand Down Expand Up @@ -117,16 +119,37 @@ const PaymentProcessing = () => {
)
}

async function failOrderForPayment() {
await failOrder({
parameters: {
orderNo,
reopenBasket: true
},
body: {
reasonCode: 'payment_confirm_failure'
/**
* Attempts to fail an order and reopen the basket.
* Only calls failOrder if the order status is 'created' (avoids hanging when order
* was already failed by webhook).
* @returns {Promise<void>}
*/
async function attemptFailOrderForPayment() {
if (!orderNo) {
return
}

try {
const {data: currentOrder} = await refetch()
if (currentOrder?.status === 'created') {
await failOrder({
parameters: {
orderNo,
reopenBasket: true
},
body: {
reasonCode: 'payment_confirm_failure'
}
})
}
})
} catch (error) {
// Swallow so flow continues (invalidate, navigate). Causes: (1) Race: refetch
Copy link
Collaborator

@jeffraab-sfdc jeffraab-sfdc Mar 3, 2026

Choose a reason for hiding this comment

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

Thanks for this comment. IMO the fact the framework level design decisions bring us to this scenario where swallowing an error is the right path, implicates those decisions not our code as a consumer. I'm ok with this.

Copy link
Contributor

@amittapalli amittapalli Mar 3, 2026

Choose a reason for hiding this comment

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

If there was a way to recover the basket even when fail order fails (and not just when it passes) then it might be consistent. That reopen flag isn't really reopening in a failure case.

The only difference is Express is throwing some toast, so doing nothing in catch is fine here
const basketRecovered = await attemptFailOrder(createdOrderNo)
if (basketRecovered) {
showErrorMessage(ERROR_MESSAGE_KEYS.FAIL_ORDER)
} else {
showErrorMessage(ERROR_MESSAGE_KEYS.ORDER_RECOVERY_FAILED)
if (usage !== EXPRESS_BUY_NOW) {
navigate('/cart')
}
}

// returned 'created' but webhook already failed the order, so failOrder fails. (2) refetch
// or failOrder threw (network, 4xx/5xx). Same behavior for all: don't hang.
} finally {
queryClient.invalidateQueries()
}
}

function showOrderConfirmation() {
Expand All @@ -139,7 +162,7 @@ const PaymentProcessing = () => {
isHandled.current = true

// Order exists but payment can't be processed for return URL
failOrderForPayment()
attemptFailOrderForPayment()
} else if (!isError && sfp && order) {
;(async () => {
if (isHandled.current) {
Expand Down Expand Up @@ -175,8 +198,8 @@ const PaymentProcessing = () => {
duration: 30000
})

// Attempt to fail the order
await failOrderForPayment()
// Attempt to fail the order (no-op if already failed by webhook, e.g. 3DS declined)
await attemptFailOrderForPayment()

// Navigate back to the checkout page to try again
navigate('/checkout')
Expand Down
97 changes: 95 additions & 2 deletions packages/template-retail-react-app/app/pages/checkout/payment-processing.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ const mockUseOrder = jest.fn()
const mockUpdatePaymentInstrumentForOrder = jest.fn()
const mockFailOrder = jest.fn()
const mockGetSFPaymentsInstrument = jest.fn()
const mockRefetchOrder = jest.fn()
const mockInvalidateQueries = jest.fn()

jest.mock('@salesforce/retail-react-app/app/hooks/use-navigation', () => ({
__esModule: true,
Expand Down Expand Up @@ -53,6 +55,16 @@ jest.mock('@salesforce/commerce-sdk-react', () => {
}
})

jest.mock('@tanstack/react-query', () => {
const actual = jest.requireActual('@tanstack/react-query')
return {
...actual,
useQueryClient: () => ({
invalidateQueries: mockInvalidateQueries
})
}
})

jest.mock('@salesforce/retail-react-app/app/utils/sf-payments-utils', () => ({
getSFPaymentsInstrument: () => mockGetSFPaymentsInstrument()
}))
Expand Down Expand Up @@ -83,8 +95,13 @@ describe('PaymentProcessing', () => {

mockUseOrder.mockReturnValue({
data: {
orderNo: '12345'
}
orderNo: '12345',
status: 'created'
},
refetch: mockRefetchOrder
})
mockRefetchOrder.mockResolvedValue({
data: {orderNo: '12345', status: 'created'}
})

mockUpdatePaymentInstrumentForOrder.mockReturnValue({})
Expand Down Expand Up @@ -196,6 +213,13 @@ describe('PaymentProcessing', () => {

test('renders error message for invalid Adyen URL missing type', async () => {
mockLocation.search = '?vendor=Adyen&orderNo=12345&zoneId=default&redirectResult=ABC123'
mockUseOrder.mockReturnValue({
data: {orderNo: '12345'},
refetch: mockRefetchOrder
})
mockRefetchOrder.mockResolvedValue({
data: {orderNo: '12345', status: 'created'}
})

renderWithProviders(<PaymentProcessing />)

Expand All @@ -205,6 +229,7 @@ describe('PaymentProcessing', () => {
expect(screen.getByText('Return to Checkout')).toBeInTheDocument()

await waitFor(() => {
expect(mockRefetchOrder).toHaveBeenCalled()
expect(mockFailOrder).toHaveBeenCalledTimes(1)
expect(mockFailOrder).toHaveBeenCalledWith({
parameters: {
Expand All @@ -220,6 +245,13 @@ describe('PaymentProcessing', () => {

test('renders error message for invalid Adyen URL missing zone id', async () => {
mockLocation.search = '?vendor=Adyen&orderNo=12345&type=klarna&redirectResult=ABC123'
mockUseOrder.mockReturnValue({
data: {orderNo: '12345'},
refetch: mockRefetchOrder
})
mockRefetchOrder.mockResolvedValue({
data: {orderNo: '12345', status: 'created'}
})

renderWithProviders(<PaymentProcessing />)

Expand All @@ -229,6 +261,7 @@ describe('PaymentProcessing', () => {
expect(screen.getByText('Return to Checkout')).toBeInTheDocument()

await waitFor(() => {
expect(mockRefetchOrder).toHaveBeenCalled()
expect(mockFailOrder).toHaveBeenCalledTimes(1)
expect(mockFailOrder).toHaveBeenCalledWith({
parameters: {
Expand All @@ -244,6 +277,13 @@ describe('PaymentProcessing', () => {

test('renders error message for invalid Adyen URL missing redirect result', async () => {
mockLocation.search = '?vendor=Adyen&orderNo=12345&type=klarna&zoneId=default'
mockUseOrder.mockReturnValue({
data: {orderNo: '12345'},
refetch: mockRefetchOrder
})
mockRefetchOrder.mockResolvedValue({
data: {orderNo: '12345', status: 'created'}
})

renderWithProviders(<PaymentProcessing />)

Expand All @@ -253,6 +293,7 @@ describe('PaymentProcessing', () => {
expect(screen.getByText('Return to Checkout')).toBeInTheDocument()

await waitFor(() => {
expect(mockRefetchOrder).toHaveBeenCalled()
expect(mockFailOrder).toHaveBeenCalledTimes(1)
expect(mockFailOrder).toHaveBeenCalledWith({
parameters: {
Expand Down Expand Up @@ -392,6 +433,9 @@ describe('PaymentProcessing', () => {

test('shows toast and calls failOrder before navigating on failed payment', async () => {
mockHandleRedirect.mockResolvedValue({responseCode: 1})
mockRefetchOrder.mockResolvedValue({
data: {orderNo: '12345', status: 'created'}
})

renderWithProviders(<PaymentProcessing />)

Expand All @@ -400,6 +444,7 @@ describe('PaymentProcessing', () => {
})

await waitFor(() => {
expect(mockRefetchOrder).toHaveBeenCalled()
expect(mockFailOrder).toHaveBeenCalledTimes(1)
expect(mockFailOrder).toHaveBeenCalledWith({
parameters: {
Expand All @@ -415,12 +460,55 @@ describe('PaymentProcessing', () => {
expect(mockNavigate).toHaveBeenCalledWith('/checkout')
})

test('does not call failOrder when order already failed by webhook', async () => {
mockHandleRedirect.mockResolvedValue({responseCode: 1})
mockRefetchOrder.mockResolvedValue({
data: {orderNo: '12345', status: 'failed'}
})

renderWithProviders(<PaymentProcessing />)

await waitFor(() => {
expect(mockToast).toHaveBeenCalled()
expect(mockNavigate).toHaveBeenCalledWith('/checkout')
})

expect(mockRefetchOrder).toHaveBeenCalled()
expect(mockFailOrder).not.toHaveBeenCalled()
})

test('shows toast and navigates to checkout when failOrder fails', async () => {
mockHandleRedirect.mockResolvedValue({responseCode: 1})
mockRefetchOrder.mockResolvedValue({
data: {orderNo: '12345', status: 'created'}
})
mockFailOrder.mockRejectedValue(new Error('Order already failed'))

renderWithProviders(<PaymentProcessing />)

await waitFor(() => {
expect(mockToast).toHaveBeenCalled()
expect(mockNavigate).toHaveBeenCalledWith('/checkout')
})

expect(mockRefetchOrder).toHaveBeenCalled()
expect(mockFailOrder).toHaveBeenCalledTimes(1)
expect(mockInvalidateQueries).toHaveBeenCalled()
})

test('handles different error response codes', async () => {
const errorCodes = [1, 2, -1, 999]

for (const code of errorCodes) {
jest.clearAllMocks()
mockHandleRedirect.mockResolvedValue({responseCode: code})
mockUseOrder.mockReturnValue({
data: {orderNo: '12345', status: 'created'},
refetch: mockRefetchOrder
})
mockRefetchOrder.mockResolvedValue({
data: {orderNo: '12345', status: 'created'}
})

renderWithProviders(<PaymentProcessing />)

Expand Down Expand Up @@ -545,13 +633,18 @@ describe('PaymentProcessing', () => {
})

test('shows toast and calls failOrder before navigating on failed payment', async () => {
mockRefetchOrder.mockResolvedValue({
data: {orderNo: '12345', status: 'created'}
})

renderWithProviders(<PaymentProcessing />)

await waitFor(() => {
expect(mockToast).toHaveBeenCalled()
})

await waitFor(() => {
expect(mockRefetchOrder).toHaveBeenCalled()
expect(mockFailOrder).toHaveBeenCalledTimes(1)
expect(mockFailOrder).toHaveBeenCalledWith({
parameters: {
Expand Down
Loading