1
0
mirror of https://github.com/Unleash/unleash.git synced 2024-12-22 19:07:54 +01:00

Chore: add strategy sorting algorithm (#5406)

This PR adds a strategy sorting algorithm to be used for the segment
deletion dialog. It assumes that you have a list of existing strategies
and a list of change request strategies. Based on the content of these
two lists, it will create one unified list sorted after a number of
criteria (as listed in the test).

# Discussion point: 

This impl does the sorting on the front end, but could we do it on the
back end? Instead of adding a new property to the segment data, could we
simply fold the change request strategies in with the existing segment
strategies and return it using the old property? If the only place we do
that is in this view, then that might be a good suggestion.

Response:

I'll leave this in the front end for now. The reason is that we can't add change request strategies to the existing `strategies` property of the API payload without it being a breaking change. The OpenAPI schema says that `id` is a required field on a strategy, and that field doesn't exist on strategies that have only been added in change requests, but not yet applied.
This commit is contained in:
Thomas Heartman 2023-11-27 11:23:10 +01:00 committed by GitHub
parent 67b9578545
commit 90915cfdd7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 229 additions and 0 deletions

View File

@ -0,0 +1,137 @@
import { sortStrategiesByFeature } from './sort-strategies';
describe('sorting strategies by feature', () => {
test('strategies with the same id are sorted: existing first, then change requests', () => {
const strategies = [{ id: 'a', featureName: 'feature1' }];
const changeRequestStrategies = [
{ id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
];
const expected = [
{ id: 'a', featureName: 'feature1' },
{ id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
];
expect(
sortStrategiesByFeature(strategies, changeRequestStrategies),
).toStrictEqual(expected);
});
test('the same strategy used in multiple change requests is sorted by change request id', () => {
const changeRequestStrategies = [
{ id: 'a', featureName: 'feature1', changeRequest: { id: 2 } },
{ id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
];
const expected = [
{ id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
{ id: 'a', featureName: 'feature1', changeRequest: { id: 2 } },
];
expect(
sortStrategiesByFeature([], changeRequestStrategies),
).toStrictEqual(expected);
});
test('strategies are sorted by id, with change requests strategies being listed before existing strategies if their ids would indicate that', () => {
const strategies = [{ id: 'b', featureName: 'feature1' }];
const changeRequestStrategies = [
{ id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
];
const expected = [
{ id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
{ id: 'b', featureName: 'feature1' },
];
expect(
sortStrategiesByFeature(strategies, changeRequestStrategies),
).toStrictEqual(expected);
});
test('strategies without ids (new strategies) are sorted by change request id', () => {
const changeRequestStrategies = [
{ featureName: 'feature1', changeRequest: { id: 2 } },
{ featureName: 'feature1', changeRequest: { id: 1 } },
];
const expected = [
{ featureName: 'feature1', changeRequest: { id: 1 } },
{ featureName: 'feature1', changeRequest: { id: 2 } },
];
expect(
sortStrategiesByFeature([], changeRequestStrategies),
).toStrictEqual(expected);
});
test('if new strategies have the same change request id, they should be listed in the same order as in the input', () => {
const changeRequestStrategies = [
{ key: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
{ key: 'b', featureName: 'feature1', changeRequest: { id: 1 } },
];
const expected = [
{ key: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
{ key: 'b', featureName: 'feature1', changeRequest: { id: 1 } },
];
expect(
sortStrategiesByFeature([], changeRequestStrategies),
).toStrictEqual(expected);
});
test('all the various sorts work together', () => {
const strategies = [
{ id: 'a', featureName: 'feature1' },
{ id: 'b', featureName: 'feature1' },
{ id: 'd', featureName: 'feature1' },
];
const changeRequestStrategies = [
{ id: 'a', featureName: 'feature1', changeRequest: { id: 2 } },
{ id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
{ id: 'c', featureName: 'feature1', changeRequest: { id: 1 } },
{ key: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
{ key: 'b', featureName: 'feature1', changeRequest: { id: 1 } },
];
const expected = [
{ id: 'a', featureName: 'feature1' },
{ id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
{ id: 'a', featureName: 'feature1', changeRequest: { id: 2 } },
{ id: 'b', featureName: 'feature1' },
{ id: 'c', featureName: 'feature1', changeRequest: { id: 1 } },
{ id: 'd', featureName: 'feature1' },
{ key: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
{ key: 'b', featureName: 'feature1', changeRequest: { id: 1 } },
];
expect(
sortStrategiesByFeature(strategies, changeRequestStrategies),
).toStrictEqual(expected);
});
test('when multiple flag names are provided, the list will be sorted on flag name first', () => {
const strategies = [
{ id: 'b', featureName: 'feature2' },
{ id: 'a', featureName: 'feature1' },
];
const changeRequestStrategies = [
{ id: 'a', featureName: 'feature1', changeRequest: { id: 2 } },
{ id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
{ id: 'b', featureName: 'feature2', changeRequest: { id: 2 } },
];
const expected = [
{ id: 'a', featureName: 'feature1' },
{ id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
{ id: 'a', featureName: 'feature1', changeRequest: { id: 2 } },
{ id: 'b', featureName: 'feature2' },
{ id: 'b', featureName: 'feature2', changeRequest: { id: 2 } },
];
expect(
sortStrategiesByFeature(strategies, changeRequestStrategies),
).toStrictEqual(expected);
});
});

View File

@ -0,0 +1,92 @@
export const sortStrategiesByFeature = <
ExistingStrategy extends { id: string; featureName?: string },
UpdatedStrategy extends {
id: string;
featureName: string;
changeRequest: { id: number };
},
NewStrategy extends {
featureName: string;
changeRequest: { id: number };
},
>(
strategies: ExistingStrategy[],
changeRequestStrategies: (UpdatedStrategy | NewStrategy)[],
): (ExistingStrategy | UpdatedStrategy | NewStrategy)[] => {
const strategiesByFlag = [...strategies, ...changeRequestStrategies].reduce(
(acc, strategy) => {
if (!strategy.featureName) {
// this shouldn't ever happen here, but because the
// type system allows it to, we need to handle it. If
// a strategy doesn't have a feature name, discard it.
return acc;
}
const registered = acc[strategy.featureName];
if (registered) {
registered.push(strategy);
} else {
acc[strategy.featureName] = [strategy];
}
return acc;
},
{} as {
[flagName: string]: (
| ExistingStrategy
| UpdatedStrategy
| NewStrategy
)[];
},
);
const flagToStrategiesList = Object.entries(strategiesByFlag);
flagToStrategiesList.sort(([flagA], [flagB]) => {
return flagA.localeCompare(flagB);
});
return flagToStrategiesList.flatMap(([_, strategies]) => {
const isExistingStrategy = (
strategy: ExistingStrategy | UpdatedStrategy | NewStrategy,
): strategy is ExistingStrategy | UpdatedStrategy => 'id' in strategy;
const isChangeRequest = (
strategy: ExistingStrategy | UpdatedStrategy | NewStrategy,
): strategy is UpdatedStrategy | NewStrategy =>
'changeRequest' in strategy;
strategies.sort((a, b) => {
if (isExistingStrategy(a) && isExistingStrategy(b)) {
// both strategies exist already
const idComp = a.id.localeCompare(b.id);
if (idComp === 0) {
const crA = isChangeRequest(a);
const crB = isChangeRequest(b);
if (crA && !crB) {
return 1;
} else if (!crA && crB) {
return -1;
} else if (crA && crB) {
return a.changeRequest.id - b.changeRequest.id;
} else {
return 0;
}
} else {
return idComp;
}
} else if (isExistingStrategy(a)) {
// strategy b is new
return -1;
} else if (isExistingStrategy(b)) {
// strategy a is new
return 1;
} else {
// both strategies are new
return a.changeRequest.id - b.changeRequest.id;
}
});
return strategies;
});
};