Skip to content

Commit ee54ad2

Browse files
rasbhatrasbhat
committed
W-21372336: Addressing comments
1 parent 1389694 commit ee54ad2

File tree

2 files changed

+65
-47
lines changed

2 files changed

+65
-47
lines changed

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

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,7 @@ import {FormattedMessage} from 'react-intl'
1414
import {Heading, Stack, Text} from '@salesforce/retail-react-app/app/components/shared/ui'
1515
import Link from '@salesforce/retail-react-app/app/components/link'
1616

17-
import {
18-
useOrder,
19-
useShopperOrdersMutation,
20-
useCommerceApi,
21-
useAccessToken
22-
} from '@salesforce/commerce-sdk-react'
17+
import {useOrder, useShopperOrdersMutation} from '@salesforce/commerce-sdk-react'
2318
import {useQueryClient} from '@tanstack/react-query'
2419
import useNavigation from '@salesforce/retail-react-app/app/hooks/use-navigation'
2520
import {useSFPayments, STATUS_SUCCESS} from '@salesforce/retail-react-app/app/hooks/use-sf-payments'
@@ -49,8 +44,6 @@ const PaymentProcessing = () => {
4944
const {sfp} = useSFPayments()
5045
const toast = useToast()
5146
const queryClient = useQueryClient()
52-
const api = useCommerceApi()
53-
const {getTokenWhenReady} = useAccessToken()
5447

5548
const {mutateAsync: updatePaymentInstrumentForOrder} = useShopperOrdersMutation(
5649
'updatePaymentInstrumentForOrder'
@@ -60,7 +53,7 @@ const PaymentProcessing = () => {
6053
const params = new URLSearchParams(location.search)
6154
const vendor = params.get('vendor')
6255
const orderNo = params.get('orderNo')
63-
const {data: order} = useOrder(
56+
const {data: order, refetch} = useOrder(
6457
{
6558
parameters: {orderNo}
6659
},
@@ -129,7 +122,7 @@ const PaymentProcessing = () => {
129122
/**
130123
* Attempts to fail an order and reopen the basket.
131124
* Only calls failOrder if the order status is 'created' (avoids hanging when order
132-
* was already failed by webhook.)
125+
* was already failed by webhook).
133126
* @returns {Promise<void>}
134127
*/
135128
async function attemptFailOrderForPayment() {
@@ -138,13 +131,8 @@ const PaymentProcessing = () => {
138131
}
139132

140133
try {
141-
const token = await getTokenWhenReady()
142-
const currentOrder = await api.shopperOrders.getOrder({
143-
parameters: {orderNo},
144-
headers: {Authorization: `Bearer ${token}`}
145-
})
146-
147-
if (currentOrder.status === 'created') {
134+
const {data: currentOrder} = await refetch()
135+
if (currentOrder?.status === 'created') {
148136
await failOrder({
149137
parameters: {
150138
orderNo,
@@ -156,9 +144,9 @@ const PaymentProcessing = () => {
156144
})
157145
}
158146
} catch (error) {
159-
// Swallow so flow continues (invalidate, navigate). Causes: (1) Race: getOrder
160-
// returned 'created' but webhook already failed the order, so failOrder fails. (2) getOrder,
161-
// getTokenWhenReady, or failOrder threw error. Behavior for all: don't hang.
147+
// Swallow so flow continues (invalidate, navigate). Causes: (1) Race: refetch
148+
// returned 'created' but webhook already failed the order, so failOrder fails. (2) refetch
149+
// or failOrder threw (network, 4xx/5xx). Same behavior for all: don't hang.
162150
} finally {
163151
queryClient.invalidateQueries()
164152
}
@@ -210,7 +198,7 @@ const PaymentProcessing = () => {
210198
duration: 30000
211199
})
212200

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

216204
// Navigate back to the checkout page to try again

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

Lines changed: 56 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ const mockUseOrder = jest.fn()
2020
const mockUpdatePaymentInstrumentForOrder = jest.fn()
2121
const mockFailOrder = jest.fn()
2222
const mockGetSFPaymentsInstrument = jest.fn()
23-
const mockGetOrder = jest.fn()
24-
const mockGetTokenWhenReady = jest.fn()
23+
const mockRefetchOrder = jest.fn()
2524
const mockInvalidateQueries = jest.fn()
2625

2726
jest.mock('@salesforce/retail-react-app/app/hooks/use-navigation', () => ({
@@ -43,14 +42,6 @@ jest.mock('@salesforce/commerce-sdk-react', () => {
4342
const actual = jest.requireActual('@salesforce/commerce-sdk-react')
4443
return {
4544
...actual,
46-
useCommerceApi: () => ({
47-
shopperOrders: {
48-
getOrder: mockGetOrder
49-
}
50-
}),
51-
useAccessToken: () => ({
52-
getTokenWhenReady: mockGetTokenWhenReady
53-
}),
5445
useShopperOrdersMutation: (mutationKey) => {
5546
if (mutationKey === 'updatePaymentInstrumentForOrder') {
5647
return {mutateAsync: mockUpdatePaymentInstrumentForOrder}
@@ -104,16 +95,18 @@ describe('PaymentProcessing', () => {
10495

10596
mockUseOrder.mockReturnValue({
10697
data: {
107-
orderNo: '12345'
108-
}
98+
orderNo: '12345',
99+
status: 'created'
100+
},
101+
refetch: mockRefetchOrder
102+
})
103+
mockRefetchOrder.mockResolvedValue({
104+
data: {orderNo: '12345', status: 'created'}
109105
})
110106

111107
mockUpdatePaymentInstrumentForOrder.mockReturnValue({})
112108

113109
mockGetSFPaymentsInstrument.mockReturnValue({})
114-
115-
mockGetOrder.mockResolvedValue({status: 'created'})
116-
mockGetTokenWhenReady.mockResolvedValue('token')
117110
})
118111

119112
afterEach(() => {
@@ -220,6 +213,13 @@ describe('PaymentProcessing', () => {
220213

221214
test('renders error message for invalid Adyen URL missing type', async () => {
222215
mockLocation.search = '?vendor=Adyen&orderNo=12345&zoneId=default&redirectResult=ABC123'
216+
mockUseOrder.mockReturnValue({
217+
data: {orderNo: '12345'},
218+
refetch: mockRefetchOrder
219+
})
220+
mockRefetchOrder.mockResolvedValue({
221+
data: {orderNo: '12345', status: 'created'}
222+
})
223223

224224
renderWithProviders(<PaymentProcessing />)
225225

@@ -229,7 +229,7 @@ describe('PaymentProcessing', () => {
229229
expect(screen.getByText('Return to Checkout')).toBeInTheDocument()
230230

231231
await waitFor(() => {
232-
expect(mockGetOrder).toHaveBeenCalled()
232+
expect(mockRefetchOrder).toHaveBeenCalled()
233233
expect(mockFailOrder).toHaveBeenCalledTimes(1)
234234
expect(mockFailOrder).toHaveBeenCalledWith({
235235
parameters: {
@@ -245,6 +245,13 @@ describe('PaymentProcessing', () => {
245245

246246
test('renders error message for invalid Adyen URL missing zone id', async () => {
247247
mockLocation.search = '?vendor=Adyen&orderNo=12345&type=klarna&redirectResult=ABC123'
248+
mockUseOrder.mockReturnValue({
249+
data: {orderNo: '12345'},
250+
refetch: mockRefetchOrder
251+
})
252+
mockRefetchOrder.mockResolvedValue({
253+
data: {orderNo: '12345', status: 'created'}
254+
})
248255

249256
renderWithProviders(<PaymentProcessing />)
250257

@@ -254,7 +261,7 @@ describe('PaymentProcessing', () => {
254261
expect(screen.getByText('Return to Checkout')).toBeInTheDocument()
255262

256263
await waitFor(() => {
257-
expect(mockGetOrder).toHaveBeenCalled()
264+
expect(mockRefetchOrder).toHaveBeenCalled()
258265
expect(mockFailOrder).toHaveBeenCalledTimes(1)
259266
expect(mockFailOrder).toHaveBeenCalledWith({
260267
parameters: {
@@ -270,6 +277,13 @@ describe('PaymentProcessing', () => {
270277

271278
test('renders error message for invalid Adyen URL missing redirect result', async () => {
272279
mockLocation.search = '?vendor=Adyen&orderNo=12345&type=klarna&zoneId=default'
280+
mockUseOrder.mockReturnValue({
281+
data: {orderNo: '12345'},
282+
refetch: mockRefetchOrder
283+
})
284+
mockRefetchOrder.mockResolvedValue({
285+
data: {orderNo: '12345', status: 'created'}
286+
})
273287

274288
renderWithProviders(<PaymentProcessing />)
275289

@@ -279,7 +293,7 @@ describe('PaymentProcessing', () => {
279293
expect(screen.getByText('Return to Checkout')).toBeInTheDocument()
280294

281295
await waitFor(() => {
282-
expect(mockGetOrder).toHaveBeenCalled()
296+
expect(mockRefetchOrder).toHaveBeenCalled()
283297
expect(mockFailOrder).toHaveBeenCalledTimes(1)
284298
expect(mockFailOrder).toHaveBeenCalledWith({
285299
parameters: {
@@ -419,6 +433,9 @@ describe('PaymentProcessing', () => {
419433

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

423440
renderWithProviders(<PaymentProcessing />)
424441

@@ -427,7 +444,7 @@ describe('PaymentProcessing', () => {
427444
})
428445

429446
await waitFor(() => {
430-
expect(mockGetOrder).toHaveBeenCalled()
447+
expect(mockRefetchOrder).toHaveBeenCalled()
431448
expect(mockFailOrder).toHaveBeenCalledTimes(1)
432449
expect(mockFailOrder).toHaveBeenCalledWith({
433450
parameters: {
@@ -445,7 +462,9 @@ describe('PaymentProcessing', () => {
445462

446463
test('does not call failOrder when order already failed by webhook', async () => {
447464
mockHandleRedirect.mockResolvedValue({responseCode: 1})
448-
mockGetOrder.mockResolvedValue({status: 'failed'})
465+
mockRefetchOrder.mockResolvedValue({
466+
data: {orderNo: '12345', status: 'failed'}
467+
})
449468

450469
renderWithProviders(<PaymentProcessing />)
451470

@@ -454,13 +473,15 @@ describe('PaymentProcessing', () => {
454473
expect(mockNavigate).toHaveBeenCalledWith('/checkout')
455474
})
456475

457-
expect(mockGetOrder).toHaveBeenCalled()
476+
expect(mockRefetchOrder).toHaveBeenCalled()
458477
expect(mockFailOrder).not.toHaveBeenCalled()
459478
})
460479

461480
test('shows toast and navigates to checkout when failOrder fails', async () => {
462481
mockHandleRedirect.mockResolvedValue({responseCode: 1})
463-
mockGetOrder.mockResolvedValue({status: 'created'})
482+
mockRefetchOrder.mockResolvedValue({
483+
data: {orderNo: '12345', status: 'created'}
484+
})
464485
mockFailOrder.mockRejectedValue(new Error('Order already failed'))
465486

466487
renderWithProviders(<PaymentProcessing />)
@@ -470,7 +491,7 @@ describe('PaymentProcessing', () => {
470491
expect(mockNavigate).toHaveBeenCalledWith('/checkout')
471492
})
472493

473-
expect(mockGetOrder).toHaveBeenCalled()
494+
expect(mockRefetchOrder).toHaveBeenCalled()
474495
expect(mockFailOrder).toHaveBeenCalledTimes(1)
475496
expect(mockInvalidateQueries).toHaveBeenCalled()
476497
})
@@ -481,8 +502,13 @@ describe('PaymentProcessing', () => {
481502
for (const code of errorCodes) {
482503
jest.clearAllMocks()
483504
mockHandleRedirect.mockResolvedValue({responseCode: code})
484-
mockGetOrder.mockResolvedValue({status: 'created'})
485-
mockGetTokenWhenReady.mockResolvedValue('token')
505+
mockUseOrder.mockReturnValue({
506+
data: {orderNo: '12345', status: 'created'},
507+
refetch: mockRefetchOrder
508+
})
509+
mockRefetchOrder.mockResolvedValue({
510+
data: {orderNo: '12345', status: 'created'}
511+
})
486512

487513
renderWithProviders(<PaymentProcessing />)
488514

@@ -607,14 +633,18 @@ describe('PaymentProcessing', () => {
607633
})
608634

609635
test('shows toast and calls failOrder before navigating on failed payment', async () => {
636+
mockRefetchOrder.mockResolvedValue({
637+
data: {orderNo: '12345', status: 'created'}
638+
})
639+
610640
renderWithProviders(<PaymentProcessing />)
611641

612642
await waitFor(() => {
613643
expect(mockToast).toHaveBeenCalled()
614644
})
615645

616646
await waitFor(() => {
617-
expect(mockGetOrder).toHaveBeenCalled()
647+
expect(mockRefetchOrder).toHaveBeenCalled()
618648
expect(mockFailOrder).toHaveBeenCalledTimes(1)
619649
expect(mockFailOrder).toHaveBeenCalledWith({
620650
parameters: {

0 commit comments

Comments
 (0)