mirror of
https://github.com/Unleash/unleash.git
synced 2025-04-06 01:15:28 +02:00
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 <andreas@getunleash.ai>
This commit is contained in:
parent
c1fe43fe64
commit
f9a686ca81
@ -1,4 +1,4 @@
|
|||||||
import { IUnleashContextDefinition } from 'interfaces/context';
|
import { ILegalValue, IUnleashContextDefinition } from 'interfaces/context';
|
||||||
import { IConstraint } from 'interfaces/strategy';
|
import { IConstraint } from 'interfaces/strategy';
|
||||||
import { DateSingleValue } from '../DateSingleValue/DateSingleValue';
|
import { DateSingleValue } from '../DateSingleValue/DateSingleValue';
|
||||||
import { FreeTextInput } from '../FreeTextInput/FreeTextInput';
|
import { FreeTextInput } from '../FreeTextInput/FreeTextInput';
|
||||||
@ -30,6 +30,25 @@ interface IResolveInputProps {
|
|||||||
error: string;
|
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 = ({
|
export const ResolveInput = ({
|
||||||
input,
|
input,
|
||||||
contextDefinition,
|
contextDefinition,
|
||||||
@ -43,20 +62,14 @@ export const ResolveInput = ({
|
|||||||
const resolveInput = () => {
|
const resolveInput = () => {
|
||||||
switch (input) {
|
switch (input) {
|
||||||
case IN_OPERATORS_LEGAL_VALUES:
|
case IN_OPERATORS_LEGAL_VALUES:
|
||||||
return (
|
|
||||||
<RestrictiveLegalValues
|
|
||||||
legalValues={contextDefinition.legalValues || []}
|
|
||||||
values={localConstraint.values || []}
|
|
||||||
setValues={setValues}
|
|
||||||
error={error}
|
|
||||||
setError={setError}
|
|
||||||
/>
|
|
||||||
);
|
|
||||||
case STRING_OPERATORS_LEGAL_VALUES:
|
case STRING_OPERATORS_LEGAL_VALUES:
|
||||||
return (
|
return (
|
||||||
<>
|
<>
|
||||||
<RestrictiveLegalValues
|
<RestrictiveLegalValues
|
||||||
legalValues={contextDefinition.legalValues || []}
|
data={resolveLegalValues(
|
||||||
|
localConstraint.values,
|
||||||
|
contextDefinition.legalValues
|
||||||
|
)}
|
||||||
values={localConstraint.values || []}
|
values={localConstraint.values || []}
|
||||||
setValues={setValues}
|
setValues={setValues}
|
||||||
error={error}
|
error={error}
|
||||||
|
@ -0,0 +1,54 @@
|
|||||||
|
import { render } from 'utils/testRenderer';
|
||||||
|
import { RestrictiveLegalValues } from './RestrictiveLegalValues';
|
||||||
|
import { screen } from '@testing-library/react';
|
||||||
|
import { vi } from 'vitest';
|
||||||
|
|
||||||
|
describe('RestictiveLegalValues', () => {
|
||||||
|
it('should show deleted legal values as disabled', async () => {
|
||||||
|
const value = 'some-value';
|
||||||
|
const values = [{ value }];
|
||||||
|
const legalValues = [{ value: 'some-other-value' }];
|
||||||
|
|
||||||
|
render(
|
||||||
|
<RestrictiveLegalValues
|
||||||
|
data={{ legalValues, deletedLegalValues: values }}
|
||||||
|
values={[value]}
|
||||||
|
setValues={vi.fn()}
|
||||||
|
error={''}
|
||||||
|
setError={vi.fn()}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
|
||||||
|
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(
|
||||||
|
<RestrictiveLegalValues
|
||||||
|
data={{ legalValues, deletedLegalValues }}
|
||||||
|
values={[value, 'value2']}
|
||||||
|
setValues={setValues}
|
||||||
|
error={''}
|
||||||
|
setError={vi.fn()}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
const btn = await screen.findByDisplayValue('some-other-value');
|
||||||
|
btn.click();
|
||||||
|
|
||||||
|
expect(setValues).toHaveBeenCalledWith(['value2', 'some-other-value']);
|
||||||
|
});
|
||||||
|
});
|
@ -6,12 +6,15 @@ import { ConstraintValueSearch } from 'component/common/ConstraintAccordion/Cons
|
|||||||
import { ConstraintFormHeader } from '../ConstraintFormHeader/ConstraintFormHeader';
|
import { ConstraintFormHeader } from '../ConstraintFormHeader/ConstraintFormHeader';
|
||||||
import { ILegalValue } from 'interfaces/context';
|
import { ILegalValue } from 'interfaces/context';
|
||||||
import {
|
import {
|
||||||
LegalValueLabel,
|
|
||||||
filterLegalValues,
|
filterLegalValues,
|
||||||
|
LegalValueLabel,
|
||||||
} from '../LegalValueLabel/LegalValueLabel';
|
} from '../LegalValueLabel/LegalValueLabel';
|
||||||
|
|
||||||
interface IRestrictiveLegalValuesProps {
|
interface IRestrictiveLegalValuesProps {
|
||||||
legalValues: ILegalValue[];
|
data: {
|
||||||
|
legalValues: ILegalValue[];
|
||||||
|
deletedLegalValues: ILegalValue[];
|
||||||
|
};
|
||||||
values: string[];
|
values: string[];
|
||||||
setValues: (values: string[]) => void;
|
setValues: (values: string[]) => void;
|
||||||
beforeValues?: JSX.Element;
|
beforeValues?: JSX.Element;
|
||||||
@ -33,13 +36,14 @@ const createValuesMap = (values: string[]): IValuesMap => {
|
|||||||
};
|
};
|
||||||
|
|
||||||
export const RestrictiveLegalValues = ({
|
export const RestrictiveLegalValues = ({
|
||||||
legalValues,
|
data,
|
||||||
values,
|
values,
|
||||||
setValues,
|
setValues,
|
||||||
error,
|
error,
|
||||||
setError,
|
setError,
|
||||||
}: IRestrictiveLegalValuesProps) => {
|
}: IRestrictiveLegalValuesProps) => {
|
||||||
const [filter, setFilter] = useState('');
|
const [filter, setFilter] = useState('');
|
||||||
|
const { legalValues, deletedLegalValues } = data;
|
||||||
const filteredValues = filterLegalValues(legalValues, filter);
|
const filteredValues = filterLegalValues(legalValues, filter);
|
||||||
|
|
||||||
// Lazily initialise the values because there might be a lot of them.
|
// Lazily initialise the values because there might be a lot of them.
|
||||||
@ -50,8 +54,19 @@ export const RestrictiveLegalValues = ({
|
|||||||
setValuesMap(createValuesMap(values));
|
setValuesMap(createValuesMap(values));
|
||||||
}, [values, setValuesMap]);
|
}, [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) => {
|
const onChange = (legalValue: string) => {
|
||||||
setError('');
|
setError('');
|
||||||
|
|
||||||
if (valuesMap[legalValue]) {
|
if (valuesMap[legalValue]) {
|
||||||
const index = values.findIndex(value => value === legalValue);
|
const index = values.findIndex(value => value === legalValue);
|
||||||
const newValues = [...values];
|
const newValues = [...values];
|
||||||
@ -60,7 +75,7 @@ export const RestrictiveLegalValues = ({
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
setValues([...values, legalValue]);
|
setValues([...cleanDeletedLegalValues(values), legalValue]);
|
||||||
};
|
};
|
||||||
|
|
||||||
return (
|
return (
|
||||||
@ -77,7 +92,7 @@ export const RestrictiveLegalValues = ({
|
|||||||
/>
|
/>
|
||||||
}
|
}
|
||||||
/>
|
/>
|
||||||
{filteredValues.map(match => (
|
{filteredValues.concat(deletedLegalValues).map(match => (
|
||||||
<LegalValueLabel
|
<LegalValueLabel
|
||||||
key={match.value}
|
key={match.value}
|
||||||
legal={match}
|
legal={match}
|
||||||
@ -87,6 +102,9 @@ export const RestrictiveLegalValues = ({
|
|||||||
onChange={() => onChange(match.value)}
|
onChange={() => onChange(match.value)}
|
||||||
name={match.value}
|
name={match.value}
|
||||||
color="primary"
|
color="primary"
|
||||||
|
disabled={deletedLegalValues
|
||||||
|
.map(({ value }) => value)
|
||||||
|
.includes(match.value)}
|
||||||
/>
|
/>
|
||||||
}
|
}
|
||||||
/>
|
/>
|
||||||
|
Loading…
Reference in New Issue
Block a user