From f9a686ca81fa2246d0fee15484e5cee6b405662e Mon Sep 17 00:00:00 2001 From: andreas-unleash Date: Fri, 11 Aug 2023 13:44:28 +0300 Subject: [PATCH] Fix/constraint with legal value that has been deleted (#4473) When editing a constraint that uses a context field with legal values: if the contraint has a value that has been deleted from the legal values of the context field: - Show the value and mark it as disabled - On any change -> 'cleans'/removed the deleted legal values from the constraint values Closes: [1-1209](https://linear.app/unleash/issue/1-1209/if-i-modified-the-legal-values-of-a-used-context-field) --------- Signed-off-by: andreas-unleash --- .../ResolveInput/ResolveInput.tsx | 35 ++++++++---- .../RestrictiveLegalValues.test.tsx | 54 +++++++++++++++++++ .../RestrictiveLegalValues.tsx | 28 ++++++++-- 3 files changed, 101 insertions(+), 16 deletions(-) create mode 100644 frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.test.tsx diff --git a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ResolveInput/ResolveInput.tsx b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ResolveInput/ResolveInput.tsx index a957d1df4e..0ebcd28930 100644 --- a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ResolveInput/ResolveInput.tsx +++ b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ResolveInput/ResolveInput.tsx @@ -1,4 +1,4 @@ -import { IUnleashContextDefinition } from 'interfaces/context'; +import { ILegalValue, IUnleashContextDefinition } from 'interfaces/context'; import { IConstraint } from 'interfaces/strategy'; import { DateSingleValue } from '../DateSingleValue/DateSingleValue'; import { FreeTextInput } from '../FreeTextInput/FreeTextInput'; @@ -30,6 +30,25 @@ interface IResolveInputProps { error: string; } +const resolveLegalValues = ( + values: IConstraint['values'], + legalValues: IUnleashContextDefinition['legalValues'] +): { legalValues: ILegalValue[]; deletedLegalValues: ILegalValue[] } => { + const deletedLegalValues = (values || []) + .filter( + value => + !(legalValues || []).some( + ({ value: legalValue }) => legalValue === value + ) + ) + .map(v => ({ value: v, description: '' })); + + return { + legalValues: legalValues || [], + deletedLegalValues, + }; +}; + export const ResolveInput = ({ input, contextDefinition, @@ -43,20 +62,14 @@ export const ResolveInput = ({ const resolveInput = () => { switch (input) { case IN_OPERATORS_LEGAL_VALUES: - return ( - - ); case STRING_OPERATORS_LEGAL_VALUES: return ( <> { + it('should show deleted legal values as disabled', async () => { + const value = 'some-value'; + const values = [{ value }]; + const legalValues = [{ value: 'some-other-value' }]; + + render( + + ); + + const input = await screen.findByDisplayValue('some-value'); + + expect(input).toBeInTheDocument(); + expect(input).toHaveProperty('disabled', true); + + expect( + await screen.findByDisplayValue('some-other-value') + ).toBeInTheDocument(); + }); + + it('should remove deleted legal values when editing values', async () => { + const value = 'some-value'; + const deletedLegalValues = [{ value }]; + const legalValues = [ + { value: 'some-other-value' }, + { value: 'value2' }, + ]; + const setValues = vi.fn(); + render( + + ); + const btn = await screen.findByDisplayValue('some-other-value'); + btn.click(); + + expect(setValues).toHaveBeenCalledWith(['value2', 'some-other-value']); + }); +}); diff --git a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.tsx b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.tsx index 930db206da..ae3ae9e258 100644 --- a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.tsx +++ b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.tsx @@ -6,12 +6,15 @@ import { ConstraintValueSearch } from 'component/common/ConstraintAccordion/Cons import { ConstraintFormHeader } from '../ConstraintFormHeader/ConstraintFormHeader'; import { ILegalValue } from 'interfaces/context'; import { - LegalValueLabel, filterLegalValues, + LegalValueLabel, } from '../LegalValueLabel/LegalValueLabel'; interface IRestrictiveLegalValuesProps { - legalValues: ILegalValue[]; + data: { + legalValues: ILegalValue[]; + deletedLegalValues: ILegalValue[]; + }; values: string[]; setValues: (values: string[]) => void; beforeValues?: JSX.Element; @@ -33,13 +36,14 @@ const createValuesMap = (values: string[]): IValuesMap => { }; export const RestrictiveLegalValues = ({ - legalValues, + data, values, setValues, error, setError, }: IRestrictiveLegalValuesProps) => { const [filter, setFilter] = useState(''); + const { legalValues, deletedLegalValues } = data; const filteredValues = filterLegalValues(legalValues, filter); // Lazily initialise the values because there might be a lot of them. @@ -50,8 +54,19 @@ export const RestrictiveLegalValues = ({ setValuesMap(createValuesMap(values)); }, [values, setValuesMap]); + const cleanDeletedLegalValues = (constraintValues: string[]): string[] => { + const deletedValuesSet = new Set( + deletedLegalValues.map(({ value }) => value) + ); + return ( + constraintValues?.filter(value => !deletedValuesSet.has(value)) || + [] + ); + }; + const onChange = (legalValue: string) => { setError(''); + if (valuesMap[legalValue]) { const index = values.findIndex(value => value === legalValue); const newValues = [...values]; @@ -60,7 +75,7 @@ export const RestrictiveLegalValues = ({ return; } - setValues([...values, legalValue]); + setValues([...cleanDeletedLegalValues(values), legalValue]); }; return ( @@ -77,7 +92,7 @@ export const RestrictiveLegalValues = ({ /> } /> - {filteredValues.map(match => ( + {filteredValues.concat(deletedLegalValues).map(match => ( onChange(match.value)} name={match.value} color="primary" + disabled={deletedLegalValues + .map(({ value }) => value) + .includes(match.value)} /> } />