diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/AddValuesWidget.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/AddValuesWidget.tsx index a320c6a24c..890b06b211 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/AddValuesWidget.tsx +++ b/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) => void; helpText?: string; validator: (...values: string[]) => ConstraintValidatorOutput; } @@ -60,7 +60,7 @@ export const AddValuesWidget = forwardRef( const [isValid, errorMessage] = validator(...newValues); if (isValid) { - onAddValues(newValues); + onAddValues(new Set(newValues)); clearInput(); setError(''); } else { diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx index e050f9a101..32d1c7dad9 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx @@ -167,6 +167,9 @@ const getInputType = (input: Input): InputType => { } }; +type ConstraintWithValueSet = Omit & { + values?: Set; +}; 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; constraintValues: string[]; @@ -194,6 +197,7 @@ export const EditableConstraint: FC = ({ }) => { const { input } = useConstraintInput({ contextDefinition, + // @ts-ignore localConstraint, }); @@ -352,7 +356,11 @@ export const EditableConstraint: FC = ({ updateConstraint({ type: 'remove value from list', @@ -388,7 +396,7 @@ export const EditableConstraint: FC = ({ contextDefinition.legalValues, )} constraintValues={constraintValues} - values={localConstraint.values || []} + values={localConstraint.values || new Set()} clearAll={() => updateConstraint({ type: 'clear values', diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraintWrapper.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraintWrapper.tsx index 869dded097..83e6d71549 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraintWrapper.tsx +++ b/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 & { + values?: Set; +}; + +const cleanConstraint = ( + constraint: Readonly, +): 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(); diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/LegalValuesSelector.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/LegalValuesSelector.tsx index 972bfcd1f8..82cb89c947 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/LegalValuesSelector.tsx +++ b/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; 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={ onChange(match.value)} name='legal-value' color='primary'