feat: modify tests and fix impl

frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/sort-strategies.test.tsx

@@ -10,12 +10,10 @@ describe('sorting strategies by feature', () => {
             { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
         ];
 
-        const expected = {
+        const expected = [
-            feature1: [
+            { id: 'a', featureName: 'feature1' },
-                { id: 'a', featureName: 'feature1' },
+            { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
-                { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
+        ];
-            ],
-        };
 
         expect(
             sortStrategiesByFeature(strategies, changeRequestStrategies),
@@ -28,12 +26,10 @@ describe('sorting strategies by feature', () => {
             { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
         ];
 
-        const expected = {
+        const expected = [
-            feature1: [
+            { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
-                { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
+            { id: 'a', featureName: 'feature1', changeRequest: { id: 2 } },
-                { id: 'a', featureName: 'feature1', changeRequest: { id: 2 } },
+        ];
-            ],
-        };
 
         expect(
             sortStrategiesByFeature([], changeRequestStrategies),
@@ -46,12 +42,10 @@ describe('sorting strategies by feature', () => {
             { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
         ];
 
-        const expected = {
+        const expected = [
-            feature1: [
+            { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
-                { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
+            { id: 'b', featureName: 'feature1' },
-                { id: 'b', featureName: 'feature1' },
+        ];
-            ],
-        };
 
         expect(
             sortStrategiesByFeature(strategies, changeRequestStrategies),
@@ -64,12 +58,10 @@ describe('sorting strategies by feature', () => {
             { featureName: 'feature1', changeRequest: { id: 1 } },
         ];
 
-        const expected = {
+        const expected = [
-            feature1: [
+            { featureName: 'feature1', changeRequest: { id: 1 } },
-                { featureName: 'feature1', changeRequest: { id: 1 } },
+            { featureName: 'feature1', changeRequest: { id: 2 } },
-                { featureName: 'feature1', changeRequest: { id: 2 } },
+        ];
-            ],
-        };
 
         expect(
             sortStrategiesByFeature([], changeRequestStrategies),
@@ -82,12 +74,10 @@ describe('sorting strategies by feature', () => {
             { key: 'b', featureName: 'feature1', changeRequest: { id: 1 } },
         ];
 
-        const expected = {
+        const expected = [
-            feature1: [
+            { key: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
-                { key: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
+            { key: 'b', featureName: 'feature1', changeRequest: { id: 1 } },
-                { key: 'b', featureName: 'feature1', changeRequest: { id: 1 } },
+        ];
-            ],
-        };
 
         expect(
             sortStrategiesByFeature([], changeRequestStrategies),
@@ -108,45 +98,43 @@ describe('sorting strategies by feature', () => {
             { key: 'b', featureName: 'feature1', changeRequest: { id: 1 } },
         ];
 
-        const expected = {
+        const expected = [
-            feature1: [
+            { id: 'a', featureName: 'feature1' },
-                { id: 'a', featureName: 'feature1' },
+            { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
-                { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
+            { id: 'a', featureName: 'feature1', changeRequest: { id: 2 } },
-                { id: 'a', featureName: 'feature1', changeRequest: { id: 2 } },
+            { id: 'b', featureName: 'feature1' },
-                { id: 'b', featureName: 'feature1' },
+            { id: 'c', featureName: 'feature1', changeRequest: { id: 1 } },
-                { id: 'c', featureName: 'feature1', changeRequest: { id: 1 } },
+            { id: 'd', featureName: 'feature1' },
-                { id: 'd', featureName: 'feature1' },
+            { key: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
-                { key: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
+            { key: 'b', 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);
     });
 });
-
-test('when multiple flag names are provided, the list will be sorted on flag name first, then the criteria used in `sortStrategiesByFeature`', () => {
-    const strategies = [
-        { id: 'b', featureName: 'feature2' },
-        { id: 'a', featureName: 'feature1' },
-    ];
-    const changeRequestStrategies = [
-        { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } },
-        { id: 'a', featureName: 'feature1', changeRequest: { id: 2 } },
-        { 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(
-        sortAndFlattenStrategies(strategies, changeRequestStrategies),
-    ).toStrictEqual(expected);
-});

frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/sort-strategies.ts

@@ -1,21 +1,25 @@
 // use generics here to make it easier to test
 export const sortStrategiesByFeature = <
-    T extends { id: string; featureName?: string },
+    ExistingStrategy extends { id: string; featureName?: string },
-    U extends {
+    UpdatedStrategy extends {
-        id?: string;
+        id: string;
+        featureName: string;
+        changeRequest: { id: number };
+    },
+    NewStrategy extends {
         featureName: string;
         changeRequest: { id: number };
     },
 >(
-    strategies: T[],
+    strategies: ExistingStrategy[],
-    changeRequestStrategies: U[],
+    changeRequestStrategies: (UpdatedStrategy | NewStrategy)[],
-): { [flagName: string]: (T | U)[] } => {
+): (ExistingStrategy | UpdatedStrategy | NewStrategy)[] => {
-    const isExistingStrategy = (strategy: T | U) => 'id' in strategy;
-    const isNewStrategy = (strategy: T | U) => !isExistingStrategy(strategy);
-
     const collected = [...strategies, ...changeRequestStrategies].reduce(
         (acc, strategy) => {
             if (!strategy.featureName) {
+                // this shouldn't ever happen here, but if it does,
+                // we'll remove it because we don't know what to do
+                // with it.
                 return acc;
             }
             const registered = acc[strategy.featureName];
@@ -27,30 +31,67 @@ export const sortStrategiesByFeature = <
 
             return acc;
         },
-        {} as { [flagName: string]: (T | U)[] },
+        {} as {
-    );
+            [flagName: string]: (
-
+                | ExistingStrategy
-    const sorted = Object.entries(collected).map(
+                | UpdatedStrategy
-        ([featureName, strategies]) => {
+                | NewStrategy
-            strategies.sort((a, b) => {
+            )[];
-                if (isNewStrategy(a) && isNewStrategy(b)) {
-                    return a.changeRequest.id - b.changeRequest.id;
-                }
-                if (isNewStrategy(a)) {
-                    return -1;
-                }
-                if (isNewStrategy(b)) {
-                    return 1;
-                }
-                return 0;
-            });
-            [featureName, strategies];
         },
     );
 
-    const collected2 = Object.fromEntries(sorted);
+    const flags: [
+        string,
+        (ExistingStrategy | UpdatedStrategy | NewStrategy)[],
+    ][] = Object.entries(collected);
 
-    return collected2;
+    flags.sort(([flagA], [flagB]) => {
+        return flagA.localeCompare(flagB);
+    });
+
+    return flags.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;
+    });
 };
 
 export const sortAndFlattenStrategies = <