From 66eeee446e3fe2c087a994376e67129725306ab0 Mon Sep 17 00:00:00 2001 From: Deep Santoshwar Date: Thu, 13 Aug 2026 01:59:56 +0530 Subject: [PATCH] fix(identities): multivariate override editor hides the identity's override value The editor for a multivariate flag only renders the environment's control value and each variation as radios, so an identity override holding any other value has nowhere to appear. The control row reads as selected and the identity looks like it is on the environment default, even though the SDK still serves the override. Saving the modal then replaced the override with the control value. Show such a value as a read-only, selected row alongside the variations, warn that it is not one of them, and keep it on save unless the user picks the control or a variation instead. --- .../utils/__tests__/multivariate.test.ts | 63 +++++++++++++++++++ frontend/common/utils/multivariate.ts | 21 +++++++ .../modals/create-feature/index.tsx | 19 +++++- .../create-feature/tabs/FeatureValueTab.tsx | 27 ++++++-- .../web/components/mv/VariationOptions.tsx | 25 +++++++- 5 files changed, 146 insertions(+), 9 deletions(-) diff --git a/frontend/common/utils/__tests__/multivariate.test.ts b/frontend/common/utils/__tests__/multivariate.test.ts index 270954873854..292e14c4336e 100644 --- a/frontend/common/utils/__tests__/multivariate.test.ts +++ b/frontend/common/utils/__tests__/multivariate.test.ts @@ -1,5 +1,6 @@ import { getDefaultVariantKey, + hasUnmatchedIdentityOverride, sortMultivariateOptions, } from 'common/utils/multivariate' @@ -51,4 +52,66 @@ describe('multivariate', () => { expect(options).toEqual([{ id: 2 }, { id: 1 }]) }) }) + + describe('hasUnmatchedIdentityOverride', () => { + it('detects an override kept from before the flag became multivariate', () => { + expect( + hasUnmatchedIdentityOverride({ + controlValue: 'ENV_DEFAULT', + overrideValue: 'MY_OVERRIDE', + variationOverrides: [], + }), + ).toBe(true) + }) + + it('does not flag an identity sitting on the control value', () => { + expect( + hasUnmatchedIdentityOverride({ + controlValue: 'ENV_DEFAULT', + overrideValue: 'ENV_DEFAULT', + variationOverrides: [], + }), + ).toBe(false) + }) + + it('does not flag an identity assigned a variation', () => { + expect( + hasUnmatchedIdentityOverride({ + controlValue: 'ENV_DEFAULT', + overrideValue: 'MY_OVERRIDE', + variationOverrides: [{ percentage_allocation: 100 }], + }), + ).toBe(false) + }) + + it('flags a partially weighted override, which does not pin a variation', () => { + expect( + hasUnmatchedIdentityOverride({ + controlValue: 'ENV_DEFAULT', + overrideValue: 'MY_OVERRIDE', + variationOverrides: [{ percentage_allocation: 60 }], + }), + ).toBe(true) + }) + + it.each` + controlValue | overrideValue | expected + ${null} | ${undefined} | ${false} + ${undefined} | ${null} | ${false} + ${null} | ${''} | ${true} + ${''} | ${null} | ${true} + ${0} | ${false} | ${true} + `( + 'treats control $controlValue against override $overrideValue as $expected', + ({ controlValue, expected, overrideValue }) => { + expect( + hasUnmatchedIdentityOverride({ + controlValue, + overrideValue, + variationOverrides: undefined, + }), + ).toBe(expected) + }, + ) + }) }) diff --git a/frontend/common/utils/multivariate.ts b/frontend/common/utils/multivariate.ts index 2dbf7d0e1540..e53b624a9fd8 100644 --- a/frontend/common/utils/multivariate.ts +++ b/frontend/common/utils/multivariate.ts @@ -1,3 +1,5 @@ +import { FlagsmithValue } from 'common/types/responses' + // The label a variant displays (and is saved with) when the user never // sets one — keep display, validation and save payloads consistent. // Kept outside Utils so Storybook-rendered components can use it without @@ -5,6 +7,25 @@ export const getDefaultVariantKey = (index: number): string => `Variant_${index + 1}` +// An identity override made before its flag became multivariate keeps a +// free-form value. The multivariate editor only offers the environment's +// control value and each variation, so such a value has nowhere to appear: +// the control row reads as selected and the identity looks like it is on the +// environment default. Detect it so the editor can show the value, and so +// saving does not quietly replace it with the control value. +export const hasUnmatchedIdentityOverride = ({ + controlValue, + overrideValue, + variationOverrides, +}: { + controlValue: FlagsmithValue + overrideValue: FlagsmithValue + variationOverrides: { percentage_allocation: number }[] | null | undefined +}): boolean => + !variationOverrides?.some( + (variation) => variation.percentage_allocation === 100, + ) && (overrideValue ?? null) !== (controlValue ?? null) + // Options not yet saved have no id and sort last, in input order. export const sortMultivariateOptions = ( options: T[], diff --git a/frontend/web/components/modals/create-feature/index.tsx b/frontend/web/components/modals/create-feature/index.tsx index d5346bd15c17..26e132f38619 100644 --- a/frontend/web/components/modals/create-feature/index.tsx +++ b/frontend/web/components/modals/create-feature/index.tsx @@ -27,6 +27,7 @@ import ExternalResourcesTable from 'components/ExternalResourcesTable' import GitHubLinkSection from 'components/GitHubLinkSection' import GitLabLinkSection from 'components/GitLabLinkSection' import type { ExternalResource } from 'common/types/responses' +import { hasUnmatchedIdentityOverride } from 'common/utils/multivariate' import { saveFeatureWithValidation } from 'components/saveFeatureWithValidation' import FeatureHistory from 'components/FeatureHistory' import { getChangeRequests } from 'common/services/useChangeRequest' @@ -330,6 +331,17 @@ const CreateFeatureModal: FC = (props) => { const hasMultivariate = props.environmentFlag?.multivariate_feature_state_values?.length + // A multivariate override is stored as the control value plus a variation + // at 100%, so its value is normally re-synced to the control on save. An + // override predating the flag becoming multivariate holds a value that + // this model cannot express, and re-syncing would destroy it. + const keepsOwnValue = hasUnmatchedIdentityOverride({ + controlValue: + projectFlag.environment_feature_state?.feature_state_value ?? null, + overrideValue: environmentFlag.feature_state_value ?? null, + variationOverrides: environmentFlag.multivariate_feature_state_values, + }) + if (identity) { !isSaving && projectFlag.name && @@ -339,9 +351,10 @@ const CreateFeatureModal: FC = (props) => { identity, identityFlag: Object.assign({}, props.identityFlag || {}, { enabled: environmentFlag.enabled, - feature_state_value: hasMultivariate - ? props.environmentFlag?.feature_state_value - : cleanInputValue(environmentFlag.feature_state_value), + feature_state_value: + hasMultivariate && !keepsOwnValue + ? props.environmentFlag?.feature_state_value + : cleanInputValue(environmentFlag.feature_state_value), multivariate_options: environmentFlag.multivariate_feature_state_values, }), diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx index cad9f93846e9..e484fb99c410 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx @@ -20,6 +20,7 @@ import { MultivariateOption, ProjectFlag, } from 'common/types/responses' +import { hasUnmatchedIdentityOverride } from 'common/utils/multivariate' import { FeatureExperimentFreeze } from 'common/hooks/useFeatureExperimentFreeze' import ExperimentFreezeNotice from 'components/modals/create-feature/components/ExperimentFreezeNotice' import { useHasPermission } from 'common/providers/Permission' @@ -281,6 +282,23 @@ const FeatureValueTab: FC = ({ !!multivariate_options.length ) + const controlValue = + projectFlag.environment_feature_state?.feature_state_value ?? null + + // An override that predates the flag becoming multivariate holds a value the + // control/variation radios cannot express, so surface it rather than letting + // the control row imply the identity is on the environment default. + const unmatchedOverride = + !!identity && + hasVariations && + hasUnmatchedIdentityOverride({ + controlValue, + overrideValue: featureState.feature_state_value ?? null, + variationOverrides: identityVariations, + }) + ? { value: featureState.feature_state_value ?? null } + : undefined + if (compareOpen && canCompareValue && environmentId) { return (
@@ -412,14 +430,15 @@ const FeatureValueTab: FC = ({
{variationsInfo} + {!!unmatchedOverride && ( + + )} diff --git a/frontend/web/components/mv/VariationOptions.tsx b/frontend/web/components/mv/VariationOptions.tsx index af3d7eed84a3..9ef7c71c9e1b 100644 --- a/frontend/web/components/mv/VariationOptions.tsx +++ b/frontend/web/components/mv/VariationOptions.tsx @@ -22,6 +22,10 @@ interface VariationOptionsProps { readOnly?: boolean removeVariation: (i: number) => void select?: boolean + // An override value that is neither the control value nor one of the + // variations. Shown read-only and selected, so the identity does not read as + // being on the control value. + unmatchedOverride?: { value: FlagsmithValue } setValue: (value: FlagsmithValue) => void setVariations: (variations: VariationOverride[]) => void unsavedVariations?: boolean[] @@ -46,6 +50,7 @@ export const VariationOptions: React.FC = ({ select, setValue, setVariations, + unmatchedOverride, unsavedVariations, updateVariation, variationOverrides, @@ -56,8 +61,9 @@ export const VariationOptions: React.FC = ({ return null } const controlSelected = - !variationOverrides || - !variationOverrides.find((v) => v.percentage_allocation === 100) + !unmatchedOverride && + (!variationOverrides || + !variationOverrides.find((v) => v.percentage_allocation === 100)) return ( <> {invalid && ( @@ -66,6 +72,21 @@ export const VariationOptions: React.FC = ({ error='Your variation percentage splits total to over 100%' /> )} + {select && !!unmatchedOverride && ( +
+
+ + + + +
+ +
+
+ )} {select && (