feat: prevent adding dependency to archived or removed parent (#4987)

Unleash · Oct 11, 2023 · 30e9fb8 · 30e9fb8
1 parent 7ea7c08
commit 30e9fb8
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 12 deletions.
diff --git a/src/lib/features/dependent-features/createDependentFeaturesService.ts b/src/lib/features/dependent-features/createDependentFeaturesService.ts
@@ -14,6 +14,8 @@ import {
     createChangeRequestAccessReadModel,
     createFakeChangeRequestAccessService,
 } from '../change-request-access-service/createChangeRequestAccessReadModel';
+import { FeaturesReadModel } from '../feature-toggle/features-read-model';
+import { FakeFeaturesReadModel } from '../feature-toggle/fake-features-read-model';
 
 export const createDependentFeaturesService = (
     db: Db,
@@ -35,12 +37,14 @@ export const createDependentFeaturesService = (
         db,
         config,
     );
-    return new DependentFeaturesService(
+    const featuresReadModel = new FeaturesReadModel(db);
+    return new DependentFeaturesService({
         dependentFeaturesStore,
         dependentFeaturesReadModel,
         changeRequestAccessReadModel,
+        featuresReadModel,
         eventService,
-    );
+    });
 };
 
 export const createFakeDependentFeaturesService = (
@@ -58,11 +62,13 @@ export const createFakeDependentFeaturesService = (
     const dependentFeaturesStore = new FakeDependentFeaturesStore();
     const dependentFeaturesReadModel = new FakeDependentFeaturesReadModel();
     const changeRequestAccessReadModel = createFakeChangeRequestAccessService();
+    const featuresReadModel = new FakeFeaturesReadModel();
 
-    return new DependentFeaturesService(
+    return new DependentFeaturesService({
         dependentFeaturesStore,
         dependentFeaturesReadModel,
         changeRequestAccessReadModel,
+        featuresReadModel,
         eventService,
-    );
+    });
 };
diff --git a/src/lib/features/dependent-features/dependent-features-service.ts b/src/lib/features/dependent-features/dependent-features-service.ts
@@ -8,6 +8,15 @@ import { User } from '../../server-impl';
 import { SKIP_CHANGE_REQUEST } from '../../types';
 import { IChangeRequestAccessReadModel } from '../change-request-access-service/change-request-access-read-model';
 import { extractUsernameFromUser } from '../../util';
+import { IFeaturesReadModel } from '../feature-toggle/features-read-model-type';
+
+interface IDependentFeaturesServiceDeps {
+    dependentFeaturesStore: IDependentFeaturesStore;
+    dependentFeaturesReadModel: IDependentFeaturesReadModel;
+    changeRequestAccessReadModel: IChangeRequestAccessReadModel;
+    featuresReadModel: IFeaturesReadModel;
+    eventService: EventService;
+}
 
 export class DependentFeaturesService {
     private dependentFeaturesStore: IDependentFeaturesStore;
@@ -16,17 +25,21 @@ export class DependentFeaturesService {
 
     private changeRequestAccessReadModel: IChangeRequestAccessReadModel;
 
+    private featuresReadModel: IFeaturesReadModel;
+
     private eventService: EventService;
 
-    constructor(
-        dependentFeaturesStore: IDependentFeaturesStore,
-        dependentFeaturesReadModel: IDependentFeaturesReadModel,
-        changeRequestAccessReadModel: IChangeRequestAccessReadModel,
-        eventService: EventService,
-    ) {
+    constructor({
+        featuresReadModel,
+        dependentFeaturesReadModel,
+        dependentFeaturesStore,
+        eventService,
+        changeRequestAccessReadModel,
+    }: IDependentFeaturesServiceDeps) {
         this.dependentFeaturesStore = dependentFeaturesStore;
         this.dependentFeaturesReadModel = dependentFeaturesReadModel;
         this.changeRequestAccessReadModel = changeRequestAccessReadModel;
+        this.featuresReadModel = featuresReadModel;
         this.eventService = eventService;
     }
 
@@ -77,15 +90,23 @@ export class DependentFeaturesService {
     ): Promise<void> {
         const { enabled, feature: parent, variants } = dependentFeature;
 
-        const children = await this.dependentFeaturesReadModel.getChildren([
-            child,
+        const [children, parentExists] = await Promise.all([
+            this.dependentFeaturesReadModel.getChildren([child]),
+            this.featuresReadModel.featureExists(parent),
         ]);
+
         if (children.length > 0) {
             throw new InvalidOperationError(
                 'Transitive dependency detected. Cannot add a dependency to the feature that other features depend on.',
             );
         }
 
+        if (!parentExists) {
+            throw new InvalidOperationError(
+                `No active feature ${parent} exists`,
+            );
+        }
+
         const featureDependency: FeatureDependency =
             enabled === false
                 ? {

diff --git a/src/lib/features/dependent-features/dependent.features.e2e.test.ts b/src/lib/features/dependent-features/dependent.features.e2e.test.ts
@@ -136,3 +136,34 @@ test('should not allow to add a parent dependency to a feature that already has
         403,
     );
 });
+
+test('should not allow to add non-existent parent dependency', async () => {
+    const grandparent = uuidv4();
+    const parent = uuidv4();
+    const child = uuidv4();
+    await app.createFeature(child);
+
+    await addFeatureDependency(
+        child,
+        {
+            feature: parent,
+        },
+        403,
+    );
+});
+
+test('should not allow to add archived parent dependency', async () => {
+    const parent = uuidv4();
+    const child = uuidv4();
+    await app.createFeature(child);
+    await app.createFeature(parent);
+    await app.archiveFeature(parent);
+
+    await addFeatureDependency(
+        child,
+        {
+            feature: parent,
+        },
+        403,
+    );
+});
diff --git a/src/lib/features/feature-toggle/fake-features-read-model.ts b/src/lib/features/feature-toggle/fake-features-read-model.ts
@@ -0,0 +1,7 @@
+import { IFeaturesReadModel } from './features-read-model-type';
+
+export class FakeFeaturesReadModel implements IFeaturesReadModel {
+    featureExists(): Promise<boolean> {
+        return Promise.resolve(false);
+    }
+}
diff --git a/src/lib/features/feature-toggle/features-read-model-type.ts b/src/lib/features/feature-toggle/features-read-model-type.ts
@@ -0,0 +1,3 @@
+export interface IFeaturesReadModel {
+    featureExists(parent: string): Promise<boolean>;
+}
diff --git a/src/lib/features/feature-toggle/features-read-model.ts b/src/lib/features/feature-toggle/features-read-model.ts
@@ -0,0 +1,19 @@
+import { Db } from '../../db/db';
+import { IFeaturesReadModel } from './features-read-model-type';
+
+export class FeaturesReadModel implements IFeaturesReadModel {
+    private db: Db;
+
+    constructor(db: Db) {
+        this.db = db;
+    }
+
+    async featureExists(parent: string): Promise<boolean> {
+        const rows = await this.db('features')
+            .where('name', parent)
+            .andWhere('archived_at', null)
+            .select('name');
+
+        return rows.length > 0;
+    }
+}