1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-09-15 17:50:48 +02:00

fix: Order properties correctly when mapping from Editable Constraint to IConstraint (#10163)

Prevents the property order from changing when constraints are set from
the editable constraint component. When we render out the API command,
we don't specify the order of properties in the objects, which means
that it can change dramatically, which can be a little jarring.

As it is right now, it first renders in one order when you open the
strategy form:

<img width="299" alt="image"
src="https://github.com/user-attachments/assets/52cf2445-d9eb-402c-b5bc-0fece5fbe822"
/>

And when you navigate to the targeting section of the strategy form, it
changes to:
<img width="299" alt="image"
src="https://github.com/user-attachments/assets/e4cb7006-dcf4-4e88-befb-ccba5b647ddd"
/>

This also applies to constraints in segments.

With this change, the order will remain the same before and after.

Additionally, there's some extra tests around constraint ids being kept
the same and being set if it doesn't exist.

## Further work

This came about as part of the issue we had with constraint editing in
segments being broken as of now. As part of that, I would like to make
some further improvements (such as making the ID required when you use a
list of editable constraints), but that will be in a follow-up. There
are some complications that might not make it a viable option, sadly.

We could also try to stabilize the property order in the API rendering
methods (which I think might be a good idea), but because there's
multiple different ones, this seems to be a faster solution.
This commit is contained in:
Thomas Heartman 2025-06-18 13:47:37 +02:00 committed by GitHub
parent 2444bd7ccd
commit ef3ffc4d94
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 76 additions and 10 deletions

View File

@ -0,0 +1,44 @@
import { createEmptyConstraint } from 'utils/createEmptyConstraint';
import { fromIConstraint, toIConstraint } from './editable-constraint-type.ts';
import { constraintId } from 'constants/constraintId';
test('mapping to and from retains the constraint id', () => {
const constraint = createEmptyConstraint('context');
expect(toIConstraint(fromIConstraint(constraint))[constraintId]).toEqual(
constraint[constraintId],
);
});
test('mapping to an editable constraint adds a constraint id if there is none', () => {
const constraint = createEmptyConstraint('context');
delete constraint[constraintId];
const editableConstraint = fromIConstraint(constraint);
expect(editableConstraint[constraintId]).toBeDefined();
const iConstraint = toIConstraint(editableConstraint);
expect(iConstraint[constraintId]).toEqual(editableConstraint[constraintId]);
});
test('mapping from an empty constraint removes redundant value / values', () => {
const constraint = createEmptyConstraint('context');
expect(constraint).toHaveProperty('value');
const transformed = toIConstraint(fromIConstraint(constraint));
expect(transformed).not.toHaveProperty('value');
});
test('mapping to constraint returns properties in expected order', () => {
const constraint = createEmptyConstraint('context');
const transformed = toIConstraint(fromIConstraint(constraint));
expect(Object.keys(transformed)).toEqual([
'values',
'inverted',
'operator',
'contextName',
'caseInsensitive',
]);
});

View File

@ -1,3 +1,4 @@
import { constraintId } from 'constants/constraintId';
import {
type DateOperator,
isDateOperator,
@ -10,6 +11,7 @@ import {
isSemVerOperator,
} from 'constants/operators';
import type { IConstraint } from 'interfaces/strategy';
import { v4 as uuidv4 } from 'uuid';
type EditableConstraintBase = Omit<
IConstraint,
@ -72,12 +74,14 @@ export const fromIConstraint = (
const { value, values, operator, ...rest } = constraint;
if (isSingleValueOperator(operator)) {
return {
[constraintId]: uuidv4(),
...rest,
operator,
value: value ?? '',
};
} else {
return {
[constraintId]: uuidv4(),
...rest,
operator,
values: new Set(values),
@ -86,13 +90,29 @@ export const fromIConstraint = (
};
export const toIConstraint = (constraint: EditableConstraint): IConstraint => {
const {
inverted,
operator,
contextName,
caseInsensitive,
[constraintId]: id,
} = constraint;
const baseValues = {
inverted,
operator,
contextName,
caseInsensitive,
[constraintId]: id,
};
if ('value' in constraint) {
return constraint;
} else {
const { values, ...rest } = constraint;
return {
...rest,
values: Array.from(values),
value: constraint.value,
...baseValues,
};
} else {
return {
values: Array.from(constraint.values),
...baseValues,
};
}
};

View File

@ -40,11 +40,13 @@ test('calls onUpdate with new state', async () => {
// gets called by useEffect, so we need to wait for the next render.
await waitFor(() => {
expect(onUpdate).toHaveBeenCalledWith({
expect(onUpdate).toHaveBeenCalledWith(
expect.objectContaining({
contextName: 'context-field',
operator: IN,
values: [],
});
}),
);
});
});