Skip to content

Commit aff72eb

Browse files
@ W-20540715 Address 1CC feature branch review comments (#3619)
* address first set of comments * address rest of code review comments * reverting default.js changes * fix package versions * shipping options fix * attempt to fix flaky tests
1 parent 8a25361 commit aff72eb

File tree

22 files changed

+508
-331
lines changed

22 files changed

+508
-331
lines changed

packages/commerce-sdk-react/CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
## v4.5.0-dev (Jan 19, 2026)
2-
- Upgrade to commerce-sdk-isomorphic v4.2.0 and introduce Payment Instrument SCAPI integration [#3552](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/3552)
1+
## v5.0.0-dev (Jan 28, 2026)
2+
- Upgrade to commerce-sdk-isomorphic v5.0.0 and introduce Payment Instrument SCAPI integration [#3552](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/3552)
33

44
## v4.4.0-dev (Dec 17, 2025)
55
- [Bugfix]Ensure code_verifier can be optional in resetPassword call [#3567](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/3567)

packages/commerce-sdk-react/package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/commerce-sdk-react/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@salesforce/commerce-sdk-react",
3-
"version": "4.4.0-dev",
3+
"version": "5.0.0-dev",
44
"description": "A library that provides react hooks for fetching data from Commerce Cloud",
55
"homepage": "https://github.com/SalesforceCommerceCloud/pwa-kit/tree/develop/packages/ecom-react-hooks#readme",
66
"bugs": {
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*
2+
* Copyright (c) 2021, salesforce.com, inc.
3+
* All rights reserved.
4+
* SPDX-License-Identifier: BSD-3-Clause
5+
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
6+
*/
7+
8+
import {useEffect, useRef, useState} from 'react'
9+
10+
/**
11+
* Generic hook for auto-selecting and applying saved customer data during checkout
12+
* @param {Object} config Configuration object
13+
* @param {number} config.currentStep - Current checkout step
14+
* @param {number} config.targetStep - Step this auto-selection should run on
15+
* @param {boolean} config.isCustomerRegistered - Whether customer is registered
16+
* @param {Array} config.items - List of items to select from (addresses, payments, etc.)
17+
* @param {Function} config.getPreferredItem - Function to find preferred item from list
18+
* @param {Function} config.shouldSkip - Optional function returning boolean if auto-select should be skipped
19+
* @param {Function} config.isAlreadyApplied - Function checking if item is already applied
20+
* @param {Function} config.applyItem - Async function to apply the selected item
21+
* @param {Function} config.onSuccess - Optional callback after successful application
22+
* @param {Function} config.onError - Optional callback after error
23+
* @param {boolean} config.enabled - Whether auto-selection is enabled (default: true)
24+
* @returns {Object} { isLoading, hasExecuted, reset }
25+
*/
26+
export const useCheckoutAutoSelect = ({
27+
currentStep,
28+
targetStep,
29+
isCustomerRegistered,
30+
items = [],
31+
getPreferredItem,
32+
shouldSkip = () => false,
33+
isAlreadyApplied = () => false,
34+
applyItem,
35+
onSuccess,
36+
onError,
37+
enabled = true
38+
}) => {
39+
const [isLoading, setIsLoading] = useState(false)
40+
const hasExecutedRef = useRef(false)
41+
42+
const reset = () => {
43+
hasExecutedRef.current = false
44+
setIsLoading(false)
45+
}
46+
47+
useEffect(() => {
48+
const autoSelect = async () => {
49+
// Early returns for conditions that prevent auto-selection
50+
if (!enabled) return
51+
if (currentStep !== targetStep) return
52+
if (hasExecutedRef.current) return
53+
if (isLoading) return
54+
if (!isCustomerRegistered) return
55+
if (!items || items.length === 0) return
56+
if (shouldSkip()) return
57+
if (isAlreadyApplied()) return
58+
59+
// Find the preferred item
60+
const preferredItem = getPreferredItem(items)
61+
if (!preferredItem) return
62+
63+
// Mark as executed before starting to prevent race conditions
64+
hasExecutedRef.current = true
65+
setIsLoading(true)
66+
67+
try {
68+
// Apply the item
69+
await applyItem(preferredItem)
70+
71+
// Call success callback if provided
72+
if (onSuccess) {
73+
await onSuccess(preferredItem)
74+
}
75+
} catch (error) {
76+
// Reset on error to allow manual selection
77+
hasExecutedRef.current = false
78+
79+
// Call error callback if provided
80+
if (onError) {
81+
onError(error)
82+
}
83+
} finally {
84+
setIsLoading(false)
85+
}
86+
}
87+
88+
autoSelect()
89+
}, [currentStep, targetStep, isCustomerRegistered, items, isLoading, enabled])
90+
91+
return {
92+
isLoading,
93+
hasExecuted: hasExecutedRef.current,
94+
reset
95+
}
96+
}
Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
/*
2+
* Copyright (c) 2021, salesforce.com, inc.
3+
* All rights reserved.
4+
* SPDX-License-Identifier: BSD-3-Clause
5+
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
6+
*/
7+
8+
import React from 'react'
9+
import PropTypes from 'prop-types'
10+
import {render, screen, act, waitFor} from '@testing-library/react'
11+
import {useCheckoutAutoSelect} from '@salesforce/retail-react-app/app/hooks/use-checkout-auto-select'
12+
13+
const STEP_SHIPPING = 1
14+
const STEP_PAYMENT = 2
15+
16+
function TestWrapper({
17+
currentStep = STEP_SHIPPING,
18+
targetStep = STEP_SHIPPING,
19+
isCustomerRegistered = true,
20+
items = [{id: 'addr-1', preferred: true}],
21+
getPreferredItem = (list) => list.find((i) => i.preferred) || list[0],
22+
shouldSkip = () => false,
23+
isAlreadyApplied = () => false,
24+
applyItem = jest.fn(() => Promise.resolve()),
25+
onSuccess = jest.fn(),
26+
onError = jest.fn(),
27+
enabled = true
28+
}) {
29+
const result = useCheckoutAutoSelect({
30+
currentStep,
31+
targetStep,
32+
isCustomerRegistered,
33+
items,
34+
getPreferredItem,
35+
shouldSkip,
36+
isAlreadyApplied,
37+
applyItem,
38+
onSuccess,
39+
onError,
40+
enabled
41+
})
42+
return (
43+
<div>
44+
<span data-testid="isLoading">{String(result.isLoading)}</span>
45+
<span data-testid="hasExecuted">{String(result.hasExecuted)}</span>
46+
<button type="button" onClick={result.reset} data-testid="reset">
47+
Reset
48+
</button>
49+
</div>
50+
)
51+
}
52+
53+
TestWrapper.propTypes = {
54+
currentStep: PropTypes.number,
55+
targetStep: PropTypes.number,
56+
isCustomerRegistered: PropTypes.bool,
57+
items: PropTypes.array,
58+
getPreferredItem: PropTypes.func,
59+
shouldSkip: PropTypes.func,
60+
isAlreadyApplied: PropTypes.func,
61+
applyItem: PropTypes.func,
62+
onSuccess: PropTypes.func,
63+
onError: PropTypes.func,
64+
enabled: PropTypes.bool
65+
}
66+
67+
describe('useCheckoutAutoSelect', () => {
68+
beforeEach(() => {
69+
jest.clearAllMocks()
70+
})
71+
72+
describe('early returns - does not call applyItem', () => {
73+
test('does not run when enabled is false', async () => {
74+
const applyItem = jest.fn(() => Promise.resolve())
75+
render(<TestWrapper enabled={false} applyItem={applyItem} items={[{id: 'a'}]} />)
76+
await waitFor(() => {
77+
expect(applyItem).not.toHaveBeenCalled()
78+
})
79+
})
80+
81+
test('does not run when currentStep does not match targetStep', async () => {
82+
const applyItem = jest.fn(() => Promise.resolve())
83+
render(
84+
<TestWrapper
85+
currentStep={STEP_PAYMENT}
86+
targetStep={STEP_SHIPPING}
87+
applyItem={applyItem}
88+
/>
89+
)
90+
await waitFor(() => {
91+
expect(applyItem).not.toHaveBeenCalled()
92+
})
93+
})
94+
95+
test('does not run when isCustomerRegistered is false', async () => {
96+
const applyItem = jest.fn(() => Promise.resolve())
97+
render(<TestWrapper isCustomerRegistered={false} applyItem={applyItem} />)
98+
await waitFor(() => {
99+
expect(applyItem).not.toHaveBeenCalled()
100+
})
101+
})
102+
103+
test('does not run when items is null or empty', async () => {
104+
const applyItem = jest.fn(() => Promise.resolve())
105+
const {rerender} = render(<TestWrapper items={null} applyItem={applyItem} />)
106+
await waitFor(() => {
107+
expect(applyItem).not.toHaveBeenCalled()
108+
})
109+
110+
rerender(<TestWrapper items={[]} applyItem={applyItem} />)
111+
await waitFor(() => {
112+
expect(applyItem).not.toHaveBeenCalled()
113+
})
114+
})
115+
116+
test('does not run when shouldSkip returns true', async () => {
117+
const applyItem = jest.fn(() => Promise.resolve())
118+
render(<TestWrapper shouldSkip={() => true} applyItem={applyItem} />)
119+
await waitFor(() => {
120+
expect(applyItem).not.toHaveBeenCalled()
121+
})
122+
})
123+
124+
test('does not run when isAlreadyApplied returns true', async () => {
125+
const applyItem = jest.fn(() => Promise.resolve())
126+
render(<TestWrapper isAlreadyApplied={() => true} applyItem={applyItem} />)
127+
await waitFor(() => {
128+
expect(applyItem).not.toHaveBeenCalled()
129+
})
130+
})
131+
132+
test('does not run when getPreferredItem returns null/undefined', async () => {
133+
const applyItem = jest.fn(() => Promise.resolve())
134+
render(<TestWrapper getPreferredItem={() => null} applyItem={applyItem} />)
135+
await waitFor(() => {
136+
expect(applyItem).not.toHaveBeenCalled()
137+
})
138+
})
139+
})
140+
141+
describe('when conditions are met', () => {
142+
test('calls applyItem with the preferred item', async () => {
143+
const applyItem = jest.fn(() => Promise.resolve())
144+
const items = [
145+
{id: 'addr-1', preferred: false},
146+
{id: 'addr-2', preferred: true}
147+
]
148+
render(<TestWrapper items={items} applyItem={applyItem} />)
149+
150+
await waitFor(() => {
151+
expect(applyItem).toHaveBeenCalledTimes(1)
152+
expect(applyItem).toHaveBeenCalledWith({id: 'addr-2', preferred: true})
153+
})
154+
})
155+
156+
test('calls onSuccess with the preferred item after applyItem resolves', async () => {
157+
const applyItem = jest.fn(() => Promise.resolve())
158+
const onSuccess = jest.fn(() => Promise.resolve())
159+
const preferred = {id: 'addr-1', preferred: true}
160+
render(<TestWrapper items={[preferred]} applyItem={applyItem} onSuccess={onSuccess} />)
161+
162+
await waitFor(() => {
163+
expect(applyItem).toHaveBeenCalledWith(preferred)
164+
expect(onSuccess).toHaveBeenCalledWith(preferred)
165+
})
166+
})
167+
168+
test('does not call onSuccess when not provided', async () => {
169+
const applyItem = jest.fn(() => Promise.resolve())
170+
render(<TestWrapper items={[{id: 'a'}]} applyItem={applyItem} />)
171+
172+
await waitFor(() => {
173+
expect(applyItem).toHaveBeenCalled()
174+
})
175+
})
176+
177+
test('resets hasExecutedRef and calls onError when applyItem throws', async () => {
178+
const error = new Error('Apply failed')
179+
const applyItem = jest.fn(() => Promise.reject(error))
180+
const onError = jest.fn()
181+
render(<TestWrapper items={[{id: 'a'}]} applyItem={applyItem} onError={onError} />)
182+
183+
await waitFor(() => {
184+
expect(applyItem).toHaveBeenCalled()
185+
expect(onError).toHaveBeenCalledWith(error)
186+
})
187+
// Effect may re-run after error (e.g. when isLoading changes), so onError can be called more than once
188+
expect(onError).toHaveBeenCalled()
189+
})
190+
191+
test('runs only once (hasExecutedRef prevents re-run)', async () => {
192+
const applyItem = jest.fn(() => Promise.resolve())
193+
const {rerender} = render(<TestWrapper items={[{id: 'a'}]} applyItem={applyItem} />)
194+
195+
await waitFor(() => {
196+
expect(applyItem).toHaveBeenCalledTimes(1)
197+
})
198+
199+
rerender(
200+
<TestWrapper
201+
items={[{id: 'a'}]}
202+
applyItem={applyItem}
203+
currentStep={STEP_SHIPPING}
204+
/>
205+
)
206+
207+
await waitFor(() => {
208+
expect(applyItem).toHaveBeenCalledTimes(1)
209+
})
210+
})
211+
})
212+
213+
describe('reset', () => {
214+
test('reset is a function that can be called without throwing', async () => {
215+
const applyItem = jest.fn(() => Promise.resolve())
216+
render(<TestWrapper items={[{id: 'a'}]} applyItem={applyItem} />)
217+
218+
await waitFor(() => {
219+
expect(applyItem).toHaveBeenCalledTimes(1)
220+
})
221+
222+
expect(() => {
223+
act(() => {
224+
screen.getByTestId('reset').click()
225+
})
226+
}).not.toThrow()
227+
})
228+
})
229+
230+
describe('return value', () => {
231+
test('returns isLoading, hasExecuted, and reset', async () => {
232+
const applyItem = jest.fn(() => Promise.resolve())
233+
render(<TestWrapper items={[{id: 'a'}]} applyItem={applyItem} />)
234+
235+
expect(screen.getByTestId('isLoading')).toBeInTheDocument()
236+
expect(screen.getByTestId('hasExecuted')).toBeInTheDocument()
237+
expect(screen.getByTestId('reset')).toBeInTheDocument()
238+
239+
await waitFor(() => {
240+
expect(applyItem).toHaveBeenCalled()
241+
})
242+
})
243+
})
244+
})

0 commit comments

Comments
 (0)