clean some more

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

@@ -28,7 +28,7 @@ const AddValuesButton = styled('button')(({ theme }) => ({
 }));
 
 interface AddValuesProps {
-    onAddValues: (newValues: string[]) => void;
+    onAddValues: (newValues: Set<string>) => void;
     helpText?: string;
     validator: (...values: string[]) => ConstraintValidatorOutput;
 }
@@ -60,7 +60,7 @@ export const AddValuesWidget = forwardRef<HTMLButtonElement, AddValuesProps>(
 
             const [isValid, errorMessage] = validator(...newValues);
             if (isValid) {
-                onAddValues(newValues);
+                onAddValues(new Set(newValues));
                 clearInput();
                 setError('');
             } else {

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

@@ -167,6 +167,9 @@ const getInputType = (input: Input): InputType => {
     }
 };
 
+type ConstraintWithValueSet = Omit<IConstraint, 'values'> & {
+    values?: Set<string>;
+};
 type ConstraintUpdateAction =
     | { type: 'add value(s)'; payload: string[] }
     | { type: 'set value'; payload: string }
@@ -178,7 +181,7 @@ type ConstraintUpdateAction =
     | { type: 'toggle inverted operator' };
 
 type Props = {
-    localConstraint: IConstraint;
+    localConstraint: ConstraintWithValueSet;
     onDelete?: () => void;
     contextDefinition: Pick<IUnleashContextDefinition, 'legalValues'>;
     constraintValues: string[];
@@ -194,6 +197,7 @@ export const EditableConstraint: FC<Props> = ({
 }) => {
     const { input } = useConstraintInput({
         contextDefinition,
+        // @ts-ignore
         localConstraint,
     });
 
@@ -352,7 +356,11 @@ export const EditableConstraint: FC<Props> = ({
                         </OperatorOptions>
                     </ConstraintOptions>
                     <ValueList
-                        values={localConstraint.values}
+                        values={
+                            localConstraint.values
+                                ? Array.from(localConstraint.values)
+                                : undefined
+                        }
                         removeValue={(value) =>
                             updateConstraint({
                                 type: 'remove value from list',
@@ -388,7 +396,7 @@ export const EditableConstraint: FC<Props> = ({
                             contextDefinition.legalValues,
                         )}
                         constraintValues={constraintValues}
-                        values={localConstraint.values || []}
+                        values={localConstraint.values || new Set()}
                         clearAll={() =>
                             updateConstraint({
                                 type: 'clear values',

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

@@ -1,16 +1,17 @@
 import { useEffect, useState } from 'react';
 import type { IConstraint } from 'interfaces/strategy';
-import { cleanConstraint } from 'utils/cleanConstraint';
 import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashContext';
 import type { IUnleashContextDefinition } from 'interfaces/context';
 import {
     DATE_AFTER,
     dateOperators,
     IN,
+    singleValueOperators,
     type Operator,
 } from 'constants/operators';
 import { EditableConstraint } from 'component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint';
 import { CURRENT_TIME_CONTEXT_FIELD } from 'utils/operatorsForContext';
+import produce from 'immer';
 
 interface IConstraintAccordionEditProps {
     constraint: IConstraint;
@@ -50,15 +51,31 @@ type ConstraintUpdateAction =
     | { type: 'toggle case sensitivity' }
     | { type: 'toggle inverted operator' };
 
+type ConstraintWithValueSet = Omit<IConstraint, 'values'> & {
+    values?: Set<string>;
+};
+
+const cleanConstraint = (
+    constraint: Readonly<ConstraintWithValueSet>,
+): ConstraintWithValueSet => {
+    return produce(constraint, (draft) => {
+        if (singleValueOperators.includes(constraint.operator)) {
+            delete draft.values;
+        } else {
+            delete draft.value;
+        }
+    });
+};
+
 export const EditableConstraintWrapper = ({
     constraint,
     onDelete,
     onAutoSave,
 }: IConstraintAccordionEditProps) => {
     const constraintReducer = (
-        state: IConstraint,
+        state: ConstraintWithValueSet,
         action: ConstraintUpdateAction,
-    ): IConstraint => {
+    ): ConstraintWithValueSet => {
         switch (action.type) {
             case 'set context field':
                 if (
@@ -68,7 +85,7 @@ export const EditableConstraintWrapper = ({
                     return cleanConstraint({
                         ...state,
                         operator: DATE_AFTER,
-                        values: [],
+                        values: new Set(),
                         value: new Date().toISOString(),
                     });
                 } else if (
@@ -78,7 +95,8 @@ export const EditableConstraintWrapper = ({
                     return cleanConstraint({
                         ...state,
                         operator: IN,
-                        values: [],
+                        values: new Set(),
+
                         value: '',
                     });
                 }
@@ -86,22 +104,23 @@ export const EditableConstraintWrapper = ({
                 return cleanConstraint({
                     ...state,
                     contextName: action.payload,
-                    values: [],
+                    values: new Set(),
+
                     value: '',
                 });
             case 'set operator':
                 return cleanConstraint({
                     ...state,
                     operator: action.payload,
-                    values: [],
+                    values: new Set(),
+
                     value: '',
                 });
             case 'add value(s)': {
-                const combinedValues = new Set([
-                    ...(state.values || []),
-                    ...action.payload,
-                ]);
-                return { ...state, values: Array.from(combinedValues) };
+                return {
+                    ...state,
+                    values: state.values?.union(new Set(action.payload)),
+                };
             }
             case 'set value':
                 return { ...state, value: action.payload };
@@ -110,25 +129,35 @@ export const EditableConstraintWrapper = ({
             case 'toggle case sensitivity':
                 return { ...state, caseInsensitive: !state.inverted };
             case 'remove value from list':
+                state.values?.delete(action.payload);
                 return {
                     ...state,
-                    values: (state.values ?? []).filter(
-                        (value) => value !== action.payload,
-                    ),
+                    values: state.values ?? new Set(),
                 };
             case 'clear values':
-                return cleanConstraint({ ...state, values: [], value: '' });
+                return cleanConstraint({
+                    ...state,
+                    values: new Set(),
+                    value: '',
+                });
         }
     };
 
-    const [localConstraint, setLocalConstraint] = useState(
-        cleanConstraint(constraint),
-    );
+    const [localConstraint, setLocalConstraint] = useState(() => {
+        const withSet = {
+            ...constraint,
+            values: new Set(constraint.values),
+        };
+        return cleanConstraint(withSet);
+    });
 
     const updateConstraint = (action: ConstraintUpdateAction) => {
         const nextState = constraintReducer(localConstraint, action);
         setLocalConstraint(nextState);
-        onAutoSave(nextState);
+        onAutoSave({
+            ...nextState,
+            values: Array.from(nextState.values ?? []),
+        });
     };
 
     const { context } = useUnleashContext();

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

@@ -1,4 +1,4 @@
-import { useEffect, useState } from 'react';
+import { useState } from 'react';
 import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
 import { Alert, Button, Checkbox, styled } from '@mui/material';
 import type { ILegalValue } from 'interfaces/context';
@@ -14,9 +14,9 @@ interface IRestrictiveLegalValuesProps {
         deletedLegalValues: ILegalValue[];
     };
     constraintValues: string[];
-    values: string[];
+    values: Set<string>;
     addValues: (values: string[]) => void;
-    removeValue: (values: string) => void;
+    removeValue: (value: string) => void;
     clearAll: () => void;
     beforeValues?: JSX.Element;
 }
@@ -83,9 +83,6 @@ export const LegalValuesSelector = ({
 
     const filteredValues = filterLegalValues(legalValues, filter);
 
-    // Lazily initialise the values because there might be a lot of them.
-    const [valuesMap, setValuesMap] = useState(() => createValuesMap(values));
-
     const cleanDeletedLegalValues = (constraintValues: string[]): string[] => {
         const deletedValuesSet = getLegalValueSet(deletedLegalValues);
         return (
@@ -99,29 +96,25 @@ export const LegalValuesSelector = ({
         deletedLegalValues,
     );
 
-    useEffect(() => {
-        setValuesMap(createValuesMap(values));
-    }, [values, setValuesMap, createValuesMap]);
-
-    useEffect(() => {
-        if (illegalValues.length > 0) {
-            // todo: impl this
-            console.log('would clean deleted values here');
-            // setValues(cleanDeletedLegalValues(values));
-        }
-    }, []);
+    //todo: maybe move this up?
+    // useEffect(() => {
+    //     if (illegalValues.length > 0) {
+    //         removeValues(...illegalValues)
+    //         // todo: impl this
+    //         console.log('would clean deleted values here');
+    //         // setvalues(cleandeletedlegalvalues(values));
+    //     }
+    // }, []);
 
     const onChange = (legalValue: string) => {
-        if (valuesMap[legalValue]) {
+        if (values.has(legalValue)) {
             removeValue(legalValue);
         } else {
             addValues([legalValue]);
         }
     };
 
-    const isAllSelected = legalValues.every((value) =>
-        values.includes(value.value),
-    );
+    const isAllSelected = legalValues.every((value) => values.has(value.value));
 
     const onSelectAll = () => {
         if (isAllSelected) {
@@ -185,7 +178,7 @@ export const LegalValuesSelector = ({
                         filter={filter}
                         control={
                             <Checkbox
-                                checked={Boolean(valuesMap[match.value])}
+                                checked={Boolean(values.has(match.value))}
                                 onChange={() => onChange(match.value)}
                                 name='legal-value'
                                 color='primary'