Skip to content

@W-20038640 Fix: 400 Error #3443

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
sf-shikhar-prasoon merged 11 commits into from
Oct 30, 2025
Merged

@W-20038640 Fix: 400 Error #3443

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
35c14ae
fix
sf-shikhar-prasoon Oct 30, 2025
fbe45f6
add/update tests
sf-shikhar-prasoon Oct 30, 2025
522a602
lint fix
sf-shikhar-prasoon Oct 30, 2025
c753399
fix tests?
sf-shikhar-prasoon Oct 30, 2025
ae16d45
variant selection fix, test fix
sf-shikhar-prasoon Oct 30, 2025
cd8c75a
refactor- duplicate code
sf-shikhar-prasoon Oct 30, 2025
351310f
fix useVariant if product is already a variant
sf-shikhar-prasoon Oct 30, 2025
303104c
only use react state for product-view modal
sf-shikhar-prasoon Oct 30, 2025
9048a9f
removed the unnecessary useCallback
sf-shikhar-prasoon Oct 30, 2025
d4d6311
add tests- Variation State Reset | URL Pollution Prevention
sf-shikhar-prasoon Oct 30, 2025
83d33af
Merge branch 'develop' into t/cc-sharks/W-20038640/400Error/main
sf-shikhar-prasoon Oct 30, 2025
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
37 changes: 36 additions & 1 deletion packages/template-retail-react-app/app/components/bonus-product-view-modal/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,40 @@ const BonusProductViewModal = ({
}
}, [product])

const productViewModalData = useProductViewModal(safeProduct, {keepPreviousData: true})
// Controlled variation values state for modal (doesn't use URL params)
const [controlledVariationValues, setControlledVariationValues] = React.useState({})

const productViewModalData = useProductViewModal(safeProduct, {
keepPreviousData: true
})

// Auto-select variation attributes with only one value
React.useEffect(() => {
if (!productViewModalData.product?.variationAttributes) return

const autoSelections = {}
productViewModalData.product.variationAttributes.forEach((attr) => {
// Only auto-select if there's exactly one value and it's not already selected
if (attr.values?.length === 1 && !controlledVariationValues[attr.id]) {
autoSelections[attr.id] = attr.values[0].value
}
})

if (Object.keys(autoSelections).length > 0) {
setControlledVariationValues((prev) => ({
...prev,
...autoSelections
}))
}
}, [productViewModalData.product?.variationAttributes, controlledVariationValues])

// Handle variation changes in controlled mode
const handleVariationChange = React.useCallback((attributeId, value) => {
setControlledVariationValues((prev) => ({
...prev,
[attributeId]: value
}))
}, [])

