mirror of
https://github.com/Unleash/unleash.git
synced 2025-01-25 00:07:47 +01:00
fix: sort segments before comparing in cr diff calculations (#7202)
This change fixes a bug where we would show the list of segments as changed (causing a conflict) if their order wasn't the same in the change as in the original. By sorting the segments before comparing them, we can avoid this issue. To avoid modifying the objects that are passed in (in case that has knock-on effects anywhere), we copy the objects. And because `toSorted` "only" has about 89% coverage now, I chose to use `sort` and spread the arrays instead.
This commit is contained in:
parent
e79e36daae
commit
0e7f1aab50
@ -99,6 +99,24 @@ describe('Strategy change conflict detection', () => {
|
|||||||
expect(resultMissing).toBeNull();
|
expect(resultMissing).toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('It sorts segments before comparing (because their order is irrelevant)', () => {
|
||||||
|
const result = getStrategyChangesThatWouldBeOverwritten(
|
||||||
|
{
|
||||||
|
...existingStrategy,
|
||||||
|
segments: [25, 26, 1],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
...change,
|
||||||
|
payload: {
|
||||||
|
...change.payload,
|
||||||
|
segments: [26, 1, 25],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
test('It treats `undefined` or missing strategy variants in old config and change as equal to `[]`', () => {
|
test('It treats `undefined` or missing strategy variants in old config and change as equal to `[]`', () => {
|
||||||
const undefinedVariantsExistingStrategy = {
|
const undefinedVariantsExistingStrategy = {
|
||||||
...existingStrategy,
|
...existingStrategy,
|
||||||
|
@ -142,9 +142,31 @@ export function getStrategyChangesThatWouldBeOverwritten(
|
|||||||
change: IChangeRequestUpdateStrategy,
|
change: IChangeRequestUpdateStrategy,
|
||||||
): ChangesThatWouldBeOverwritten | null {
|
): ChangesThatWouldBeOverwritten | null {
|
||||||
const fallbacks = { segments: [], variants: [], title: '' };
|
const fallbacks = { segments: [], variants: [], title: '' };
|
||||||
|
|
||||||
|
const withSortedSegments = (() => {
|
||||||
|
if (!(change.payload.segments && currentStrategyConfig?.segments)) {
|
||||||
|
return { current: currentStrategyConfig, change: change };
|
||||||
|
}
|
||||||
|
|
||||||
|
const changeCopy = {
|
||||||
|
...change,
|
||||||
|
payload: {
|
||||||
|
...change.payload,
|
||||||
|
segments: [...change.payload.segments].sort(),
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
const currentCopy = {
|
||||||
|
...currentStrategyConfig,
|
||||||
|
segments: [...currentStrategyConfig.segments].sort(),
|
||||||
|
};
|
||||||
|
|
||||||
|
return { current: currentCopy, change: changeCopy };
|
||||||
|
})();
|
||||||
|
|
||||||
return getChangesThatWouldBeOverwritten(
|
return getChangesThatWouldBeOverwritten(
|
||||||
omit(currentStrategyConfig, 'strategyName'),
|
omit(withSortedSegments.current, 'strategyName'),
|
||||||
change,
|
withSortedSegments.change,
|
||||||
fallbacks,
|
fallbacks,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user