mirror of
https://github.com/Unleash/unleash.git
synced 2025-07-26 13:48:33 +02:00
Fix(1-3485)/handle deleted constraints (#9999)
Improves handling of constraints in use that have been deleted. This change implments a few small changes on both the front and the back end on how we deal with constraints that have been deleted. The most important change is on the back end, in the `/constraints/validate` endpoint. We used to throw here if the constraint couldn't be found, but the only reason we wanted to look for the constraint in the db was to check for legal values. Now, instead, we'll allow you to pass a constraint field that doesn't exist in the database. We'll still check the values against the operator for validity, we just don't control legal values anymore (because there aren't any). On the front end, we improve the handling by showing the deleted context filed in the dropdown, both when the selector dropdown is closed and when it is open. However, if you change the context field, we remove the deleted field from the list. This seems like a sensible tradeoff. Means you can't select it if you've deselected it.
This commit is contained in:
parent
4d92d54f9a
commit
082a03afd7
@ -258,9 +258,19 @@ export const EditableConstraint: FC<Props> = ({
|
||||
return null;
|
||||
}
|
||||
|
||||
const constraintNameOptions = context.map((context) => {
|
||||
return { key: context.name, label: context.name };
|
||||
});
|
||||
const extantContextFieldNames = context.map((context) => context.name);
|
||||
const contextFieldHasBeenDeleted = !extantContextFieldNames.includes(
|
||||
localConstraint.contextName,
|
||||
);
|
||||
|
||||
const availableContextFieldNames = contextFieldHasBeenDeleted
|
||||
? [...extantContextFieldNames, localConstraint.contextName].toSorted()
|
||||
: extantContextFieldNames;
|
||||
|
||||
const contextFieldOptions = availableContextFieldNames.map((option) => ({
|
||||
key: option,
|
||||
label: option,
|
||||
}));
|
||||
|
||||
return (
|
||||
<Container>
|
||||
@ -272,7 +282,7 @@ export const EditableConstraint: FC<Props> = ({
|
||||
id='context-field-select'
|
||||
name='contextName'
|
||||
label='Context Field'
|
||||
options={constraintNameOptions}
|
||||
options={contextFieldOptions}
|
||||
value={contextName || ''}
|
||||
onChange={(contextField) =>
|
||||
updateConstraint({
|
||||
|
@ -245,7 +245,7 @@ export class ContextController extends Controller {
|
||||
tags: ['Context'],
|
||||
summary: 'Validate a context field',
|
||||
description:
|
||||
'Check whether the provided data can be used to create a context field. If the data is not valid, ...?',
|
||||
'Check whether the provided data can be used to create a context field. If the data is not valid, returns a 400 status code with the reason why it is not valid.',
|
||||
operationId: 'validate',
|
||||
requestBody: createRequestSchema('nameSchema'),
|
||||
responses: {
|
||||
|
@ -497,10 +497,6 @@ export class FeatureToggleService {
|
||||
async validateConstraint(input: IConstraint): Promise<IConstraint> {
|
||||
const constraint = await constraintSchema.validateAsync(input);
|
||||
const { operator } = constraint;
|
||||
const contextDefinition = await this.contextFieldStore.get(
|
||||
constraint.contextName,
|
||||
);
|
||||
|
||||
if (oneOf(NUM_OPERATORS, operator)) {
|
||||
await validateNumber(constraint.value);
|
||||
}
|
||||
@ -519,20 +515,26 @@ export class FeatureToggleService {
|
||||
await validateDate(constraint.value);
|
||||
}
|
||||
|
||||
if (
|
||||
contextDefinition?.legalValues &&
|
||||
contextDefinition.legalValues.length > 0
|
||||
) {
|
||||
const valuesToValidate = oneOf(
|
||||
[...DATE_OPERATORS, ...SEMVER_OPERATORS, ...NUM_OPERATORS],
|
||||
operator,
|
||||
)
|
||||
? constraint.value
|
||||
: constraint.values;
|
||||
validateLegalValues(
|
||||
contextDefinition.legalValues,
|
||||
valuesToValidate,
|
||||
if (await this.contextFieldStore.exists(constraint.contextName)) {
|
||||
const contextDefinition = await this.contextFieldStore.get(
|
||||
constraint.contextName,
|
||||
);
|
||||
|
||||
if (
|
||||
contextDefinition?.legalValues &&
|
||||
contextDefinition.legalValues.length > 0
|
||||
) {
|
||||
const valuesToValidate = oneOf(
|
||||
[...DATE_OPERATORS, ...SEMVER_OPERATORS, ...NUM_OPERATORS],
|
||||
operator,
|
||||
)
|
||||
? constraint.value
|
||||
: constraint.values;
|
||||
validateLegalValues(
|
||||
contextDefinition.legalValues,
|
||||
valuesToValidate,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return constraint;
|
||||
|
@ -43,3 +43,25 @@ test('should accept valid constraints', async () => {
|
||||
.send({ contextName: 'environment', operator: 'IN', values: ['a'] })
|
||||
.expect(204);
|
||||
});
|
||||
|
||||
test('should allow unknown constraints if their values are valid', async () => {
|
||||
await app.request
|
||||
.post(PATH)
|
||||
.send({
|
||||
contextName: 'not-a-default-context-value',
|
||||
operator: 'NUM_EQ',
|
||||
value: 1,
|
||||
})
|
||||
.expect(204);
|
||||
});
|
||||
|
||||
test('should block unknown constraints if their values are invalid', async () => {
|
||||
await app.request
|
||||
.post(PATH)
|
||||
.send({
|
||||
contextName: 'not-a-default-context-value',
|
||||
operator: 'IN',
|
||||
value: 1,
|
||||
})
|
||||
.expect(400);
|
||||
});
|
||||
|
Loading…
Reference in New Issue
Block a user