From 51f87bdfd9c227548ceab45c7c39f0ab1b522f16 Mon Sep 17 00:00:00 2001 From: Jaanus Sellin Date: Tue, 28 Nov 2023 21:34:57 +0200 Subject: [PATCH] feat: search now also returns segments used (#5429) --- .../feature-search/feature.search.e2e.test.ts | 60 ++++- .../feature-toggle-strategies-store.ts | 74 +++++- src/test/e2e/api/admin/segment.e2e.test.ts | 243 ++++++++++++++---- src/test/e2e/helpers/test-helper.ts | 12 + 4 files changed, 331 insertions(+), 58 deletions(-) diff --git a/src/lib/features/feature-search/feature.search.e2e.test.ts b/src/lib/features/feature-search/feature.search.e2e.test.ts index 5e55bf0808..755942e065 100644 --- a/src/lib/features/feature-search/feature.search.e2e.test.ts +++ b/src/lib/features/feature-search/feature.search.e2e.test.ts @@ -6,6 +6,7 @@ import { import getLogger from '../../../test/fixtures/no-logger'; import { FeatureSearchQueryParameters } from '../../openapi/spec/feature-search-query-parameters'; import { IUnleashStores } from '../../types'; +import { DEFAULT_ENV } from '../../util'; let app: IUnleashTest; let db: ITestDb; @@ -470,7 +471,7 @@ test('should not return duplicate entries when sorting by last seen', async () = await stores.environmentStore.create({ name: 'production', - type: 'production', + type: 'development', }); await app.linkProjectToEnvironment('default', 'production'); @@ -509,20 +510,31 @@ test('should not return duplicate entries when sorting by last seen', async () = test('should search features by description', async () => { const description = 'secretdescription'; await app.createFeature('my_feature_a'); - await app.createFeature({ name: 'my_feature_b', description }); + await app.createFeature({ + name: 'my_feature_b', + description, + }); const { body } = await searchFeatures({ query: 'descr', }); expect(body).toMatchObject({ - features: [{ name: 'my_feature_b', description }], + features: [ + { + name: 'my_feature_b', + description, + }, + ], }); }); test('should support multiple search values', async () => { const description = 'secretdescription'; await app.createFeature('my_feature_a'); - await app.createFeature({ name: 'my_feature_b', description }); + await app.createFeature({ + name: 'my_feature_b', + description, + }); await app.createFeature('my_feature_c'); const { body } = await searchFeatures({ @@ -530,7 +542,10 @@ test('should support multiple search values', async () => { }); expect(body).toMatchObject({ features: [ - { name: 'my_feature_b', description }, + { + name: 'my_feature_b', + description, + }, { name: 'my_feature_c' }, ], }); @@ -598,3 +613,38 @@ test('should search features by project with operators', async () => { features: [{ name: 'my_feature_b' }], }); }); + +test('should return segments in payload with no duplicates/nulls', async () => { + await app.createFeature('my_feature_a'); + const { body: mySegment } = await app.createSegment({ + name: 'my_segment_a', + constraints: [], + }); + + await stores.environmentStore.create({ + name: 'development', + type: 'development', + }); + + await app.linkProjectToEnvironment('default', 'development'); + await app.enableFeature('my_feature_a', 'development'); + await app.addStrategyToFeatureEnv( + { + name: 'default', + segments: [mySegment.id], + }, + DEFAULT_ENV, + 'my_feature_a', + ); + + const { body } = await searchFeatures({}); + + expect(body).toMatchObject({ + features: [ + { + name: 'my_feature_a', + segments: [mySegment.name], + }, + ], + }); +}); diff --git a/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts b/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts index 9694860396..8221f27fe4 100644 --- a/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts +++ b/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts @@ -614,6 +614,21 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { 'feature_tag as ft', 'ft.feature_name', 'features.name', + ) + .leftJoin( + 'feature_strategies', + 'feature_strategies.feature_name', + 'features.name', + ) + .leftJoin( + 'feature_strategy_segment', + 'feature_strategy_segment.feature_strategy_id', + 'feature_strategies.id', + ) + .leftJoin( + 'segments', + 'feature_strategy_segment.segment_id', + 'segments.id', ); if (this.flagResolver.isEnabled('useLastSeenRefactor')) { @@ -645,6 +660,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { 'environments.sort_order as environment_sort_order', 'ft.tag_value as tag_value', 'ft.tag_type as tag_type', + 'segments.name as segment_name', ] as (string | Raw | Knex.QueryBuilder)[]; let lastSeenQuery = 'feature_environments.last_seen_at'; @@ -735,7 +751,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { const rows = await finalQuery; if (rows.length > 0) { - const overview = this.getFeatureOverviewData(rows); + const overview = this.getAggregatedSearchData(rows); const features = sortEnvironments(overview); return { features, @@ -861,6 +877,62 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { return []; } + getAggregatedSearchData(rows): IFeatureOverview { + return rows.reduce((acc, row) => { + if (acc[row.feature_name] !== undefined) { + const environmentExists = acc[ + row.feature_name + ].environments.some( + (existingEnvironment) => + existingEnvironment.name === row.environment, + ); + if (!environmentExists) { + acc[row.feature_name].environments.push( + FeatureStrategiesStore.getEnvironment(row), + ); + } + + const segmentExists = acc[row.feature_name].segments.includes( + row.segment_name, + ); + + if (row.segment_name && !segmentExists) { + acc[row.feature_name].segments.push(row.segment_name); + } + + if (this.isNewTag(acc[row.feature_name], row)) { + this.addTag(acc[row.feature_name], row); + } + } else { + acc[row.feature_name] = { + type: row.type, + description: row.description, + favorite: row.favorite, + name: row.feature_name, + createdAt: row.created_at, + stale: row.stale, + impressionData: row.impression_data, + lastSeenAt: row.last_seen_at, + environments: [FeatureStrategiesStore.getEnvironment(row)], + segments: row.segment_name ? [row.segment_name] : [], + }; + + if (this.isNewTag(acc[row.feature_name], row)) { + this.addTag(acc[row.feature_name], row); + } + } + const featureRow = acc[row.feature_name]; + if ( + featureRow.lastSeenAt === undefined || + new Date(row.env_last_seen_at) > + new Date(featureRow.last_seen_at) + ) { + featureRow.lastSeenAt = row.env_last_seen_at; + } + return acc; + }, {}); + } + getFeatureOverviewData(rows): IFeatureOverview { return rows.reduce((acc, row) => { if (acc[row.feature_name] !== undefined) { diff --git a/src/test/e2e/api/admin/segment.e2e.test.ts b/src/test/e2e/api/admin/segment.e2e.test.ts index 8dd98d1c25..90971caac2 100644 --- a/src/test/e2e/api/admin/segment.e2e.test.ts +++ b/src/test/e2e/api/admin/segment.e2e.test.ts @@ -56,15 +56,6 @@ const fetchSegmentStrategies = ( .expect(200) .then((res) => res.body); -const createSegment = ( - postData: object, - expectStatusCode = 201, -): Promise => - app.request - .post(SEGMENTS_BASE_PATH) - .send(postData) - .expect(expectStatusCode); - const updateSegment = ( id: number, postData: object, @@ -94,7 +85,13 @@ const addSegmentsToStrategy = ( const mockFeatureToggle = () => ({ name: randomId(), - strategies: [{ name: 'flexibleRollout', constraints: [], parameters: {} }], + strategies: [ + { + name: 'flexibleRollout', + constraints: [], + parameters: {}, + }, + ], }); const validateSegment = ( @@ -136,13 +133,38 @@ afterEach(async () => { }); test('should validate segments', async () => { - await createSegment({ something: 'a' }, 400); - await createSegment({ name: randomId(), something: 'b' }, 400); - await createSegment({ name: randomId(), constraints: 'b' }, 400); - await createSegment({ constraints: [] }, 400); - await createSegment({ name: randomId(), constraints: [{}] }, 400); - await createSegment({ name: randomId(), constraints: [] }); - await createSegment({ name: randomId(), description: '', constraints: [] }); + await app.createSegment({ something: 'a' }, 400); + await app.createSegment( + { + name: randomId(), + something: 'b', + }, + 400, + ); + await app.createSegment( + { + name: randomId(), + constraints: 'b', + }, + 400, + ); + await app.createSegment({ constraints: [] }, 400); + await app.createSegment( + { + name: randomId(), + constraints: [{}], + }, + 400, + ); + await app.createSegment({ + name: randomId(), + constraints: [], + }); + await app.createSegment({ + name: randomId(), + description: '', + constraints: [], + }); }); test('should fail on missing properties', async () => { @@ -159,23 +181,38 @@ test('should fail on missing properties', async () => { }); test('should create segments', async () => { - await createSegment({ name: 'a', constraints: [] }); - await createSegment({ name: 'c', constraints: [] }); - await createSegment({ name: 'b', constraints: [] }); + await app.createSegment({ + name: 'a', + constraints: [], + }); + await app.createSegment({ + name: 'c', + constraints: [], + }); + await app.createSegment({ + name: 'b', + constraints: [], + }); const segments = await fetchSegments(); expect(segments.map((s) => s.name)).toEqual(['a', 'b', 'c']); }); test('should update segments', async () => { - await createSegment({ name: 'a', constraints: [] }); + await app.createSegment({ + name: 'a', + constraints: [], + }); const [segmentA] = await fetchSegments(); expect(segmentA.id).toBeGreaterThan(0); expect(segmentA.name).toEqual('a'); expect(segmentA.createdAt).toBeDefined(); expect(segmentA.constraints.length).toEqual(0); - await updateSegment(segmentA.id, { ...segmentA, name: 'b' }); + await updateSegment(segmentA.id, { + ...segmentA, + name: 'b', + }); const [segmentB] = await fetchSegments(); expect(segmentB.id).toEqual(segmentA.id); @@ -185,15 +222,29 @@ test('should update segments', async () => { }); test('should update segment constraints', async () => { - const constraintA = { contextName: 'a', operator: 'IN', values: ['x'] }; - const constraintB = { contextName: 'b', operator: 'IN', values: ['y'] }; - await createSegment({ name: 'a', constraints: [constraintA] }); + const constraintA = { + contextName: 'a', + operator: 'IN', + values: ['x'], + }; + const constraintB = { + contextName: 'b', + operator: 'IN', + values: ['y'], + }; + await app.createSegment({ + name: 'a', + constraints: [constraintA], + }); const [segmentA] = await fetchSegments(); expect(segmentA.constraints).toEqual([constraintA]); await app.request .put(`${SEGMENTS_BASE_PATH}/${segmentA.id}`) - .send({ ...segmentA, constraints: [constraintB, constraintA] }) + .send({ + ...segmentA, + constraints: [constraintB, constraintA], + }) .expect(204); const [segmentB] = await fetchSegments(); @@ -201,7 +252,10 @@ test('should update segment constraints', async () => { }); test('should delete segments', async () => { - await createSegment({ name: 'a', constraints: [] }); + await app.createSegment({ + name: 'a', + constraints: [], + }); const segments = await fetchSegments(); expect(segments.length).toEqual(1); @@ -213,7 +267,10 @@ test('should delete segments', async () => { }); test('should not delete segments used by strategies', async () => { - await createSegment({ name: 'a', constraints: [] }); + await app.createSegment({ + name: 'a', + constraints: [], + }); const toggle = mockFeatureToggle(); await createFeatureToggle(app, toggle); const [segment] = await fetchSegments(); @@ -238,9 +295,18 @@ test('should not delete segments used by strategies', async () => { }); test('should list strategies by segment', async () => { - await createSegment({ name: 'S1', constraints: [] }); - await createSegment({ name: 'S2', constraints: [] }); - await createSegment({ name: 'S3', constraints: [] }); + await app.createSegment({ + name: 'S1', + constraints: [], + }); + await app.createSegment({ + name: 'S2', + constraints: [], + }); + await app.createSegment({ + name: 'S3', + constraints: [], + }); const toggle1 = mockFeatureToggle(); const toggle2 = mockFeatureToggle(); const toggle3 = mockFeatureToggle(); @@ -305,9 +371,18 @@ test('should list strategies by segment', async () => { }); test('should list segments by strategy', async () => { - await createSegment({ name: 'S1', constraints: [] }); - await createSegment({ name: 'S2', constraints: [] }); - await createSegment({ name: 'S3', constraints: [] }); + await app.createSegment({ + name: 'S1', + constraints: [], + }); + await app.createSegment({ + name: 'S2', + constraints: [], + }); + await app.createSegment({ + name: 'S3', + constraints: [], + }); const toggle1 = mockFeatureToggle(); const toggle2 = mockFeatureToggle(); const toggle3 = mockFeatureToggle(); @@ -376,7 +451,10 @@ test('should list segments by strategy', async () => { test('should reject duplicate segment names', async () => { await validateSegment({ name: 'a' }); - await createSegment({ name: 'a', constraints: [] }); + await app.createSegment({ + name: 'a', + constraints: [], + }); await validateSegment({ name: 'a' }, 409); await validateSegment({ name: 'b' }); }); @@ -388,31 +466,75 @@ test('should reject empty segment names', async () => { }); test('should reject duplicate segment names on create', async () => { - await createSegment({ name: 'a', constraints: [] }); - await createSegment({ name: 'a', constraints: [] }, 409); + await app.createSegment({ + name: 'a', + constraints: [], + }); + await app.createSegment( + { + name: 'a', + constraints: [], + }, + 409, + ); await validateSegment({ name: 'b' }); }); test('should reject duplicate segment names on update', async () => { - await createSegment({ name: 'a', constraints: [] }); - await createSegment({ name: 'b', constraints: [] }); + await app.createSegment({ + name: 'a', + constraints: [], + }); + await app.createSegment({ + name: 'b', + constraints: [], + }); const [segmentA, segmentB] = await fetchSegments(); - await updateSegment(segmentA.id, { name: 'b', constraints: [] }, 409); - await updateSegment(segmentB.id, { name: 'a', constraints: [] }, 409); - await updateSegment(segmentA.id, { name: 'a', constraints: [] }); - await updateSegment(segmentA.id, { name: 'c', constraints: [] }); + await updateSegment( + segmentA.id, + { + name: 'b', + constraints: [], + }, + 409, + ); + await updateSegment( + segmentB.id, + { + name: 'a', + constraints: [], + }, + 409, + ); + await updateSegment(segmentA.id, { + name: 'a', + constraints: [], + }); + await updateSegment(segmentA.id, { + name: 'c', + constraints: [], + }); }); test('Should anonymise createdBy field if anonymiseEventLog flag is set', async () => { - await createSegment({ name: 'a', constraints: [] }); - await createSegment({ name: 'b', constraints: [] }); + await app.createSegment({ + name: 'a', + constraints: [], + }); + await app.createSegment({ + name: 'b', + constraints: [], + }); const segments = await fetchSegments(); expect(segments).toHaveLength(2); expect(segments[0].createdBy).toContain('unleash.run'); }); test('Should show usage in features and projects', async () => { - await createSegment({ name: 'a', constraints: [] }); + await app.createSegment({ + name: 'a', + constraints: [], + }); const toggle = mockFeatureToggle(); await createFeatureToggle(app, toggle); const [segment] = await fetchSegments(); @@ -427,7 +549,12 @@ test('Should show usage in features and projects', async () => { await addSegmentsToStrategy([segment.id], feature.strategies[0].id); const segments = await fetchSegments(); - expect(segments).toMatchObject([{ usedInFeatures: 1, usedInProjects: 1 }]); + expect(segments).toMatchObject([ + { + usedInFeatures: 1, + usedInProjects: 1, + }, + ]); }); describe('detect strategy usage in change requests', () => { @@ -497,7 +624,10 @@ describe('detect strategy usage in change requests', () => { }); test('should not delete segments used by strategies in CRs', async () => { - await createSegment({ name: 'a', constraints: [] }); + await app.createSegment({ + name: 'a', + constraints: [], + }); const toggle = mockFeatureToggle(); await createFeatureToggle(enterpriseApp, toggle); const [segment] = await enterpriseFetchSegments(); @@ -538,7 +668,10 @@ describe('detect strategy usage in change requests', () => { }); test('Should show segment usage in addStrategy events', async () => { - await createSegment({ name: 'a', constraints: [] }); + await app.createSegment({ + name: 'a', + constraints: [], + }); const toggle = mockFeatureToggle(); await createFeatureToggle(enterpriseApp, toggle); const [segment] = await enterpriseFetchSegments(); @@ -585,7 +718,10 @@ describe('detect strategy usage in change requests', () => { }); test('Should show segment usage in updateStrategy events', async () => { - await createSegment({ name: 'a', constraints: [] }); + await app.createSegment({ + name: 'a', + constraints: [], + }); const toggle = mockFeatureToggle(); await createFeatureToggle(enterpriseApp, toggle); const [segment] = await enterpriseFetchSegments(); @@ -641,7 +777,10 @@ describe('detect strategy usage in change requests', () => { }); test('If a segment is used in an existing strategy and in a CR for the same strategy, the strategy should be listed both places', async () => { - await createSegment({ name: 'a', constraints: [] }); + await app.createSegment({ + name: 'a', + constraints: [], + }); const toggle = mockFeatureToggle(); await createFeatureToggle(enterpriseApp, toggle); const [segment] = await enterpriseFetchSegments(); @@ -697,7 +836,7 @@ describe('detect strategy usage in change requests', () => { // because they use the same db, we can use the regular app // (through `createSegment` and `createFeatureToggle`) to // create the segment and the flag - await createSegment({ name: 'a', constraints: [] }); + await app.createSegment({ name: 'a', constraints: [] }); const toggle = mockFeatureToggle(); await createFeatureToggle(app, toggle); const [segment] = await enterpriseFetchSegments(); diff --git a/src/test/e2e/helpers/test-helper.ts b/src/test/e2e/helpers/test-helper.ts index 25234eb541..580e2c0f7b 100644 --- a/src/test/e2e/helpers/test-helper.ts +++ b/src/test/e2e/helpers/test-helper.ts @@ -99,6 +99,8 @@ export interface IUnleashHttpAPI { ): supertest.Test; getRecordedEvents(): supertest.Test; + + createSegment(postData: object, expectStatusCode?: number): supertest.Test; } function httpApis( @@ -260,6 +262,16 @@ function httpApis( .set('Content-Type', 'application/json') .expect(expectedResponseCode); }, + createSegment( + postData: object, + expectedResponseCode = 201, + ): supertest.Test { + return request + .post(`/api/admin/segments`) + .send(postData) + .set('Content-Type', 'application/json') + .expect(expectedResponseCode); + }, getRecordedEvents( project: string | null = null,