fix(identities): multivariate override editor hides the identity's override value

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)
+      },
+    )
+  })
 })

frontend/common/utils/multivariate.ts

@@ -1,10 +1,31 @@
+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
 // pulling in Utils' store dependencies (Storybook stubs out Utils).
 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 = <T extends { id?: number | null }>(
   options: T[],

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<CreateFeatureModalProps> = (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<CreateFeatureModalProps> = (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,
           }),

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<FeatureValueTabProps> = ({
     !!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 (
       <div className={`${identity ? 'mx-3' : ''}`}>
@@ -412,14 +430,15 @@ const FeatureValueTab: FC<FeatureValueTabProps> = ({
         <div>
           <FormGroup className='mb-4'>
             {variationsInfo}
+            {!!unmatchedOverride && (
+              <WarningMessage warningMessage="This identity override contains a value that is not one of this flag's variations. We recommend changing it." />
+            )}
             <VariationOptions
               canCreateFeature={false}
               disabled
               select
-              controlValue={
-                projectFlag.environment_feature_state?.feature_state_value ??
-                null
-              }
+              unmatchedOverride={unmatchedOverride}
+              controlValue={controlValue}
               controlPercentage={controlPercentage}
               variationOverrides={identityVariations as any}
               setValue={(value) =>

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<VariationOptionsProps> = ({
   select,
   setValue,
   setVariations,
+  unmatchedOverride,
   unsavedVariations,
   updateVariation,
   variationOverrides,
@@ -56,8 +61,9 @@ export const VariationOptions: React.FC<VariationOptionsProps> = ({
     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<VariationOptionsProps> = ({
           error='Your variation percentage splits total to over 100%'
         />
       )}
+      {select && !!unmatchedOverride && (
+        <div className='panel panel--flat panel-without-heading mb-2'>
+          <div className='panel-content'>
+            <Row>
+              <Flex>
+                <ValueEditor
+                  disabled
+                  value={Utils.getTypedValue(unmatchedOverride.value)}
+                />
+              </Flex>
+              <div className='btn-radio btn-radio-on ml-2' />
+            </Row>
+          </div>
+        </div>
+      )}
       {select && (
         <div className='panel panel--flat panel-without-heading mb-2'>
           <div className='panel-content'>