diff --git a/src/lib/features/segment/segment-controller.ts b/src/lib/features/segment/segment-controller.ts index 8e6089f8ea..1ca21e14d2 100644 --- a/src/lib/features/segment/segment-controller.ts +++ b/src/lib/features/segment/segment-controller.ts @@ -354,18 +354,41 @@ export class SegmentsController extends Controller { user.id, ); - // Remove unnecessary IFeatureStrategy fields from the response. - const segmentStrategies = strategies.strategies.map((strategy) => ({ - id: strategy.id, - projectId: strategy.projectId, - featureName: strategy.featureName, - strategyName: strategy.strategyName, - environment: strategy.environment, - })); + if (this.flagResolver.isEnabled('detectSegmentUsageInChangeRequests')) { + // Remove unnecessary IFeatureStrategy fields from the response. + const mapStrategies = (strategy) => ({ + id: strategy.id, + projectId: strategy.projectId, + featureName: strategy.featureName, + strategyName: strategy.strategyName, + environment: strategy.environment, + }); - res.json({ - strategies: segmentStrategies, - }); + const mapChangeRequestStrategies = (strategy) => ({ + ...(strategy.id ? { id: strategy.id } : {}), + projectId: strategy.projectId, + featureName: strategy.featureName, + strategyName: strategy.strategyName, + environment: strategy.environment, + }); + + res.json({ + strategies: strategies.strategies.map(mapStrategies), + changeRequestStrategies: strategies.changeRequestStrategies.map( + mapChangeRequestStrategies, + ), + }); + } else { + const segmentStrategies = strategies.strategies.map((strategy) => ({ + id: strategy.id, + projectId: strategy.projectId, + featureName: strategy.featureName, + strategyName: strategy.strategyName, + environment: strategy.environment, + })); + + res.json({ strategies: segmentStrategies }); + } } async removeSegment( diff --git a/src/lib/services/segment-service.ts b/src/lib/services/segment-service.ts index 3b4cfe7df6..74751d800f 100644 --- a/src/lib/services/segment-service.ts +++ b/src/lib/services/segment-service.ts @@ -94,25 +94,25 @@ export class SegmentService implements ISegmentService { id: number, userId: number, ): Promise { - const strategies = await this.getAllStrategies(id); + const allStrategies = await this.getAllStrategies(id); if (this.flagResolver.isEnabled('privateProjects')) { const accessibleProjects = await this.privateProjectChecker.getUserAccessibleProjects( userId, ); if (accessibleProjects.mode === 'all') { - return strategies; + return allStrategies; } else { const filter = (strategy) => accessibleProjects.projects.includes(strategy.projectId); return { - strategies: strategies.strategies.filter(filter), + strategies: allStrategies.strategies.filter(filter), changeRequestStrategies: - strategies.changeRequestStrategies.filter(filter), + allStrategies.changeRequestStrategies.filter(filter), }; } } - return strategies; + return allStrategies; } async getAllStrategies(id: number): Promise { @@ -123,6 +123,7 @@ export class SegmentService implements ISegmentService { await this.changeRequestSegmentUsageReadModel.getStrategiesUsedInActiveChangeRequests( id, ); + return { strategies, changeRequestStrategies }; } diff --git a/src/test/e2e/api/admin/segment.e2e.test.ts b/src/test/e2e/api/admin/segment.e2e.test.ts index d043eeaf01..4326b8eb41 100644 --- a/src/test/e2e/api/admin/segment.e2e.test.ts +++ b/src/test/e2e/api/admin/segment.e2e.test.ts @@ -52,7 +52,7 @@ const fetchSegmentStrategies = (segmentId: number): Promise => app.request .get(`${SEGMENTS_BASE_PATH}/${segmentId}/strategies`) .expect(200) - .then((res) => res.body.strategies); + .then((res) => res.body); const createSegment = ( postData: object, @@ -283,15 +283,15 @@ test('should list strategies by segment', async () => { const segmentStrategies2 = await fetchSegmentStrategies(segment2.id); const segmentStrategies3 = await fetchSegmentStrategies(segment3.id); - expect(collectIds(segmentStrategies1)).toEqual( + expect(collectIds(segmentStrategies1.strategies)).toEqual( collectIds(feature1.strategies), ); - expect(collectIds(segmentStrategies2)).toEqual( + expect(collectIds(segmentStrategies2.strategies)).toEqual( collectIds([...feature1.strategies, ...feature2.strategies]), ); - expect(collectIds(segmentStrategies3)).toEqual( + expect(collectIds(segmentStrategies3.strategies)).toEqual( collectIds([ ...feature1.strategies, ...feature2.strategies, @@ -434,12 +434,7 @@ describe('detect strategy usage in change requests', () => { user = await db.stores.userStore.insert({ username: 'test', }); - }); - afterAll(async () => { - user = await db.stores.userStore.delete(user.id); - }); - beforeEach(async () => { await db.rawDatabase.table('change_requests').insert({ id: CR_ID, environment: 'default', @@ -451,9 +446,13 @@ describe('detect strategy usage in change requests', () => { title: 'My change request', }); }); + afterAll(async () => { + user = await db.stores.userStore.delete(user.id); + await db.rawDatabase.table('change_requests').delete(); + }); afterEach(async () => { - await db.rawDatabase.table('change_requests').delete(); + await db.rawDatabase.table('change_request_events').delete(); }); test('should not delete segments used by strategies in CRs', async () => { @@ -492,6 +491,47 @@ describe('detect strategy usage in change requests', () => { expect((await fetchSegments()).length).toEqual(1); }); + test('Should show segment usage in addStrategy events', async () => { + await createSegment({ name: 'a', constraints: [] }); + const toggle = mockFeatureToggle(); + await createFeatureToggle(app, toggle); + const [segment] = await fetchSegments(); + + await db.rawDatabase.table('change_request_events').insert({ + feature: toggle.name, + action: 'addStrategy', + payload: { + name: 'flexibleRollout', + title: '', + disabled: false, + segments: [segment.id], + variants: [], + parameters: { + groupId: toggle.name, + rollout: '100', + stickiness: 'default', + }, + constraints: [], + }, + created_at: '2023-01-01 00:01:00', + change_request_id: CR_ID, + created_by: user.id, + }); + + const { strategies, changeRequestStrategies } = + await fetchSegmentStrategies(segment.id); + + expect(changeRequestStrategies).toMatchObject([ + { + environment: 'default', + featureName: toggle.name, + projectId: 'default', + strategyName: 'flexibleRollout', + }, + ]); + expect(strategies).toStrictEqual([]); + }); + test('Should show segment usage in updateStrategy events', async () => { await createSegment({ name: 'a', constraints: [] }); const toggle = mockFeatureToggle(); @@ -531,9 +571,6 @@ describe('detect strategy usage in change requests', () => { created_by: user.id, }); - // for addStrategy, add strategy to feature with segment - // check that getStrategies for segments contains the CR strategies - const { strategies, changeRequestStrategies } = await fetchSegmentStrategies(segment.id); @@ -541,43 +578,6 @@ describe('detect strategy usage in change requests', () => { expect(strategies).toStrictEqual([]); }); - test('Should show segment usage in addStrategy events', async () => { - await createSegment({ name: 'a', constraints: [] }); - const toggle = mockFeatureToggle(); - await createFeatureToggle(app, toggle); - const [segment] = await fetchSegments(); - - await db.rawDatabase.table('change_request_events').insert({ - feature: toggle.name, - action: 'addStrategy', - payload: { - name: 'flexibleRollout', - title: '', - disabled: false, - segments: [segment.id], - variants: [], - parameters: { - groupId: toggle.name, - rollout: '100', - stickiness: 'default', - }, - constraints: [], - }, - created_at: '2023-01-01 00:01:00', - change_request_id: CR_ID, - created_by: user.id, - }); - - // for updateStrategy, add existing strategy, then add segment in CR - // check that getStrategies for segments contains the CR strategies - - const { strategies, changeRequestStrategies } = - await fetchSegmentStrategies(segment.id); - - expect(changeRequestStrategies).toMatchObject([{ x: 'bluh' }]); - expect(strategies).toStrictEqual([]); - }); - test('If a segment is used in an existing strategy and in a CR for the same strategy, the strategy should only be listed once', async () => { await createSegment({ name: 'a', constraints: [] }); const toggle = mockFeatureToggle();