Repository navigation
Show feedback for oversized flag values - #8920
TejasLamba2006 wants to merge 1 commit into
Conversation
|
@TejasLamba2006 is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
Bot
added
the
front-end
Walkthrough
Merge Risk:
 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
63bb7e1d-6210-45e7-b3c7-9131161da1ce
-
frontend/common/utils/__tests__/featureValueValidation.test.tsfrontend/common/utils/featureValueValidation.tsfrontend/web/components/SegmentOverrides.jsfrontend/web/components/ValueEditor/ValueEditor.tsxfrontend/web/components/modals/create-feature/index.tsxfrontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsxfrontend/web/components/mv/VariationValueInput/VariationValueInput.tsx
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
63bb7e1d-6210-45e7-b3c7-9131161da1ce
-
frontend/common/utils/__tests__/featureValueValidation.test.tsfrontend/common/utils/featureValueValidation.tsfrontend/web/components/SegmentOverrides.jsfrontend/web/components/ValueEditor/ValueEditor.tsxfrontend/web/components/modals/create-feature/index.tsxfrontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsxfrontend/web/components/mv/VariationValueInput/VariationValueInput.tsx
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
-
| import { | ||
| getFeatureValueLengthError, | ||
| hasInvalidFeatureValueLength, | ||
| } from 'common/utils/featureValueValidation' | ||
|
|
||
| describe('getFeatureValueLengthError', () => { | ||
| it('accepts values up to the configured maximum', () => { | ||
| expect(getFeatureValueLengthError('x'.repeat(20_000))).toBeUndefined() | ||
| }) | ||
|
|
||
| it('reports the actual and maximum character counts when exceeded', () => { | ||
| expect(getFeatureValueLengthError('x'.repeat(20_001))).toBe( | ||
| 'Flag value is 20,001 characters. Maximum length is 20,000 characters.', | ||
| ) | ||
| }) | ||
|
|
||
| it('counts Unicode code points as characters', () => { | ||
| expect(getFeatureValueLengthError('😀'.repeat(20_001))).toContain( | ||
| '20,001 characters', | ||
| ) | ||
| }) | ||
|
|
||
| it('does not apply the string limit to non-string values', () => { | ||
| expect(getFeatureValueLengthError(20_001)).toBeUndefined() | ||
| }) | ||
|
|
||
| it('detects any over-limit value in a save', () => { | ||
| expect(hasInvalidFeatureValueLength(['valid', 'x'.repeat(20_001)])).toBe( | ||
| true, | ||
| ) | ||
| expect(hasInvalidFeatureValueLength(['valid', 'also valid'])).toBe(false) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Test value-length validation through the save paths.
Add integration tests for oversized control, variant and segment values during flag creation, direct saves and change requests. Assert that saving is blocked and the editor shows an alert. Also assert that saving succeeds when the only oversized segment override is marked for removal.
These tests would protect the save guard and removal exception, which the utility tests do not exercise.
| const getValueLengthErrors = () => | ||
| getFeatureValueLengthErrors([ | ||
| { | ||
| label: identity ? 'Identity override' : 'Flag value', | ||
| value: environmentFlag.feature_state_value, | ||
| }, | ||
| ...(projectFlag.multivariate_options || []).map( | ||
| (option: any, index: number) => ({ | ||
| label: `Variant ${option.key || index + 1} value`, | ||
| value: option.string_value, | ||
| }), | ||
| ), | ||
| ...(segmentOverrides || []) | ||
| .filter((override: any) => !override.toRemove) | ||
| .map((override: any, index: number) => ({ | ||
| label: `Segment override ${override.segment_name || index + 1} value`, | ||
| value: override.value, | ||
| })), | ||
| ]) |
There was a problem hiding this comment.
-
Script executed:
sed -n '320,440p' frontend/web/components/modals/create-feature/index.tsx sed -n '670,780p' frontend/web/components/modals/create-feature/index.tsx
Repository: Flagsmith/flagsmith
Length of output: 9206
Script executed:
printf '%s\n' '--- validation and save callers ---' rg -n -F -e 'getValueLengthErrors' -e 'showValueLengthErrors' -e 'saveFeatureWithValidation' -e 'cleanInputValue' -e 'valueToFeatureState' -- frontend/web/components/modals/create-feature/index.tsx frontend/common/utils/utils.tsx printf '%s\n' '--- save and validation source ---' nl -ba frontend/web/components/modals/create-feature/index.tsx | sed -n '320,485p;590,790p;855,895p' printf '%s\n' '--- conversion implementation ---' nl -ba frontend/common/utils/utils.tsx | sed -n '580,630p;825,870p' printf '%s\n' '--- validator ---' nl -ba frontend/common/utils/featureValueValidation.ts
Repository: Flagsmith/flagsmith
Length of output: 25144
Script executed:
printf '%s\n' '--- wrapper, save branches and change-request callback ---' nl -ba frontend/web/components/modals/create-feature/index.tsx | sed -n '495,535p;760,850p' printf '%s\n' '--- editor state producer ---' nl -ba frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx | sed -n '395,435p' printf '%s\n' '--- payload consumer locations ---' rg -n -F -e 'createChangeRequest(' -e 'editFeatureValue(' -e 'valueToFeatureState(' -- frontend/common/stores/feature-list-store.ts frontend/common/providers/FeatureListProvider.js frontend/web/components/modals/change-request frontend/common/services/useChangeRequest.ts || test "$?" -eq 1 printf '%s\n' '--- backend limits ---' nl -ba api/features/feature_states/models.py | sed -n '20,45p' nl -ba api/features/multivariate/models.py | sed -n '25,42p'
Repository: Flagsmith/flagsmith
Length of output: 9739
Script executed:
printf '%s\n' '--- change request bindings ---' rg -n -F -e 'createChangeRequest' -e 'editFeatureValue' -e 'featureStates=' -- frontend/common/providers/FeatureListProvider.js frontend/common/stores/feature-list-store.ts frontend/web/components/modals/create-feature/index.tsx rg -n -F -e 'function ChangeRequestModal' -e 'const ChangeRequestModal' -e 'class ChangeRequestModal' -- frontend/web/components frontend/common || test "$?" -eq 1 printf '%s\n' '--- provider callbacks ---' nl -ba frontend/common/providers/FeatureListProvider.js | sed -n '1,180p' printf '%s\n' '--- store candidate payloads ---' nl -ba frontend/common/stores/feature-list-store.ts | sed -n '290,370p;410,485p;545,615p;615,680p' printf '%s\n' '--- save wrapper and UI state validation ---' rg -n -F -e 'validateLength' -e 'hasInvalidFeatureValueLength' -- frontend/web/components/ValueEditor/ValueEditor.tsx frontend/web/components/saveFeatureWithValidation* frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx || test "$?" -eq 1
Repository: Flagsmith/flagsmith
Length of output: 21822
Script executed:
printf '%s\n' '--- change request provider and store ownership ---' nl -ba frontend/common/providers/FeatureListProvider.js | sed -n '185,240p' nl -ba frontend/common/stores/feature-list-store.ts | sed -n '510,545p;570,595p;640,667p' printf '%s\n' '--- change request modal use of featureStates ---' rg -n -F -e 'featureStates' -e 'onSave' -e 'no changes' -- frontend/web/components/modals/ChangeRequestModal.tsx nl -ba frontend/web/components/modals/ChangeRequestModal.tsx | sed -n '1,220p' printf '%s\n' '--- frontend validation mechanisms ---' nl -ba frontend/web/components/ValueEditor/ValueEditor.tsx | sed -n '45,90p;115,150p' rg --files frontend/web/components -g '*saveFeatureWithValidation*' rg -n -F -e 'validateValueLengths(' -e 'showValueLengthErrors(' -- frontend/web/components/modals/create-feature/index.tsx
Repository: Flagsmith/flagsmith
Length of output: 18376
Script executed:
printf '%s\n' '--- modal conflict-check invocation ---' nl -ba frontend/web/components/modals/ChangeRequestModal.tsx | sed -n '275,330p' printf '%s\n' '--- conflict-check source ---' rg --files frontend/web/components -g '*ChangeRequestConflictCheck*' rg -n -F -e 'featureStates' -e 'onChange' -e 'hasChanges' -e 'getFeatureStateCrud' -- frontend/web/components/ChangeRequestConflictCheck* printf '%s\n' '--- complete component ---' cat -n frontend/web/components/ChangeRequestConflictCheck.tsx printf '%s\n' '--- save-with-validation helper ---' cat -n frontend/web/components/saveFeatureWithValidation.ts
Repository: Flagsmith/flagsmith
Length of output: 7688
Script executed:
printf '%s\n' '--- change comparison hook ---' cat -n frontend/common/hooks/useHasFeatureStateChanges.ts printf '%s\n' '--- related comparison bindings ---' rg -n -F -e 'getFeatureStateCrud' -e 'hasFeatureStateChanges' -- frontend/common/hooks/useHasFeatureStateChanges.ts frontend/common/utils
Repository: Flagsmith/flagsmith
Length of output: 3250
Script executed:
printf '%s\n' '--- comparison implementation ---' rg -n -F -e 'getFeatureStateCrud' -e 'feature_state_value' -- frontend/common/services/useFeatureVersion.ts nl -ba frontend/common/services/useFeatureVersion.ts | sed -n '1,240p' printf '%s\n' '--- change-request dispatch binding ---' rg -n -F -e 'editEnvironmentFlagChangeRequest' -e 'editFeatureStateChangeRequest' -- frontend/common/dispatcher/app-actions.js frontend/common/stores/feature-list-store.ts printf '%s\n' '--- identity provider payload ---' rg -n -F -e 'identityFlag' -e 'editFeatureValue' -- frontend/common/providers/IdentityProvider.js
Repository: Flagsmith/flagsmith
Length of output: 10846
Script executed:
printf '%s\n' '--- diff implementation ---' rg -n -F -e 'getFeatureStateDiff' -e 'string_value' -- frontend/web/components/diff/diff-utils* nl -ba frontend/web/components/diff/diff-utils.ts | sed -n '1,180p' printf '%s\n' '--- dispatch binding ---' nl -ba frontend/common/dispatcher/app-actions.js | sed -n '120,145p' nl -ba frontend/common/stores/feature-list-store.ts | sed -n '1055,1095p'
Repository: Flagsmith/flagsmith
Length of output: 8719
Validate the flag value sent by save
A flag value with 20,000 characters followed by a space is rejected, although direct save trims it. Change-request payloads also use the trimmed value. Use that value for the modal’s change comparison as well.
Suggested fix
-
- value: environmentFlag.feature_state_value, + value: cleanInputValue(environmentFlag.feature_state_value),
feature_state_value: Utils.valueToFeatureState( - environmentFlag.feature_state_value, + cleanInputValue(environmentFlag.feature_state_value), ),
| const showValueLengthErrors = (errors = getValueLengthErrors()) => { | ||
| errors.forEach((error) => toast(error, 'danger')) | ||
| return errors.length > 0 | ||
| } | ||
|
|
||
| const save = (func: (...args: any[]) => void, isSaving: boolean) => { | ||
| if (showValueLengthErrors()) return |
There was a problem hiding this comment.
-
Script executed:
git diff 13539afcb99c20b4351e26f215d01b49f2a9be03 85c5f6566801e363eeb79fbb6b55ee9f838d393b -- frontend/web/components/modals/create-feature/index.tsx frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx frontend/common/utils/featureValueValidation.ts rg -n -F -e 'showValueLengthErrors' -e 'validateValueLengths' -e 'saveFeatureValue' -e 'saveFeatureSegments' -e 'IdentitySaveFooter' -e 'onCreateFeature' -e 'toast(' frontend/web/components/modals/create-feature/index.tsx frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx nl -ba frontend/web/components/modals/create-feature/index.tsx | sed -n '300,410p;460,535p;580,700p;830,910p' nl -ba frontend/common/utils/featureValueValidation.ts rg -n -F -e 'function toast' -e 'const toast =' -e 'export const toast' -e 'export function toast' frontend
Repository: Flagsmith/flagsmith
Length of output: 27604
Script executed:
nl -ba frontend/web/components/modals/create-feature/index.tsx | sed -n '374,452p;671,833p;897,952p;1115,1170p' nl -ba frontend/web/project/toast.tsx | sed -n '1,240p' rg -n -F -e 'onSaveFeatureValue' -e 'onSaveVariationValues' frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx nl -ba frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx | sed -n '130,260p;450,650p' nl -ba frontend/web/components/saveFeatureWithValidation.tsx 2>/dev/null || nl -ba frontend/web/components/saveFeatureWithValidation.ts rg -n -F -e 'saveFeatureSegments' -e 'onSave' frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab* nl -ba frontend/web/components/modals/create-feature/components/IdentitySaveFooter.tsx
Repository: Flagsmith/flagsmith
Length of output: 40750
Script executed:
rg -n -F -e 'toast-messages' -e 'toast-message' frontend --glob '*.*' rg --files frontend/web/components | rg 'FlagValueFooter|AddVariationButton|VariationOptions|SegmentOverridesTab' rg -n -F -e 'onSaveFeatureValue' -e 'onSaveVariationValues' frontend/web/components/modals/create-feature/components frontend/web/components/modals/create-feature/tabs rg -n -F -e 'max_variations' -e 'maximum variations' -e 'MAX_VARIATION' frontend/web/components/mv frontend/web/components/modals/create-feature frontend/common || test "$?" -eq 1
Repository: Flagsmith/flagsmith
Length of output: 1808
Script executed:
nl -ba frontend/web/styles/components/_toast.scss | sed -n '1,180p' nl -ba frontend/web/styles/project/_utils.scss | sed -n '1,80p' nl -ba frontend/web/components/modals/FlagValueFooter.tsx | sed -n '1,260p' nl -ba frontend/web/components/mv/AddVariationButton.tsx nl -ba frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx | sed -n '125,158p;390,435p;505,556p' rg -n -F -e '<ToastMessages' -e 'ToastMessages' frontend/web
Repository: Flagsmith/flagsmith
Length of output: 19272
Script executed:
nl -ba frontend/web/main.js | sed -n '95,122p' rg -n -F -e 'id="toast"' -e "id='toast'" -e "getElementById('toast')" -e 'getElementById("toast")' frontend/web frontend --glob '*.html' --glob '*.js' --glob '*.tsx' --glob '*.ts'
Repository: Flagsmith/flagsmith
Length of output: 1558
Summarise length errors in one toast.
If several values exceed the limit, one save creates a toast for each value. The stack can cover the editor for five seconds. Show one summary toast while preserving the individual error details.
| const lengthError = validateLength | ||
| ? getFeatureValueLengthError( | ||
| text, | ||
| typeof label === 'string' ? label : 'Value', | ||
| MAX_FEATURE_VALUE_LENGTH, | ||
| ) | ||
| : undefined |
There was a problem hiding this comment.
-
Script executed:
git diff 13539afcb99c20b4351e26f215d01b49f2a9be03 85c5f6566801e363eeb79fbb6b55ee9f838d393b -- frontend/web/components/ValueEditor/ValueEditor.tsx frontend/common/utils/featureValueValidation.ts nl -ba frontend/web/components/ValueEditor/ValueEditor.tsx | sed -n '1,230p' nl -ba frontend/common/utils/featureValueValidation.ts rg -n -F -e 'validateLength' -e 'ValueEditor' -- frontend/web/components/SegmentOverrides.js frontend/web/components/modals/create-feature frontend/web/components/mv/VariationValueInput frontend/web/components/ValueEditor
Repository: Flagsmith/flagsmith
Length of output: 15370
Script executed:
rg --files frontend/web/components -g '*Highlight*' rg -n -F -e 'VariationValueInput' -e 'SegmentOverrides' -e 'onChange' -- frontend/web/components/mv/VariationValueInput/VariationValueInput.tsx frontend/web/components/SegmentOverrides.js frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx nl -ba frontend/web/components/mv/VariationValueInput/VariationValueInput.tsx nl -ba frontend/web/components/SegmentOverrides.js | sed -n '1,340p' nl -ba frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx | sed -n '350,435p' rg -n -F -e 'VariationValueInput' -e 'SegmentOverrides' -- frontend/web/components/mv frontend/web/components/modals/create-feature
Repository: Flagsmith/flagsmith
Length of output: 29163
Script executed:
nl -ba frontend/web/components/Highlight.js nl -ba frontend/web/components/mv/VariationOptions.tsx | sed -n '1,260p' nl -ba frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx | sed -n '550,630p' rg -n -F -e 'MAX_FEATURE_VALUE_LENGTH' -e 'maxLength' -e 'max_length' -- frontend/web/components/Highlight.js frontend/web/components/mv frontend/web/components/ValueEditor frontend/common/utils/featureValueValidation.ts rg -n -F -e 'onEnvironmentFlagChange' -e 'setVariations' -- frontend/web/components/modals/create-feature/index.tsx frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx frontend/web/components/mv/VariationOptions.tsx
Repository: Flagsmith/flagsmith
Length of output: 23105
Script executed:
nl -ba frontend/web/components/modals/create-feature/index.tsx | sed -n '845,910p' nl -ba frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx | sed -n '115,235p' nl -ba frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx | sed -n '510,558p' rg -n -F -e 'updateVariation=' -e 'updateVariation:' -e 'handleUpdateVariation' -e 'setEnvironmentFlag' -- frontend/web/components/modals/create-feature
Repository: Flagsmith/flagsmith
Length of output: 11851
Script executed:
rg -n -F -e 'AddVariationButton' -e 'MAX_VARIATIONS' -e 'max_variations' -e 'MAX_MULTIVARIATE' -- frontend/web/components frontend/common | sed -n '1,140p' rg --files frontend/web/components -g '*AddVariationButton*'
Repository: Flagsmith/flagsmith
Length of output: 785
Script executed:
nl -ba frontend/web/components/mv/AddVariationButton.tsx
Repository: Flagsmith/flagsmith
Length of output: 1063
Script executed:
nl -ba frontend/web/components/modals/create-feature/index.tsx | sed -n '320,405p' nl -ba frontend/web/components/modals/create-feature/index.tsx | sed -n '480,520p' nl -ba frontend/web/components/modals/create-feature/index.tsx | sed -n '590,685p' rg -n -F -e 'getFeatureValueLengthErrors' -e 'hasInvalidFeatureValueLength' -e 'validateValueLengths' -- frontend/web/components/modals/create-feature/index.tsx frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx
Repository: Flagsmith/flagsmith
Length of output: 10558
Script executed:
rg -n -F -e 'valueToFeatureState' -e 'featureStateToValue' -e 'getTypedValue' -- frontend/common/utils/utils.js frontend/common/utils/utils.ts
Repository: Flagsmith/flagsmith
Length of output: 307
Script executed:
rg --files frontend/common/utils | rg '(^|/)utils(/|\.|$)' rg -n -F -e 'valueToFeatureState' -e 'featureStateToValue' -e 'getTypedValue' -- frontend/common/utils
Repository: Flagsmith/flagsmith
Length of output: 4101
Script executed:
nl -ba frontend/common/utils/utils.tsx | sed -n '575,645p;825,865p' nl -ba frontend/common/utils/featureStateToValue.ts
Repository: Flagsmith/flagsmith
Length of output: 5457
Memoise the length check for unchanged editors.
When many variants have large values, editing one variant recounts every unchanged value and allocates an array for each. This can cause typing lag. Memoise the check so unchanged editors reuse their results.
Suggested fix
-
--- "a/frontend/web/components/ValueEditor/ValueEditor.tsx" +++ "b/frontend/web/components/ValueEditor/ValueEditor.tsx" @@ -63,13 +63,17 @@ const language = languageProp ?? picked ?? detected ?? 'txt' const labelId = useId() const text = value === undefined || value === null ? '' : `${value}` - const lengthError = validateLength - ? getFeatureValueLengthError( - text, - typeof label === 'string' ? label : 'Value', - MAX_FEATURE_VALUE_LENGTH, - ) - : undefined + const lengthError = useMemo( + () => + validateLength + ? getFeatureValueLengthError( + text, + typeof label === 'string' ? label : 'Value', + MAX_FEATURE_VALUE_LENGTH, + ) + : undefined, + [text, label, validateLength], + ) // Detection waits for a value rather than running on mount: values load // after mount, and a mount-only check left JSON rendering as plaintext.
Committable suggestion
-
Review the replacement and indentation, then test before committing.Suggested changeconst lengthError = validateLength ? getFeatureValueLengthError( text, typeof label === 'string' ? label : 'Value', MAX_FEATURE_VALUE_LENGTH, ) : undefined const lengthError = useMemo( () => validateLength ? getFeatureValueLengthError( text, typeof label === 'string' ? label : 'Value', MAX_FEATURE_VALUE_LENGTH, ) : undefined, [text, label, validateLength], )
| <Highlight | ||
| aria-labelledby={label ? labelId : undefined} | ||
| aria-readonly={disabled || undefined} | ||
| aria-invalid={!!lengthError || undefined} | ||
| disabled={disabled} | ||
| onChange={disabled ? null : onChange} |
There was a problem hiding this comment.
-
Script executed:
git diff 13539afcb99c20b4351e26f215d01b49f2a9be03 85c5f6566801e363eeb79fbb6b55ee9f838d393b -- frontend/web/components/ValueEditor/ValueEditor.tsx nl -ba frontend/web/components/ValueEditor/ValueEditor.tsx | sed -n '1,220p' rg -n -F -e 'class Highlight' -e 'function Highlight' -e 'const Highlight' -e 'from '\"'\"'components/Highlight'\"'\"'' -- frontend/web frontend/common || test "$?" -eq 1
Repository: Flagsmith/flagsmith
Length of output: 8581
Script executed:
nl -ba frontend/web/components/Highlight.js | sed -n '1,260p' nl -ba frontend/common/utils/featureValueValidation.ts | sed -n '1,140p' rg -n -C 5 -F -- 'validateLength' frontend/web/components/SegmentOverrides.js frontend/web/components/modals/create-feature frontend/web/components/mv/VariationValueInput frontend/web/components/ValueEditor git diff 13539afcb99c20b4351e26f215d01b49f2a9be03 85c5f6566801e363eeb79fbb6b55ee9f838d393b -- frontend/web/components/Highlight.js
Repository: Flagsmith/flagsmith
Length of output: 16366
Forward the invalid state to the editable textbox.
When a value exceeds the limit, Highlight drops aria-invalid, so the textbox does not expose its invalid state to assistive technology. Forward the prop to the <code> element and include it in shouldComponentUpdate so the state updates during editing.
Summary
Closes #8327