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 && (