diff --git a/src/lib/features/change-request-segment-usage-service/change-request-segment-usage-read-model.test.ts b/src/lib/features/change-request-segment-usage-service/change-request-segment-usage-read-model.test.ts index 26ae25c690..f2581c2208 100644 --- a/src/lib/features/change-request-segment-usage-service/change-request-segment-usage-read-model.test.ts +++ b/src/lib/features/change-request-segment-usage-service/change-request-segment-usage-read-model.test.ts @@ -113,7 +113,7 @@ describe.each([ ])('Should handle %s changes correctly', (_, addOrUpdateStrategy) => { test.each([ ['Draft', true], - ['In Review', true], + ['In review', true], ['Scheduled', true], ['Approved', true], ['Rejected', false], @@ -133,3 +133,76 @@ describe.each([ }, ); }); + +test.each([ + ['Draft', true], + ['In review', true], + ['Scheduled', true], + ['Approved', true], + ['Rejected', false], + ['Cancelled', false], + ['Applied', false], +])( + 'addStrategy events in %s CRs should show up only of the CR is active', + async (state, isActiveCr) => { + await createCR(state); + + const segmentId = 3; + + await addStrategyToCr(segmentId, FLAG_NAME); + + const result = await readModel.getStrategiesUsedInActiveChangeRequests( + segmentId, + ); + if (isActiveCr) { + expect(result).toStrictEqual([ + { + projectId: 'default', + strategyName: 'flexibleRollout', + environment: 'default', + featureName: FLAG_NAME, + }, + ]); + } else { + expect(result).toStrictEqual([]); + } + }, +); + +test.each([ + ['Draft', true], + ['In review', true], + ['Scheduled', true], + ['Approved', true], + ['Rejected', false], + ['Cancelled', false], + ['Applied', false], +])( + `updateStrategy events in %s CRs should show up only of the CR is active`, + async (state, isActiveCr) => { + await createCR(state); + + const segmentId = 3; + + const strategyId = randomId(); + await updateStrategyInCr(strategyId, segmentId, FLAG_NAME); + + const result = await readModel.getStrategiesUsedInActiveChangeRequests( + segmentId, + ); + + if (isActiveCr) { + expect(result).toMatchObject([ + { + id: strategyId, + projectId: 'default', + strategyName: 'flexibleRollout', + environment: 'default', + featureName: FLAG_NAME, + }, + ]); + } else { + expect(result).toStrictEqual([]); + } + }, +); diff --git a/src/lib/features/change-request-segment-usage-service/change-request-segment-usage-read-model.ts b/src/lib/features/change-request-segment-usage-service/change-request-segment-usage-read-model.ts index 08ba2f3217..4b680fd006 100644 --- a/src/lib/features/change-request-segment-usage-service/change-request-segment-usage-read-model.ts +++ b/src/lib/features/change-request-segment-usage-service/change-request-segment-usage-read-model.ts @@ -1,3 +1,17 @@ +type NewStrategy = { + projectId: string; + featureName: string; + strategyName: string; + environment: string; +}; + +type ExistingStrategy = NewStrategy & { id: string }; + +export type ChangeRequestStrategy = NewStrategy | ExistingStrategy; + export interface IChangeRequestSegmentUsageReadModel { isSegmentUsedInActiveChangeRequests(segmentId: number): Promise; + getStrategiesUsedInActiveChangeRequests( + segmentId: number, + ): Promise; } diff --git a/src/lib/features/change-request-segment-usage-service/fake-change-request-segment-usage-read-model.ts b/src/lib/features/change-request-segment-usage-service/fake-change-request-segment-usage-read-model.ts index d5cb25554a..a5e0573b45 100644 --- a/src/lib/features/change-request-segment-usage-service/fake-change-request-segment-usage-read-model.ts +++ b/src/lib/features/change-request-segment-usage-service/fake-change-request-segment-usage-read-model.ts @@ -1,16 +1,32 @@ -import { IChangeRequestSegmentUsageReadModel } from './change-request-segment-usage-read-model'; +import { + ChangeRequestStrategy, + IChangeRequestSegmentUsageReadModel, +} from './change-request-segment-usage-read-model'; export class FakeChangeRequestSegmentUsageReadModel implements IChangeRequestSegmentUsageReadModel { private isSegmentUsedInActiveChangeRequestsValue: boolean; + strategiesUsedInActiveChangeRequests: ChangeRequestStrategy[]; - constructor(isSegmentUsedInActiveChangeRequests = false) { + constructor( + isSegmentUsedInActiveChangeRequests = false, + strategiesUsedInActiveChangeRequests = [], + ) { this.isSegmentUsedInActiveChangeRequestsValue = isSegmentUsedInActiveChangeRequests; + + this.strategiesUsedInActiveChangeRequests = + strategiesUsedInActiveChangeRequests; } public async isSegmentUsedInActiveChangeRequests(): Promise { return this.isSegmentUsedInActiveChangeRequestsValue; } + + public async getStrategiesUsedInActiveChangeRequests(): Promise< + ChangeRequestStrategy[] + > { + return this.strategiesUsedInActiveChangeRequests; + } } diff --git a/src/lib/features/change-request-segment-usage-service/sql-change-request-segment-usage-read-model.ts b/src/lib/features/change-request-segment-usage-service/sql-change-request-segment-usage-read-model.ts index 47dbf632ae..f33f5606cc 100644 --- a/src/lib/features/change-request-segment-usage-service/sql-change-request-segment-usage-read-model.ts +++ b/src/lib/features/change-request-segment-usage-service/sql-change-request-segment-usage-read-model.ts @@ -1,5 +1,8 @@ import { Db } from '../../db/db'; -import { IChangeRequestSegmentUsageReadModel } from './change-request-segment-usage-read-model'; +import { + ChangeRequestStrategy, + IChangeRequestSegmentUsageReadModel, +} from './change-request-segment-usage-read-model'; export class ChangeRequestSegmentUsageReadModel implements IChangeRequestSegmentUsageReadModel @@ -9,7 +12,6 @@ export class ChangeRequestSegmentUsageReadModel constructor(db: Db) { this.db = db; } - public async isSegmentUsedInActiveChangeRequests( segmentId: number, ): Promise { @@ -17,7 +19,7 @@ export class ChangeRequestSegmentUsageReadModel `SELECT events.* FROM change_request_events events JOIN change_requests cr ON events.change_request_id = cr.id - WHERE cr.state IN ('Draft', 'In Review', 'Scheduled', 'Approved') + WHERE cr.state IN ('Draft', 'In review', 'Scheduled', 'Approved') AND events.action IN ('updateStrategy', 'addStrategy');`, ); @@ -27,4 +29,34 @@ export class ChangeRequestSegmentUsageReadModel return isUsed; } + + mapRow = (row): ChangeRequestStrategy => { + const { payload, project, environment, feature } = row; + return { + projectId: project, + featureName: feature, + environment: environment, + strategyName: payload.name, + ...(payload.id ? { id: payload.id } : {}), + }; + }; + + public async getStrategiesUsedInActiveChangeRequests( + segmentId: number, + ): Promise { + const query = this.db.raw( + `SELECT events.*, cr.project, cr.environment + FROM change_request_events events + JOIN change_requests cr ON events.change_request_id = cr.id + WHERE cr.state NOT IN ('Applied', 'Cancelled', 'Rejected') + AND events.action IN ('updateStrategy', 'addStrategy');`, + ); + + const queryResult = await query; + const strategies = queryResult.rows + .filter((row) => row.payload?.segments?.includes(segmentId)) + .map(this.mapRow); + + return strategies; + } } diff --git a/src/lib/openapi/spec/admin-strategies-schema.ts b/src/lib/openapi/spec/admin-strategies-schema.ts index e6124d83e1..53cc4b74b2 100644 --- a/src/lib/openapi/spec/admin-strategies-schema.ts +++ b/src/lib/openapi/spec/admin-strategies-schema.ts @@ -26,7 +26,8 @@ export const segmentStrategiesSchema = { }, featureName: { type: 'string', - description: 'The ID of the strategy', + description: + 'The name of the feature flag that this strategy belongs to.', example: 'new-signup-flow', }, projectId: { diff --git a/src/test/e2e/api/admin/segment.e2e.test.ts b/src/test/e2e/api/admin/segment.e2e.test.ts index 90dccbfa0a..e4e6cf2236 100644 --- a/src/test/e2e/api/admin/segment.e2e.test.ts +++ b/src/test/e2e/api/admin/segment.e2e.test.ts @@ -249,7 +249,7 @@ test('should not delete segments used by strategies in CRs', async () => { await db.rawDatabase.table('change_requests').insert({ id: CR_ID, environment: 'default', - state: 'In Review', + state: 'In review', project: 'default', created_by: user.id, created_at: '2023-01-01 00:00:00',