From 46d8ead7aa717e89a7fe119845819d4356fcc5ff Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Thu, 23 Nov 2023 14:09:32 +0100 Subject: [PATCH] feat: modify tests and fix impl --- .../sort-strategies.test.tsx | 120 ++++++++---------- .../sort-strategies.ts | 99 ++++++++++----- 2 files changed, 124 insertions(+), 95 deletions(-) diff --git a/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/sort-strategies.test.tsx b/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/sort-strategies.test.tsx index 6e8435e677..3acb8b8821 100644 --- a/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/sort-strategies.test.tsx +++ b/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 = { - feature1: [ - { id: 'a', featureName: 'feature1' }, - { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } }, - ], - }; + const expected = [ + { id: 'a', featureName: 'feature1' }, + { 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 = { - feature1: [ - { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } }, - { id: 'a', featureName: 'feature1', changeRequest: { id: 2 } }, - ], - }; + const expected = [ + { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } }, + { 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 = { - feature1: [ - { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } }, - { id: 'b', featureName: 'feature1' }, - ], - }; + const expected = [ + { id: 'a', featureName: 'feature1', changeRequest: { id: 1 } }, + { id: 'b', featureName: 'feature1' }, + ]; expect( sortStrategiesByFeature(strategies, changeRequestStrategies), @@ -64,12 +58,10 @@ describe('sorting strategies by feature', () => { { featureName: 'feature1', changeRequest: { id: 1 } }, ]; - const expected = { - feature1: [ - { featureName: 'feature1', changeRequest: { id: 1 } }, - { featureName: 'feature1', changeRequest: { id: 2 } }, - ], - }; + const expected = [ + { featureName: 'feature1', changeRequest: { id: 1 } }, + { 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 = { - feature1: [ - { 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), @@ -108,45 +98,43 @@ describe('sorting strategies by feature', () => { { key: 'b', featureName: 'feature1', changeRequest: { id: 1 } }, ]; - const expected = { - feature1: [ - { 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 } }, - ], - }; + 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); }); }); - -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); -}); diff --git a/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/sort-strategies.ts b/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/sort-strategies.ts index 2a583ee1b3..1f3b78ed26 100644 --- a/frontend/src/component/segments/SegmentDelete/SegmentDeleteUsedSegment/sort-strategies.ts +++ b/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 }, - U extends { - id?: string; + ExistingStrategy extends { id: string; featureName?: string }, + UpdatedStrategy extends { + id: string; + featureName: string; + changeRequest: { id: number }; + }, + NewStrategy extends { featureName: string; changeRequest: { id: number }; }, >( - strategies: T[], - changeRequestStrategies: U[], -): { [flagName: string]: (T | U)[] } => { - const isExistingStrategy = (strategy: T | U) => 'id' in strategy; - const isNewStrategy = (strategy: T | U) => !isExistingStrategy(strategy); - + strategies: ExistingStrategy[], + changeRequestStrategies: (UpdatedStrategy | NewStrategy)[], +): (ExistingStrategy | UpdatedStrategy | NewStrategy)[] => { 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)[] }, - ); - - const sorted = Object.entries(collected).map( - ([featureName, strategies]) => { - 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]; + {} as { + [flagName: string]: ( + | ExistingStrategy + | UpdatedStrategy + | NewStrategy + )[]; }, ); - 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 = <