test/refactor: useEditableConstraint hook (#9970)

Adds a test suite for the useEditableConstraint hook, attempting to test
all the parts of it that we can't test in isolation.

Plus: a few, small refactorings:
- Renames `onAutoSave` on `onUpdate` to better match `onDelete` (and
because autosave doesn't really mean anything anymore).
- Simplifies and collapses some types

frontend/src/component/common/NewConstraintAccordion/ConstraintsList/EditableConstraintsList.tsx

@@ -75,7 +75,7 @@ export const EditableConstraintsList = forwardRef<
                         key={constraint[constraintId]}
                         constraint={constraint}
                         onDelete={() => onDelete(index)}
-                        onAutoSave={onAutoSave(constraint[constraintId])}
+                        onUpdate={onAutoSave(constraint[constraintId])}
                     />
                 ))}
             </ConstraintsList>

frontend/src/component/common/NewConstraintAccordion/NewConstraintAccordionList/NewConstraintAccordionList.tsx

@@ -122,7 +122,7 @@ export const NewConstraintAccordionList = forwardRef<
                                 // @ts-ignore todo: find a better way to do this
                                 onDelete={() => onRemove(index)}
                                 // @ts-ignore
-                                onAutoSave={onAutoSave(constraintId)}
+                                onUpdate={onAutoSave(constraintId)}
                             />
                         ) : (
                             <ConstraintAccordionView

frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx

@@ -218,22 +218,20 @@ const TopRowInput: FC<{
 type Props = {
     constraint: IConstraint;
     onDelete: () => void;
-    onAutoSave: (constraint: IConstraint) => void;
+    onUpdate: (constraint: IConstraint) => void;
 };
 
 export const EditableConstraint: FC<Props> = ({
     onDelete,
     constraint,
-    onAutoSave,
+    onUpdate,
 }) => {
     const {
         constraint: localConstraint,
         updateConstraint,
         validator,
-        ...legalValueData
+        legalValueData,
-    } = useEditableConstraint(constraint, onAutoSave);
+    } = useEditableConstraint(constraint, onUpdate);
-
-    const isLegalValueConstraint = 'legalValues' in legalValueData;
 
     const { context } = useUnleashContext();
     const { contextName, operator } = localConstraint;
@@ -332,8 +330,7 @@ export const EditableConstraint: FC<Props> = ({
                         values={
                             isMultiValueConstraint(localConstraint)
                                 ? Array.from(localConstraint.values)
-                                : 'legalValues' in legalValueData &&
+                                : legalValueData && localConstraint.value
-                                    localConstraint.value
                                   ? [localConstraint.value]
                                   : undefined
                         }
@@ -348,7 +345,7 @@ export const EditableConstraint: FC<Props> = ({
                             deleteButtonRef.current
                         }
                     >
-                        {isLegalValueConstraint ? null : (
+                        {legalValueData ? null : (
                             <TopRowInput
                                 localConstraint={localConstraint}
                                 updateConstraint={updateConstraint}
@@ -371,7 +368,7 @@ export const EditableConstraint: FC<Props> = ({
                     </StyledIconButton>
                 </HtmlTooltip>
             </TopRow>
-            {'legalValues' in legalValueData ? (
+            {legalValueData ? (
                 <LegalValuesContainer>
                     {isMultiValueConstraint(localConstraint) ? (
                         <LegalValuesSelector

frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/useEditableConstraint/useEditableConstraint.test.tsx

@@ -0,0 +1,247 @@
+import { renderHook, waitFor } from '@testing-library/react';
+import {
+    dateOperators,
+    IN,
+    multipleValueOperators,
+    NOT_IN,
+    numOperators,
+    semVerOperators,
+} from 'constants/operators';
+import type { IConstraint } from 'interfaces/strategy';
+import { useEditableConstraint } from './useEditableConstraint';
+import { vi } from 'vitest';
+import { testServerRoute, testServerSetup } from 'utils/testServer';
+import type { ContextFieldSchema } from 'openapi';
+import { NUM_EQ } from '@server/util/constants';
+
+const server = testServerSetup();
+
+const setupApi = (contextField?: ContextFieldSchema) => {
+    testServerRoute(server, '/api/admin/context', [contextField]);
+};
+
+test('calls onUpdate with new state', () => {
+    const initial: IConstraint = {
+        contextName: 'context-field',
+        operator: NOT_IN,
+        values: ['A', 'B'],
+    };
+
+    const onUpdate = vi.fn();
+
+    const { result } = renderHook(() =>
+        useEditableConstraint(initial, onUpdate),
+    );
+    result.current.updateConstraint({
+        type: 'set operator',
+        payload: IN,
+    });
+
+    expect(onUpdate).toHaveBeenCalledWith({
+        contextName: 'context-field',
+        operator: IN,
+        values: [],
+    });
+});
+
+describe('validators', () => {
+    const checkValidator = (
+        validator: (...values: string[]) => [boolean, string],
+        expectations: [string | string[], boolean][],
+    ) => {
+        expect(
+            expectations.every(([value, outcome]) =>
+                Array.isArray(value)
+                    ? validator(...value)[0] === outcome
+                    : validator(value)[0] === outcome,
+            ),
+        ).toBe(true);
+    };
+
+    test.each(numOperators)(
+        'picks the right validator for num operator: %s',
+        (operator) => {
+            const initial: IConstraint = {
+                contextName: 'context-field',
+                operator: operator,
+                value: '',
+            };
+
+            const { result } = renderHook(() =>
+                useEditableConstraint(initial, () => {}),
+            );
+
+            checkValidator(result.current.validator, [
+                ['5', true],
+                ['5.6', true],
+                ['5,6', false],
+                ['not a number', false],
+                ['1.2.6', false],
+                ['2025-05-13T07:39:23.053Z', false],
+            ]);
+        },
+    );
+    test.each(semVerOperators)(
+        'picks the right validator for semVer operator: %s',
+        (operator) => {
+            const initial: IConstraint = {
+                contextName: 'context-field',
+                operator: operator,
+                value: '',
+            };
+
+            const { result } = renderHook(() =>
+                useEditableConstraint(initial, () => {}),
+            );
+
+            checkValidator(result.current.validator, [
+                ['5', false],
+                ['5.6', false],
+                ['5,6', false],
+                ['not a number', false],
+                ['1.2.6', true],
+                ['2025-05-13T07:39:23.053Z', false],
+            ]);
+        },
+    );
+    test.each(dateOperators)(
+        'picks the right validator for date operator: %s',
+        (operator) => {
+            const initial: IConstraint = {
+                contextName: 'context-field',
+                operator: operator,
+                value: '',
+            };
+
+            const { result } = renderHook(() =>
+                useEditableConstraint(initial, () => {}),
+            );
+
+            checkValidator(result.current.validator, [
+                ['5', false],
+                ['5.6', false],
+                ['5,6', false],
+                ['not a number', false],
+                ['1.2.6', false],
+                ['2025-05-13T07:39:23.053Z', true],
+            ]);
+        },
+    );
+    test.each(multipleValueOperators)(
+        'picks the right value for multi-value operator: %s',
+        (operator) => {
+            const initial: IConstraint = {
+                contextName: 'context-field',
+                operator: operator,
+                values: [],
+            };
+
+            const { result } = renderHook(() =>
+                useEditableConstraint(initial, () => {}),
+            );
+
+            checkValidator(result.current.validator, [
+                ['5', true],
+                [['hey'], true],
+                // @ts-expect-error
+                [[5, 6], false],
+            ]);
+        },
+    );
+});
+
+describe('legal values', () => {
+    const definition = {
+        name: 'context-field',
+        legalValues: [{ value: 'A' }, { value: '6' }],
+    };
+    setupApi(definition);
+
+    test('provides them if present', async () => {
+        const initial: IConstraint = {
+            contextName: definition.name,
+            operator: IN,
+            values: [],
+        };
+
+        const { result } = renderHook(() =>
+            useEditableConstraint(initial, () => {}),
+        );
+        await waitFor(() => {
+            expect(result.current.legalValueData?.legalValues).toStrictEqual(
+                definition.legalValues,
+            );
+        });
+    });
+
+    test('updates context definition when changing context field', async () => {
+        const initial: IConstraint = {
+            contextName: definition.name,
+            operator: IN,
+            values: [],
+        };
+
+        const { result } = renderHook(() =>
+            useEditableConstraint(initial, () => {}),
+        );
+        await waitFor(() => {
+            expect(result.current.legalValueData?.legalValues).toStrictEqual(
+                definition.legalValues,
+            );
+        });
+
+        result.current.updateConstraint({
+            type: 'set context field',
+            payload: 'field-without-legal-values',
+        });
+
+        await waitFor(() => {
+            expect(result.current.legalValueData).toBeUndefined();
+        });
+    });
+    test('does not add them if no legal values', () => {
+        const initial: IConstraint = {
+            contextName: 'field-with-no-legal-values',
+            operator: IN,
+            values: [],
+        };
+
+        const { result } = renderHook(() =>
+            useEditableConstraint(initial, () => {}),
+        );
+
+        expect(result.current.legalValueData).toBeUndefined();
+    });
+    test('identifies deleted legal values', async () => {
+        const initial: IConstraint = {
+            contextName: definition.name,
+            operator: IN,
+            values: ['A', 'B'],
+        };
+
+        const { result } = renderHook(() =>
+            useEditableConstraint(initial, () => {}),
+        );
+        await waitFor(() => {
+            expect(
+                result.current.legalValueData?.deletedLegalValues,
+            ).toStrictEqual(new Set(['B']));
+        });
+    });
+    test('identifies invalid legal values', async () => {
+        const initial: IConstraint = {
+            contextName: definition.name,
+            operator: NUM_EQ,
+            values: [],
+        };
+
+        const { result } = renderHook(() =>
+            useEditableConstraint(initial, () => {}),
+        );
+        await waitFor(() => {
+            expect(
+                result.current.legalValueData?.invalidLegalValues,
+            ).toStrictEqual(new Set(['A']));
+        });
+    });
+});

frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/useEditableConstraint/useEditableConstraint.tsx

@@ -2,8 +2,7 @@ import { useMemo, useState } from 'react';
 import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashContext';
 import type { IConstraint } from 'interfaces/strategy';
 import {
-    type EditableMultiValueConstraint,
+    type EditableConstraint,
-    type EditableSingleValueConstraint,
     fromIConstraint,
     isSingleValueConstraint,
     toIConstraint,
@@ -44,44 +43,32 @@ const resolveContextDefinition = (
     );
 };
 
-type SingleValueConstraintState = {
-    constraint: EditableSingleValueConstraint;
-};
-
-type MultiValueConstraintState = {
-    constraint: EditableMultiValueConstraint;
-};
-
 type LegalValueData = {
     legalValues: ILegalValue[];
     deletedLegalValues?: Set<string>;
     invalidLegalValues?: Set<string>;
 };
 
-type LegalValueConstraintState = {
-    constraint: EditableMultiValueConstraint;
-} & LegalValueData;
-
 type EditableConstraintState = {
     updateConstraint: (action: ConstraintUpdateAction) => void;
     validator: (...values: string[]) => ConstraintValidationResult;
-} & (
+    legalValueData?: LegalValueData;
-    | SingleValueConstraintState
+    constraint: EditableConstraint;
-    | MultiValueConstraintState
+};
-    | LegalValueConstraintState
-);
 
 export const useEditableConstraint = (
     constraint: IConstraint,
-    onAutoSave: (constraint: IConstraint) => void,
+    onUpdate: (constraint: IConstraint) => void,
 ): EditableConstraintState => {
     const [localConstraint, setLocalConstraint] = useState(() => {
         return fromIConstraint(constraint);
     });
 
     const { context } = useUnleashContext();
-    const [contextDefinition, setContextDefinition] = useState(
+
-        resolveContextDefinition(context, localConstraint.contextName),
+    const contextDefinition = useMemo(
+        () => resolveContextDefinition(context, localConstraint.contextName),
+        [JSON.stringify(context), localConstraint.contextName],
     );
 
     const validator = constraintValidator(localConstraint);
@@ -124,50 +111,23 @@ export const useEditableConstraint = (
             action,
             deletedLegalValues,
         );
-        const contextFieldHasChanged =
-            localConstraint.contextName !== nextState.contextName;
 
         setLocalConstraint(nextState);
-        onAutoSave(toIConstraint(nextState));
+        onUpdate(toIConstraint(nextState));
-
-        if (contextFieldHasChanged) {
-            setContextDefinition(
-                resolveContextDefinition(context, nextState.contextName),
-            );
-        }
     };
 
-    if (contextDefinition.legalValues?.length) {
+    const legalValueData = contextDefinition.legalValues?.length
-        if (isSingleValueConstraint(localConstraint)) {
+        ? {
-            return {
-                updateConstraint,
-                constraint: localConstraint,
-                validator,
               legalValues: contextDefinition.legalValues,
               invalidLegalValues,
               deletedLegalValues,
-            };
-        }
-        return {
-            updateConstraint,
-            constraint: localConstraint,
-            validator,
-            legalValues: contextDefinition.legalValues,
-            invalidLegalValues,
-            deletedLegalValues,
-        };
-    }
-    if (isSingleValueConstraint(localConstraint)) {
-        return {
-            updateConstraint,
-            constraint: localConstraint,
-            validator,
-        };
           }
+        : undefined;
 
     return {
         updateConstraint,
         constraint: localConstraint,
         validator,
-    };
+        legalValueData,
+    } as EditableConstraintState;
 };