diff --git a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEdit.tsx b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEdit.tsx index 67aac897a9..57c50e2ff9 100644 --- a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEdit.tsx +++ b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEdit.tsx @@ -252,6 +252,8 @@ export const ConstraintAccordionEdit = ({ setValue={setValue} setError={setError} localConstraint={localConstraint} + constraintValues={constraint?.values || []} + constraintValue={constraint?.value || ''} input={input} error={error} contextDefinition={contextDefinition} 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 0ebcd28930..c0d0d45cf8 100644 --- a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ResolveInput/ResolveInput.tsx +++ b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ResolveInput/ResolveInput.tsx @@ -22,6 +22,8 @@ import React from 'react'; interface IResolveInputProps { contextDefinition: IUnleashContextDefinition; localConstraint: IConstraint; + constraintValues: string[]; + constraintValue: string; setValue: (value: string) => void; setValues: (values: string[]) => void; setError: React.Dispatch>; @@ -34,6 +36,13 @@ const resolveLegalValues = ( values: IConstraint['values'], legalValues: IUnleashContextDefinition['legalValues'] ): { legalValues: ILegalValue[]; deletedLegalValues: ILegalValue[] } => { + if (legalValues?.length === 0) { + return { + legalValues: [], + deletedLegalValues: [], + }; + } + const deletedLegalValues = (values || []) .filter( value => @@ -52,6 +61,8 @@ const resolveLegalValues = ( export const ResolveInput = ({ input, contextDefinition, + constraintValues, + constraintValue, localConstraint, setValue, setValues, @@ -67,9 +78,10 @@ export const ResolveInput = ({ <> { - it('should show deleted legal values as disabled', async () => { - const value = 'some-value'; - const values = [{ value }]; - const legalValues = [{ value: 'some-other-value' }]; +test('should show alert when you have illegal legal values', async () => { + const contextDefinitionValues = [{ value: 'value1' }, { value: 'value2' }]; + const fixedValues = ['value1', 'value2']; + const localValues = ['value1', 'value2']; + const deletedLegalValues = [{ value: 'value1' }]; - render( - - ); + render( + {}} + error={''} + setError={() => {}} + /> + ); - 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']); - }); + await screen.findByText( + 'This constraint is using legal values that have been deleted as valid options. If you save changes on this constraint and then save the strategy the following values will be removed:' + ); +}); + +test('Should remove illegal legal values from internal value state when mounting', () => { + const contextDefinitionValues = [{ value: 'value1' }, { value: 'value2' }]; + const fixedValues = ['value1', 'value2']; + let localValues = ['value1', 'value2']; + const deletedLegalValues = [{ value: 'value1' }]; + + const setValues = (values: string[]) => { + localValues = values; + }; + + render( + {}} + /> + ); + + expect(localValues).toEqual(['value2']); }); 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 ae3ae9e258..572a5af2de 100644 --- a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.tsx +++ b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.tsx @@ -1,6 +1,6 @@ import { useEffect, useState } from 'react'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; -import { Checkbox } from '@mui/material'; +import { Alert, Checkbox } from '@mui/material'; import { useThemeStyles } from 'themes/themeStyles'; import { ConstraintValueSearch } from 'component/common/ConstraintAccordion/ConstraintValueSearch/ConstraintValueSearch'; import { ConstraintFormHeader } from '../ConstraintFormHeader/ConstraintFormHeader'; @@ -15,6 +15,7 @@ interface IRestrictiveLegalValuesProps { legalValues: ILegalValue[]; deletedLegalValues: ILegalValue[]; }; + constraintValues: string[]; values: string[]; setValues: (values: string[]) => void; beforeValues?: JSX.Element; @@ -35,35 +36,59 @@ const createValuesMap = (values: string[]): IValuesMap => { }, {}); }; +export const getLegalValueSet = (values: ILegalValue[]) => { + return new Set(values.map(({ value }) => value)); +}; + +export const getIllegalValues = ( + constraintValues: string[], + deletedLegalValues: ILegalValue[] +) => { + const deletedValuesSet = getLegalValueSet(deletedLegalValues); + + return constraintValues.filter(value => deletedValuesSet.has(value)); +}; + export const RestrictiveLegalValues = ({ data, values, setValues, error, setError, + constraintValues, }: 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. const [valuesMap, setValuesMap] = useState(() => createValuesMap(values)); const { classes: styles } = useThemeStyles(); - useEffect(() => { - setValuesMap(createValuesMap(values)); - }, [values, setValuesMap]); - const cleanDeletedLegalValues = (constraintValues: string[]): string[] => { - const deletedValuesSet = new Set( - deletedLegalValues.map(({ value }) => value) - ); + const deletedValuesSet = getLegalValueSet(deletedLegalValues); return ( constraintValues?.filter(value => !deletedValuesSet.has(value)) || [] ); }; + const illegalValues = getIllegalValues( + constraintValues, + deletedLegalValues + ); + + useEffect(() => { + setValuesMap(createValuesMap(values)); + }, [values, setValuesMap, createValuesMap]); + + useEffect(() => { + if (illegalValues.length > 0) { + setValues(cleanDeletedLegalValues(values)); + } + }, []); + const onChange = (legalValue: string) => { setError(''); @@ -80,6 +105,23 @@ export const RestrictiveLegalValues = ({ return ( <> + 0)} + show={ + + This constraint is using legal values that have been + deleted as valid options. If you save changes on this + constraint and then save the strategy the following + values will be removed: +
    + {illegalValues?.map(value => ( +
  • {value}
    • + ))} +
+ + } + /> + Select values from a predefined set @@ -92,7 +134,7 @@ export const RestrictiveLegalValues = ({ /> } /> - {filteredValues.concat(deletedLegalValues).map(match => ( + {filteredValues.map(match => ( ))} + {error}

} diff --git a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/SingleLegalValue/SingleLegalValue.test.tsx b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/SingleLegalValue/SingleLegalValue.test.tsx new file mode 100644 index 0000000000..503a110476 --- /dev/null +++ b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/SingleLegalValue/SingleLegalValue.test.tsx @@ -0,0 +1,27 @@ +import { render } from 'utils/testRenderer'; +import { screen } from '@testing-library/react'; +import { SingleLegalValue } from './SingleLegalValue'; + +test('should show alert when you have illegal legal values', async () => { + const contextDefinitionValues = [{ value: 'value1' }, { value: 'value2' }]; + const fixedValue = 'value1'; + const localValue = 'value1'; + const deletedLegalValues = [{ value: 'value1' }]; + + render( + {}} + type="number" + legalValues={contextDefinitionValues} + error={''} + setError={() => {}} + /> + ); + + await screen.findByText( + 'This constraint is using legal values that have been deleted as a valid option. Please select a new value from the remaining predefined legal values. The constraint will be updated with the new value when you save the strategy.' + ); +}); diff --git a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/SingleLegalValue/SingleLegalValue.tsx b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/SingleLegalValue/SingleLegalValue.tsx index b83db6fa1e..19d8a45e2b 100644 --- a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/SingleLegalValue/SingleLegalValue.tsx +++ b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/SingleLegalValue/SingleLegalValue.tsx @@ -1,6 +1,6 @@ import React, { useState } from 'react'; import { ConstraintFormHeader } from '../ConstraintFormHeader/ConstraintFormHeader'; -import { FormControl, RadioGroup, Radio } from '@mui/material'; +import { FormControl, RadioGroup, Radio, Alert } from '@mui/material'; import { ConstraintValueSearch } from 'component/common/ConstraintAccordion/ConstraintValueSearch/ConstraintValueSearch'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import { useThemeStyles } from 'themes/themeStyles'; @@ -9,6 +9,7 @@ import { LegalValueLabel, filterLegalValues, } from '../LegalValueLabel/LegalValueLabel'; +import { getIllegalValues } from '../RestrictiveLegalValues/RestrictiveLegalValues'; interface ISingleLegalValueProps { setValue: (value: string) => void; @@ -17,6 +18,11 @@ interface ISingleLegalValueProps { legalValues: ILegalValue[]; error: string; setError: React.Dispatch>; + data: { + legalValues: ILegalValue[]; + deletedLegalValues: ILegalValue[]; + }; + constraintValue: string; } export const SingleLegalValue = ({ @@ -26,13 +32,38 @@ export const SingleLegalValue = ({ legalValues, error, setError, + data, + constraintValue, }: ISingleLegalValueProps) => { const [filter, setFilter] = useState(''); const { classes: styles } = useThemeStyles(); const filteredValues = filterLegalValues(legalValues, filter); + const { deletedLegalValues } = data; + + const illegalValues = getIllegalValues( + [constraintValue], + deletedLegalValues + ); + return ( <> + 0)} + show={ + ({ marginTop: theme.spacing(1) })} + > + {' '} + This constraint is using legal values that have been + deleted as a valid option. Please select a new value + from the remaining predefined legal values. The + constraint will be updated with the new value when you + save the strategy. + + } + /> Add a single {type.toLowerCase()} value diff --git a/frontend/src/component/segments/SegmentTable/SegmentTable.test.tsx b/frontend/src/component/segments/SegmentTable/SegmentTable.test.tsx index fdd3ba0be8..09d3dff4f5 100644 --- a/frontend/src/component/segments/SegmentTable/SegmentTable.test.tsx +++ b/frontend/src/component/segments/SegmentTable/SegmentTable.test.tsx @@ -1,6 +1,5 @@ import { render } from '../../../utils/testRenderer'; import { screen } from '@testing-library/react'; -import React from 'react'; import { SegmentTable } from './SegmentTable'; import { testServerRoute, testServerSetup } from '../../../utils/testServer'; import { UIProviderContainer } from '../../providers/UIProvider/UIProviderContainer';