diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintsList/EditableConstraintsList.tsx b/frontend/src/component/common/NewConstraintAccordion/ConstraintsList/EditableConstraintsList.tsx index 62b32d5bd1..2817a3e39a 100644 --- a/frontend/src/component/common/NewConstraintAccordion/ConstraintsList/EditableConstraintsList.tsx +++ b/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])} /> ))} diff --git a/frontend/src/component/common/NewConstraintAccordion/NewConstraintAccordionList/NewConstraintAccordionList.tsx b/frontend/src/component/common/NewConstraintAccordion/NewConstraintAccordionList/NewConstraintAccordionList.tsx index 862d5e742e..216c67fd17 100644 --- a/frontend/src/component/common/NewConstraintAccordion/NewConstraintAccordionList/NewConstraintAccordionList.tsx +++ b/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)} /> ) : ( void; - onAutoSave: (constraint: IConstraint) => void; + onUpdate: (constraint: IConstraint) => void; }; export const EditableConstraint: FC = ({ onDelete, constraint, - onAutoSave, + onUpdate, }) => { const { constraint: localConstraint, updateConstraint, validator, - ...legalValueData - } = useEditableConstraint(constraint, onAutoSave); - - const isLegalValueConstraint = 'legalValues' in legalValueData; + legalValueData, + } = useEditableConstraint(constraint, onUpdate); const { context } = useUnleashContext(); const { contextName, operator } = localConstraint; @@ -332,8 +330,7 @@ export const EditableConstraint: FC = ({ values={ isMultiValueConstraint(localConstraint) ? Array.from(localConstraint.values) - : 'legalValues' in legalValueData && - localConstraint.value + : legalValueData && localConstraint.value ? [localConstraint.value] : undefined } @@ -348,7 +345,7 @@ export const EditableConstraint: FC = ({ deleteButtonRef.current } > - {isLegalValueConstraint ? null : ( + {legalValueData ? null : ( = ({ - {'legalValues' in legalValueData ? ( + {legalValueData ? ( {isMultiValueConstraint(localConstraint) ? ( { + 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'])); + }); + }); +}); diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/useEditableConstraint/useEditableConstraint.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/useEditableConstraint/useEditableConstraint.tsx index ffdfcc0e0c..c126bb9b2f 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/useEditableConstraint/useEditableConstraint.tsx +++ b/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 EditableSingleValueConstraint, + type EditableConstraint, fromIConstraint, isSingleValueConstraint, toIConstraint, @@ -44,44 +43,32 @@ const resolveContextDefinition = ( ); }; -type SingleValueConstraintState = { - constraint: EditableSingleValueConstraint; -}; - -type MultiValueConstraintState = { - constraint: EditableMultiValueConstraint; -}; - type LegalValueData = { legalValues: ILegalValue[]; deletedLegalValues?: Set; invalidLegalValues?: Set; }; -type LegalValueConstraintState = { - constraint: EditableMultiValueConstraint; -} & LegalValueData; - type EditableConstraintState = { updateConstraint: (action: ConstraintUpdateAction) => void; validator: (...values: string[]) => ConstraintValidationResult; -} & ( - | SingleValueConstraintState - | MultiValueConstraintState - | LegalValueConstraintState -); + legalValueData?: LegalValueData; + constraint: EditableConstraint; +}; 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)); - - if (contextFieldHasChanged) { - setContextDefinition( - resolveContextDefinition(context, nextState.contextName), - ); - } + onUpdate(toIConstraint(nextState)); }; - if (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, - }; - } + const legalValueData = contextDefinition.legalValues?.length + ? { + legalValues: contextDefinition.legalValues, + invalidLegalValues, + deletedLegalValues, + } + : undefined; return { updateConstraint, constraint: localConstraint, validator, - }; + legalValueData, + } as EditableConstraintState; };