Skip to content

Commit 61c0584

Browse files
committed
fix edge cases
Signed-off-by: d.phan <d.phan@salesforce.com>
1 parent c1059b8 commit 61c0584

File tree

4 files changed

+71
-27
lines changed

4 files changed

+71
-27
lines changed

packages/template-retail-react-app/app/pages/cart/index.jsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,15 @@ const Cart = () => {
377377
return
378378
}
379379

380+
// Don't assign methods until at least one delivery shipment has an address
381+
const deliveryShipments = basket.shipments?.filter((s) => !isPickupShipment(s)) || []
382+
const hasDeliveryWithAddress = deliveryShipments.some(
383+
(s) => s.shippingAddress?.address1
384+
)
385+
if (deliveryShipments.length > 0 && !hasDeliveryWithAddress) {
386+
return
387+
}
388+
380389
setIsProcessingShippingMethods(true)
381390
try {
382391
await updateShipmentsWithoutMethods()

packages/template-retail-react-app/app/pages/checkout-one-click/partials/one-click-shipping-address.jsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,14 @@ export default function ShippingAddress(props) {
4848
const {enableUserRegistration = false, isShipmentCleanupComplete = true} = props
4949
const {formatMessage} = useIntl()
5050
const [isManualSubmitLoading, setIsManualSubmitLoading] = useState(false)
51-
const [isMultiShipping, setIsMultiShipping] = useState(false)
52-
const [openedByUser, setOpenedByUser] = useState(false)
5351
const {data: customer} = useCurrentCustomer()
5452
const currentBasketQuery = useCurrentBasket()
5553
const {data: basket} = currentBasketQuery
5654
const deliveryShipments =
5755
basket?.shipments?.filter((shipment) => !isPickupShipment(shipment)) || []
56+
const hasMultipleDeliveryShipments = deliveryShipments.length > 1
57+
const [isMultiShipping, setIsMultiShipping] = useState(hasMultipleDeliveryShipments)
58+
const [openedByUser, setOpenedByUser] = useState(false)
5859
const selectedShippingAddress = deliveryShipments[0]?.shippingAddress
5960
const targetDeliveryShipmentId = deliveryShipments[0]?.shipmentId || 'me'
6061
const isAddressFilled = selectedShippingAddress?.address1 && selectedShippingAddress?.city
@@ -65,7 +66,7 @@ export default function ShippingAddress(props) {
6566
'updateShippingAddressForShipment'
6667
)
6768
const multishipEnabled = getConfig()?.app?.multishipEnabled ?? true
68-
const hasMultipleDeliveryShipments = deliveryShipments.length > 1
69+
6970
const {removeEmptyShipments} = useMultiship(basket)
7071
const {updateItemsToDeliveryShipment} = useItemShipmentManagement(basket?.basketId)
7172

packages/template-retail-react-app/app/pages/checkout-one-click/partials/one-click-shipping-address.test.js

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,14 @@ describe('ShippingAddress Component', () => {
618618
expect(screen.getByRole('button', {name: 'Ship items to one address'})).toBeInTheDocument()
619619
})
620620
test('should consolidate multiple shipments when shipping to single address', async () => {
621+
mockUpdateShippingAddress.mutateAsync.mockResolvedValue({})
622+
mockUpdateCustomerAddress.mutateAsync.mockResolvedValue({})
623+
mockUpdateItemsToDeliveryShipment.mockResolvedValue({
624+
basketId: 'test-basket-id',
625+
shipments: []
626+
})
627+
mockRemoveEmptyShipments.mockResolvedValue({})
628+
621629
jest.resetModules()
622630
jest.doMock('@salesforce/retail-react-app/app/hooks/use-current-basket', () => ({
623631
useCurrentBasket: () => ({
@@ -658,18 +666,33 @@ describe('ShippingAddress Component', () => {
658666

659667
const {user} = localRenderWithProviders(<Component />)
660668

661-
//only get first
662-
const continueButton = screen.getAllByRole('button', {
663-
name: 'Continue to Shipping Method'
664-
})[0]
669+
// With multiple delivery shipments the component shows multi-address view first.
670+
// Click "Ship items to one address" to switch to single-address form.
671+
const shipToOneAddressButtons = screen.getAllByRole('button', {
672+
name: 'Ship items to one address'
673+
})
674+
await act(async () => {
675+
await user.click(shipToOneAddressButtons[0])
676+
})
665677

678+
const continueButtons = screen.getAllByRole('button', {
679+
name: 'Continue to Shipping Method'
680+
})
681+
const continueButton = continueButtons[0]
666682
await act(async () => {
667683
await user.click(continueButton)
668684
})
669-
// Expect removeEmptyShipments to be called
670-
expect(mockRemoveEmptyShipments).toHaveBeenCalled()
671-
// Expect updateItemsToDeliveryShipment to be called
672-
expect(mockUpdateItemsToDeliveryShipment).toHaveBeenCalled()
685+
686+
// Wait for async submit flow (address update -> optional customer address -> remove empty)
687+
await waitFor(() => {
688+
expect(mockUpdateShippingAddress.mutateAsync).toHaveBeenCalled()
689+
})
690+
await waitFor(() => {
691+
expect(mockRemoveEmptyShipments).toHaveBeenCalled()
692+
})
693+
// updateItemsToDeliveryShipment is called when basket has productItems in multiple
694+
// delivery shipments; that path is covered in "auto-selects preferred address for
695+
// multi-shipment orders and consolidates items"
673696
})
674697

675698
test('does not show multiship option when only one delivery item exists with pickup items', async () => {
@@ -907,8 +930,10 @@ describe('ShippingAddress Component', () => {
907930
mockUpdateShippingAddress.mutateAsync.mockResolvedValue({})
908931
const {user} = renderWithProviders(<ShippingAddress enableUserRegistration={true} />)
909932

910-
const stepContainers = screen.getAllByTestId('sf-toggle-card-step-1')
911-
const selection = within(stepContainers[0]).getByTestId('shipping-address-selection')
933+
// Wait for the address selection form to be visible (may race with auto-select)
934+
const selection = await waitFor(() => screen.getByTestId('shipping-address-selection'), {
935+
timeout: 2000
936+
})
912937
const submitButton = within(selection).getByRole('button', {
913938
name: /Continue to Shipping Method/i
914939
})

packages/template-retail-react-app/app/pages/checkout-one-click/partials/one-click-shipping-options.jsx

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,21 +58,22 @@ export default function ShippingOptions() {
5858
const hasMultipleDeliveryShipments = deliveryShipments.length > 1
5959
const targetDeliveryShipment = hasMultipleDeliveryShipments ? null : deliveryShipments[0]
6060

61-
const {data: shippingMethods} = useShippingMethodsForShipment(
62-
{
63-
parameters: {
64-
basketId: basket?.basketId,
65-
shipmentId: targetDeliveryShipment?.shipmentId || 'me'
61+
const {data: shippingMethods, isFetching: isShippingMethodsFetching} =
62+
useShippingMethodsForShipment(
63+
{
64+
parameters: {
65+
basketId: basket?.basketId,
66+
shipmentId: targetDeliveryShipment?.shipmentId || 'me'
67+
}
68+
},
69+
{
70+
enabled:
71+
Boolean(basket?.basketId) &&
72+
step === STEPS.SHIPPING_OPTIONS &&
73+
!hasMultipleDeliveryShipments
74+
// Single-shipment "no methods" toast is handled in useEffect when data is available
6675
}
67-
},
68-
{
69-
enabled:
70-
Boolean(basket?.basketId) &&
71-
step === STEPS.SHIPPING_OPTIONS &&
72-
!hasMultipleDeliveryShipments
73-
// Single-shipment "no methods" toast is handled in useEffect when data is available
74-
}
75-
)
76+
)
7677

7778
const selectedShippingMethod = targetDeliveryShipment?.shippingMethod
7879
const selectedShippingAddress = targetDeliveryShipment?.shippingAddress
@@ -85,6 +86,13 @@ export default function ShippingOptions() {
8586
setNoMethodsToastShown(false)
8687
}, [deliveryAddressStateKey])
8788

89+
useEffect(() => {
90+
if (!hasMultipleDeliveryShipments) {
91+
setShipmentIdsWithNoMethods(() => new Set())
92+
setNoMethodsToastShown(false)
93+
}
94+
}, [hasMultipleDeliveryShipments])
95+
8896
// Filter out pickup methods for delivery shipment
8997
const deliveryMethods = getDeliveryShippingMethods(
9098
shippingMethods?.applicableShippingMethods || []
@@ -135,6 +143,7 @@ export default function ShippingOptions() {
135143
const singleShipmentNoMethods =
136144
!hasMultipleDeliveryShipments &&
137145
step === STEPS.SHIPPING_OPTIONS &&
146+
!isShippingMethodsFetching &&
138147
shippingMethods != null &&
139148
deliveryMethods.length === 0
140149
useEffect(() => {

0 commit comments

Comments
 (0)