feat: API prevents you from deleting segments in crs (#5308)

This PR hooks up the changes introduced in #5301 to the API and puts
them behind a feature flag. A new test has been added and the test setup
has been slightly tweaked to allow this test.

When the flag is enabled, the API will now not let you delete a segment
that's used in any active CRs.

src/lib/features/change-request-segment-usage-read-model.ts/change-request-segment-usage-read-model.test.ts → src/lib/features/change-request-segment-usage-service/change-request-segment-usage-read-model.test.ts

@@ -2,7 +2,7 @@ import { IUser } from 'lib/server-impl';
 import dbInit, { ITestDb } from '../../../test/e2e/helpers/database-init';
 import getLogger from '../../../test/fixtures/no-logger';
 import { IChangeRequestSegmentUsageReadModel } from './change-request-segment-usage-read-model';
-import { createChangeRequestSegmentUsageModel } from './createChangeRequestSegmentUsageReadModel';
+import { createChangeRequestSegmentUsageReadModel } from './createChangeRequestSegmentUsageReadModel';
 import { randomId } from '../../../lib/util';
 
 let db: ITestDb;
@@ -20,7 +20,7 @@ beforeAll(async () => {
         username: 'cr-creator',
     });
 
-    readModel = createChangeRequestSegmentUsageModel(db.rawDatabase);
+    readModel = createChangeRequestSegmentUsageReadModel(db.rawDatabase);
 
     await db.stores.featureToggleStore.create('default', {
         name: FLAG_NAME,

src/lib/features/change-request-segment-usage-read-model.ts/createChangeRequestSegmentUsageReadModel.ts → src/lib/features/change-request-segment-usage-service/createChangeRequestSegmentUsageReadModel.ts

@@ -3,13 +3,13 @@ import { ChangeRequestSegmentUsageReadModel } from './sql-change-request-segment
 import { FakeChangeRequestSegmentUsageReadModel } from './fake-change-request-segment-usage-read-model';
 import { IChangeRequestSegmentUsageReadModel } from './change-request-segment-usage-read-model';
 
-export const createChangeRequestSegmentUsageModel = (
+export const createChangeRequestSegmentUsageReadModel = (
     db: Db,
 ): IChangeRequestSegmentUsageReadModel => {
     return new ChangeRequestSegmentUsageReadModel(db);
 };
 
-export const createFakeChangeRequestAccessService =
+export const createFakeChangeRequestSegmentUsageReadModel =
     (): IChangeRequestSegmentUsageReadModel => {
         return new FakeChangeRequestSegmentUsageReadModel();
     };

src/lib/features/segment/createSegmentService.ts

@@ -11,6 +11,10 @@ import {
     createChangeRequestAccessReadModel,
     createFakeChangeRequestAccessService,
 } from '../change-request-access-service/createChangeRequestAccessReadModel';
+import {
+    createChangeRequestSegmentUsageReadModel,
+    createFakeChangeRequestSegmentUsageReadModel,
+} from '../change-request-segment-usage-service/createChangeRequestSegmentUsageReadModel';
 import {
     createFakePrivateProjectChecker,
     createPrivateProjectChecker,
@@ -40,6 +44,10 @@ export const createSegmentService = (
         db,
         config,
     );
+
+    const changeRequestSegmentUsageReadModel =
+        createChangeRequestSegmentUsageReadModel(db);
+
     const privateProjectChecker = createPrivateProjectChecker(db, config);
 
     const eventService = new EventService(
@@ -53,6 +61,7 @@ export const createSegmentService = (
     return new SegmentService(
         { segmentStore, featureStrategiesStore },
         changeRequestAccessReadModel,
+        changeRequestSegmentUsageReadModel,
         config,
         eventService,
         privateProjectChecker,
@@ -66,6 +75,8 @@ export const createFakeSegmentService = (
     const segmentStore = new FakeSegmentStore();
     const featureStrategiesStore = new FakeFeatureStrategiesStore();
     const changeRequestAccessReadModel = createFakeChangeRequestAccessService();
+    const changeRequestSegmentUsageReadModel =
+        createFakeChangeRequestSegmentUsageReadModel();
 
     const privateProjectChecker = createFakePrivateProjectChecker();
 
@@ -80,6 +91,7 @@ export const createFakeSegmentService = (
     return new SegmentService(
         { segmentStore, featureStrategiesStore },
         changeRequestAccessReadModel,
+        changeRequestSegmentUsageReadModel,
         config,
         eventService,
         privateProjectChecker,

src/lib/features/segment/segment-controller.ts

@@ -373,9 +373,16 @@ export class SegmentsController extends Controller {
         res: Response,
     ): Promise<void> {
         const id = Number(req.params.id);
-        const strategies = await this.segmentService.getAllStrategies(id);
 
-        if (strategies.length > 0) {
+        let segmentIsInUse = false;
+        if (this.flagResolver.isEnabled('detectSegmentUsageInChangeRequests')) {
+            segmentIsInUse = await this.segmentService.isInUse(id);
+        } else {
+            const strategies = await this.segmentService.getAllStrategies(id);
+            segmentIsInUse = strategies.length > 0;
+        }
+
+        if (segmentIsInUse) {
             res.status(409).send();
         } else {
             await this.segmentService.delete(id, req.user);

src/lib/segments/segment-service-interface.ts

@@ -60,4 +60,6 @@ export interface ISegmentService {
         sourceStrategyId: string,
         targetStrategyId: string,
     ): Promise<void>;
+
+    isInUse(id: number): Promise<boolean>;
 }

src/lib/services/index.ts

@@ -63,6 +63,10 @@ import {
     createChangeRequestAccessReadModel,
     createFakeChangeRequestAccessService,
 } from '../features/change-request-access-service/createChangeRequestAccessReadModel';
+import {
+    createChangeRequestSegmentUsageReadModel,
+    createFakeChangeRequestSegmentUsageReadModel,
+} from '../features/change-request-segment-usage-service/createChangeRequestSegmentUsageReadModel';
 import ConfigurationRevisionService from '../features/feature-toggle/configuration-revision-service';
 import {
     createFakeProjectService,
@@ -322,9 +326,15 @@ export const createServices = (
     const changeRequestAccessReadModel = db
         ? createChangeRequestAccessReadModel(db, config)
         : createFakeChangeRequestAccessService();
+
+    const changeRequestSegmentUsageReadModel = db
+        ? createChangeRequestSegmentUsageReadModel(db)
+        : createFakeChangeRequestSegmentUsageReadModel();
+
     const segmentService = new SegmentService(
         stores,
         changeRequestAccessReadModel,
+        changeRequestSegmentUsageReadModel,
         config,
         eventService,
         privateProjectChecker,

src/lib/services/segment-service.ts

@@ -23,6 +23,7 @@ import { PermissionError } from '../error';
 import { IChangeRequestAccessReadModel } from '../features/change-request-access-service/change-request-access-read-model';
 import { IPrivateProjectChecker } from '../features/private-project/privateProjectCheckerType';
 import EventService from './event-service';
+import { IChangeRequestSegmentUsageReadModel } from 'lib/features/change-request-segment-usage-service/change-request-segment-usage-read-model';
 
 export class SegmentService implements ISegmentService {
     private logger: Logger;
@@ -33,6 +34,8 @@ export class SegmentService implements ISegmentService {
 
     private changeRequestAccessReadModel: IChangeRequestAccessReadModel;
 
+    private changeRequestSegmentUsageReadModel: IChangeRequestSegmentUsageReadModel;
+
     private config: IUnleashConfig;
 
     private flagResolver: IFlagResolver;
@@ -47,6 +50,7 @@ export class SegmentService implements ISegmentService {
             featureStrategiesStore,
         }: Pick<IUnleashStores, 'segmentStore' | 'featureStrategiesStore'>,
         changeRequestAccessReadModel: IChangeRequestAccessReadModel,
+        changeRequestSegmentUsageReadModel: IChangeRequestSegmentUsageReadModel,
         config: IUnleashConfig,
         eventService: EventService,
         privateProjectChecker: IPrivateProjectChecker,
@@ -55,6 +59,8 @@ export class SegmentService implements ISegmentService {
         this.featureStrategiesStore = featureStrategiesStore;
         this.eventService = eventService;
         this.changeRequestAccessReadModel = changeRequestAccessReadModel;
+        this.changeRequestSegmentUsageReadModel =
+            changeRequestSegmentUsageReadModel;
         this.privateProjectChecker = privateProjectChecker;
         this.logger = config.getLogger('services/segment-service.ts');
         this.flagResolver = config.flagResolver;
@@ -108,6 +114,17 @@ export class SegmentService implements ISegmentService {
         return strategies;
     }
 
+    async isInUse(id: number): Promise<boolean> {
+        const strategies = await this.getAllStrategies(id);
+        if (strategies.length > 0) {
+            return true;
+        }
+
+        return await this.changeRequestSegmentUsageReadModel.isSegmentUsedInActiveChangeRequests(
+            id,
+        );
+    }
+
     async create(
         data: unknown,
         user: Partial<Pick<User, 'username' | 'email'>>,

src/test/e2e/api/admin/segment.e2e.test.ts

@@ -108,14 +108,18 @@ const validateSegment = (
 
 beforeAll(async () => {
     db = await dbInit('segments_api_serial', getLogger);
-    app = await setupAppWithCustomConfig(db.stores, {
-        experimental: {
-            flags: {
-                strictSchemaValidation: true,
-                anonymiseEventLog: true,
+    app = await setupAppWithCustomConfig(
+        db.stores,
+        {
+            experimental: {
+                flags: {
+                    anonymiseEventLog: true,
+                    detectSegmentUsageInChangeRequests: true,
+                },
             },
         },
-    });
+        db.rawDatabase,
+    );
 });
 
 afterAll(async () => {
@@ -230,6 +234,57 @@ test('should not delete segments used by strategies', async () => {
     expect((await fetchSegments()).length).toEqual(1);
 });
 
+test('should not delete segments used by strategies in CRs', async () => {
+    await createSegment({ name: 'a', constraints: [] });
+    const toggle = mockFeatureToggle();
+    await createFeatureToggle(app, toggle);
+    const [segment] = await fetchSegments();
+
+    const CR_ID = 54321;
+
+    const user = await db.stores.userStore.insert({
+        username: 'test',
+    });
+
+    await db.rawDatabase.table('change_requests').insert({
+        id: CR_ID,
+        environment: 'default',
+        state: 'In Review',
+        project: 'default',
+        created_by: user.id,
+        created_at: '2023-01-01 00:00:00',
+        min_approvals: 1,
+        title: 'My change request',
+    });
+
+    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,
+    });
+
+    expect((await fetchSegments()).length).toEqual(1);
+
+    await app.request.delete(`${SEGMENTS_BASE_PATH}/${segment.id}`).expect(409);
+
+    expect((await fetchSegments()).length).toEqual(1);
+});
+
 test('should list strategies by segment', async () => {
     await createSegment({ name: 'S1', constraints: [] });
     await createSegment({ name: 'S2', constraints: [] });