-
Notifications
You must be signed in to change notification settings - Fork 214
@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.
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
develop
from
t/cc-sharks/W-20038640/400Error/main
Oct 30, 2025
Merged
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
35c14ae
fix
sf-shikhar-prasoon fbe45f6
add/update tests
sf-shikhar-prasoon 522a602
lint fix
sf-shikhar-prasoon c753399
fix tests?
sf-shikhar-prasoon ae16d45
variant selection fix, test fix
sf-shikhar-prasoon cd8c75a
refactor- duplicate code
sf-shikhar-prasoon 351310f
fix useVariant if product is already a variant
sf-shikhar-prasoon 303104c
only use react state for product-view modal
sf-shikhar-prasoon 9048a9f
removed the unnecessary useCallback
sf-shikhar-prasoon d4d6311
add tests- Variation State Reset | URL Pollution Prevention
sf-shikhar-prasoon 83d33af
Merge branch 'develop' into t/cc-sharks/W-20038640/400Error/main
sf-shikhar-prasoon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
58 changes: 58 additions & 0 deletions
58
packages/template-retail-react-app/app/hooks/use-controlled-variations.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| /* | ||
| * Copyright (c) 2021, salesforce.com, inc. | ||
| * All rights reserved. | ||
| * SPDX-License-Identifier: BSD-3-Clause | ||
| * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause | ||
| */ | ||
|
|
||
| import {useState, useEffect, useCallback} from 'react' | ||
|
|
||
| /** | ||
| * Custom hook for managing controlled variation state in modals. | ||
| * Provides React state-based variation management (instead of URL parameters). | ||
| * | ||
| * Features: | ||
| * - Manages variation selection state | ||
| * - Auto-selects single-value variation attributes | ||
| * - Provides callback for handling variation changes | ||
| * | ||
| * @param {Object} product - The product object with variationAttributes | ||
| * @returns {Object} - { controlledVariationValues, handleVariationChange } | ||
| */ | ||
| export const useControlledVariations = (product) => { | ||
| const [controlledVariationValues, setControlledVariationValues] = useState({}) | ||
|
|
||
| // Auto-select variation attributes with only one value | ||
| useEffect(() => { | ||
| if (!product?.variationAttributes) return | ||
|
|
||
| const autoSelections = {} | ||
| 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 | ||
| })) | ||
| } | ||
| }, [product?.variationAttributes, controlledVariationValues]) | ||
|
|
||
| // Handle variation changes in controlled mode | ||
| const handleVariationChange = useCallback((attributeId, value) => { | ||
| setControlledVariationValues((prev) => ({ | ||
| ...prev, | ||
| [attributeId]: value | ||
| })) | ||
| }, []) | ||
|
|
||
| return { | ||
| controlledVariationValues, | ||
| handleVariationChange | ||
| } | ||
| } | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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:
app/components/product-view-modal/index.jsxapp/components/bonus-product-view-modal/index.jsxapp/components/product-view-modal/bundle.jsxUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.