// Keep a stable reference to the last successfully loaded product
// This prevents constant re-renders while fetching
Expand Down Expand Up @@ -471,6 +504,8 @@ const BonusProductViewModal = ({
<HideOnMobile>{BackToSelectionButton}</HideOnMobile>
) : null
}
controlledVariationValues={controlledVariationValues}
onVariationChange={handleVariationChange}
{...props}
/>
)}
Expand Down
40 changes: 39 additions & 1 deletion packages/template-retail-react-app/app/components/product-view-modal/bundle.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,50 @@ const BundleProductViewModal = ({
showDeliveryOptions,
...props
}) => {
// Controlled variation values state for modal (doesn't use URL params)
const [controlledVariationValues, setControlledVariationValues] = React.useState({})

const productViewModalData = useProductViewModal(bundle)
const {variationParams} = useDerivedProduct(bundle)
const {variationParams} = useDerivedProduct(
bundle,
false,
false,
false,
controlledVariationValues
)
const childProductRefs = useRef({})
const [childProductOrderability, setChildProductOrderability] = useState({})
const [selectedChildProducts, setSelectedChildProducts] = useState([])
const [selectedBundleQuantity, setSelectedBundleQuantity] = useState(
productViewModalData?.product?.quantity
)

// Auto-select variation attributes with only one value
React.useEffect(() => {
if (!productViewModalData.product?.variationAttributes) return

const autoSelections = {}
productViewModalData.product.variationAttributes.forEach((attr) => {
if (attr.values?.length === 1 && !controlledVariationValues[attr.id]) {
autoSelections[attr.id] = attr.values[0].value
}
})

if (Object.keys(autoSelections).length > 0) {
setControlledVariationValues((prev) => ({
...prev,
...autoSelections
}))
}
}, [productViewModalData.product?.variationAttributes, controlledVariationValues])

// Handle variation changes in controlled mode
const handleVariationChange = React.useCallback((attributeId, value) => {
setControlledVariationValues((prev) => ({
...prev,
[attributeId]: value
}))
}, [])
const trueIfMobile = useBreakpointValue({base: true, lg: false})

let childProductIds = productViewModalData.product?.bundledProductItems
Expand Down Expand Up @@ -120,6 +156,8 @@ const BundleProductViewModal = ({
}}
childProductOrderability={childProductOrderability}
setSelectedBundleQuantity={setSelectedBundleQuantity}
controlledVariationValues={controlledVariationValues}
onVariationChange={handleVariationChange}
{...props}
/>
</Box>
Expand Down
32 changes: 32 additions & 0 deletions packages/template-retail-react-app/app/components/product-view-modal/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,38 @@ import {useIntl} from 'react-intl'
* A Modal that contains Product View
*/
const ProductViewModal = ({product, isOpen, onClose, ...props}) => {
// Controlled variation values state for modal (doesn't use URL params)
const [controlledVariationValues, setControlledVariationValues] = React.useState({})

const productViewModalData = useProductViewModal(product)

// Auto-select variation attributes with only one value
React.useEffect(() => {
if (!productViewModalData.product?.variationAttributes) return

const autoSelections = {}
productViewModalData.product.variationAttributes.forEach((attr) => {
if (attr.values?.length === 1 && !controlledVariationValues[attr.id]) {
autoSelections[attr.id] = attr.values[0].value
}
})

if (Object.keys(autoSelections).length > 0) {
setControlledVariationValues((prev) => ({
...prev,
...autoSelections
}))
}
}, [productViewModalData.product?.variationAttributes, controlledVariationValues])

// Handle variation changes in controlled mode
const handleVariationChange = React.useCallback((attributeId, value) => {
setControlledVariationValues((prev) => ({
...prev,
[attributeId]: value
}))
}, [])

Copy link
Contributor

@sf-deepali-bharmal sf-deepali-bharmal Oct 30, 2025

Choose a reason for hiding this comment

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

I see all 3 are using same code for auto selection of variant product.
Whats the different between three modals ? whats the use case for each ?

Also its being used at 3 places, can we extract it out in some util/helper ?

Copy link
Contributor

@sf-deepali-bharmal sf-deepali-bharmal Oct 30, 2025

Choose a reason for hiding this comment

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

Also do we need this auto selection now ? Does it help with the 400 bug ? How ?

Copy link
Contributor

@sf-cboscenco sf-cboscenco Oct 30, 2025

Choose a reason for hiding this comment

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

We need to refactor the duplicated code from:

  1. app/components/product-view-modal/index.jsx
  2. app/components/bonus-product-view-modal/index.jsx
  3. app/components/product-view-modal/bundle.jsx

Copy link
Contributor Author

@sf-shikhar-prasoon sf-shikhar-prasoon Oct 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

  • BonusProductViewModal: Shows bonus products that customers can select when they qualify for promotions (e.g., "Buy 2, get 1 free - choose your free item")
  • ProductViewModal: Quick view modal for regular products from product listing pages
  • BundleProductViewModal: Shows products that are part of a bundle on the cart page when editing bundle items

Since we are changing architecture for one modal, we should not let other modals use the URL architecture. All three needed the same refactoring because they all display products with variations that were previously using URL state.

The auto-selection logic was already there before in the old code - we just had to re-implement it in the new React state approach. Previously, auto-selection happened via URL parameters. Now, we're doing it in react state.
If I recall correctly, without it, the modal doesn't fetch image etc on the product-view modal. You had to click the size for it to start displaying the image

Yes, I'll refactor duplicate code

Copy link
Contributor Author

@sf-shikhar-prasoon sf-shikhar-prasoon Oct 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@sf-deepali-bharmal @sf-cboscenco I changed it
Instead of all 3 modals, now only product-view modal will use react state.

A test was failing where clicking the update button on a product view modal was not sending the error notification if the update failed. I couldn't find how fix it and I was suspecting the changes to modals caused this. So I reverted making changes to other modals.

Copy link
Contributor

@sf-deepali-bharmal sf-deepali-bharmal Oct 30, 2025

Choose a reason for hiding this comment

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

@sf-shikhar-prasoon Please test all three modals.

const intl = useIntl()
const label = intl.formatMessage(
{
Expand All @@ -44,6 +74,8 @@ const ProductViewModal = ({product, isOpen, onClose, ...props}) => {
imageSize="sm"
product={productViewModalData.product}
isLoading={productViewModalData.isFetching}
controlledVariationValues={controlledVariationValues}
onVariationChange={handleVariationChange}
{...props}
/>
</ModalBody>
Expand Down
22 changes: 19 additions & 3 deletions packages/template-retail-react-app/app/components/product-view/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ const ProductView = forwardRef(
showDeliveryOptions = true,
customButtons = [],
maxOrderQuantity = null,
imageGalleryFooter = null
imageGalleryFooter = null,
controlledVariationValues = null,
onVariationChange = null
},
ref
) => {
Expand Down Expand Up @@ -183,7 +185,14 @@ const ProductView = forwardRef(
unfulfillable,
isSelectedStoreOutOfStock,
selectedStore
} = useDerivedProduct(product, isProductPartOfSet, isProductPartOfBundle, pickupInStore)
} = useDerivedProduct(
product,
isProductPartOfSet,
isProductPartOfBundle,
pickupInStore,
controlledVariationValues,
onVariationChange
)
const priceData = useMemo(() => {
return getPriceData(product, {quantity})
}, [product, quantity])
Expand Down Expand Up @@ -630,6 +639,11 @@ const ProductView = forwardRef(
},
{variantType: name}
)}
handleChange={
onVariationChange
? (value) => onVariationChange(id, value)
: undefined
}
>
{swatches}
</SwatchGroup>
Expand Down Expand Up @@ -930,7 +944,9 @@ ProductView.propTypes = {
promotionId: PropTypes.string,
maxOrderQuantity: PropTypes.number,
imageGalleryFooter: PropTypes.node,
alignItems: PropTypes.string
alignItems: PropTypes.string,
controlledVariationValues: PropTypes.object,
onVariationChange: PropTypes.func
}

export default ProductView
22 changes: 18 additions & 4 deletions packages/template-retail-react-app/app/hooks/use-derived-product.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ export const useDerivedProduct = (
product,
isProductPartOfSet = false,
isProductPartOfBundle = false,
pickupInStore = false
pickupInStore = false,
controlledVariationValues = null,
onVariationChange = null
) => {
const showLoading = !product
const isProductABundle = product?.type?.bundle
Expand All @@ -40,13 +42,25 @@ export const useDerivedProduct = (
// used for product bundles when there are multiple products
const lowestStockLevelProductName = product?.inventory?.lowestStockLevelProductName
const intl = useIntl()
const variant = useVariant(product, isProductPartOfSet, isProductPartOfBundle)
const variant = useVariant(
product,
isProductPartOfSet,
isProductPartOfBundle,
controlledVariationValues
)
const isStandardProduct = product?.type?.item
const variationParams = useVariationParams(product, isProductPartOfSet, isProductPartOfBundle)
const variationParams = useVariationParams(
product,
isProductPartOfSet,
isProductPartOfBundle,
controlledVariationValues
)
const variationAttributes = useVariationAttributes(
product,
isProductPartOfSet,
isProductPartOfBundle
isProductPartOfBundle,
controlledVariationValues,
onVariationChange
)
const [quantity, setQuantity] = useState(initialQuantity)
const {selectedStore} = useSelectedStore()
Expand Down
33 changes: 6 additions & 27 deletions packages/template-retail-react-app/app/hooks/use-product-view-modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,27 @@
*/

import {useEffect, useState} from 'react'
import {removeQueryParamsFromPath} from '@salesforce/retail-react-app/app/utils/url'
import {useHistory, useLocation} from 'react-router-dom'
import {useVariant} from '@salesforce/retail-react-app/app/hooks/use-variant'
import {useToast} from '@salesforce/retail-react-app/app/hooks/use-toast'
import {useIntl} from 'react-intl'
import {API_ERROR_MESSAGE} from '@salesforce/retail-react-app/app/constants'
import {useProduct} from '@salesforce/commerce-sdk-react'

/**
* This hook is responsible for fetching a product detail based on the variation selection
* and managing the variation params on the url when the modal is open/close
* This hook is responsible for fetching a product detail based on the current product/variant.
* Note: This hook does NOT manage URL params. It expects the modal to manage variation selection
* via React state passed through controlledVariationValues in the hooks chain.
*
* @param initialProduct - the initial product when the modal is first open
* @param queryOptions - optional React Query options to pass to useProduct
* @returns object
* @returns object containing product data and loading state
*/
export const useProductViewModal = (initialProduct, queryOptions = {}) => {
const location = useLocation()
const history = useHistory()
const intl = useIntl()
const toast = useToast()
const [product, setProduct] = useState(initialProduct)
const variant = useVariant(product)

const {data: currentProduct, isFetching} = useProduct(
{parameters: {id: (variant || product)?.productId}},
{parameters: {id: product?.productId}},
{
placeholderData: initialProduct,
...queryOptions,
Expand Down Expand Up @@ -58,25 +54,8 @@ export const useProductViewModal = (initialProduct, queryOptions = {}) => {
if (currentProduct) setProduct(currentProduct)
}, [currentProduct])

const cleanUpVariantParams = () => {
const paramToRemove = [...(product?.variationAttributes?.map(({id}) => id) ?? []), 'pid']
const updatedParams = removeQueryParamsFromPath(`${location.search}`, paramToRemove)

history.replace({search: updatedParams})
}

useEffect(() => {
// when the modal is first mounted,
// clean up the params in case there are variant params not related to current product
cleanUpVariantParams()
return () => {
cleanUpVariantParams()
}
}, [])

return {
product,
variant,
isFetching
}
}
Loading
Loading