feat: allow to delete dependencies when no orphans (#4952)

Unleash · Oct 6, 2023 · 8b0cf8b · 8b0cf8b
1 parent 52fa872
commit 8b0cf8b
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 9 deletions.
diff --git a/src/lib/features/dependent-features/dependent-features-read-model-type.ts b/src/lib/features/dependent-features/dependent-features-read-model-type.ts
@@ -2,6 +2,9 @@ import { IDependency } from '../../types';
 
 export interface IDependentFeaturesReadModel {
     getChildren(parents: string[]): Promise<string[]>;
+    // given a list of parents and children verifies if some children would be orphaned after deletion
+    // we're interested in the list of parents, not orphans
+    getOrphanParents(parentsAndChildren: string[]): Promise<string[]>;
     getParents(child: string): Promise<IDependency[]>;
     getParentOptions(child: string): Promise<string[]>;
     hasDependencies(feature: string): Promise<boolean>;

diff --git a/src/lib/features/dependent-features/dependent-features-read-model.ts b/src/lib/features/dependent-features/dependent-features-read-model.ts
@@ -9,6 +9,21 @@ export class DependentFeaturesReadModel implements IDependentFeaturesReadModel {
         this.db = db;
     }
 
+    async getOrphanParents(parentsAndChildren: string[]): Promise<string[]> {
+        const rows = await this.db('dependent_features')
+            .distinct('parent')
+            .whereIn('parent', parentsAndChildren)
+            .andWhere(function () {
+                this.whereIn('parent', function () {
+                    this.select('parent')
+                        .from('dependent_features')
+                        .whereNotIn('child', parentsAndChildren);
+                });
+            });
+
+        return rows.map((row) => row.parent);
+    }
+
     async getChildren(parents: string[]): Promise<string[]> {
         const rows = await this.db('dependent_features').whereIn(
             'parent',

diff --git a/src/lib/features/dependent-features/fake-dependent-features-read-model.ts b/src/lib/features/dependent-features/fake-dependent-features-read-model.ts
@@ -19,4 +19,8 @@ export class FakeDependentFeaturesReadModel
     hasDependencies(): Promise<boolean> {
         return Promise.resolve(false);
     }
+
+    getOrphanParents(parentsAndChildren: string[]): Promise<string[]> {
+        return Promise.resolve([]);
+    }
 }
diff --git a/src/lib/features/index.ts b/src/lib/features/index.ts
@@ -4,3 +4,4 @@ export * from './feature-toggle/createFeatureToggleService';
 export * from './project/createProjectService';
 export * from './change-request-access-service/createChangeRequestAccessReadModel';
 export * from './segment/createSegmentService';
+export * from './dependent-features/createDependentFeaturesService';
diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts
@@ -256,13 +256,27 @@ class FeatureToggleService {
         }
     }
 
-    async validateNoChildren(featureNames: string[]): Promise<void> {
+    async validateNoChildren(featureName: string): Promise<void> {
         if (this.flagResolver.isEnabled('dependentFeatures')) {
-            if (featureNames.length === 0) return;
-            const children = await this.dependentFeaturesReadModel.getChildren(
-                featureNames,
-            );
+            const children = await this.dependentFeaturesReadModel.getChildren([
+                featureName,
+            ]);
             if (children.length > 0) {
+                throw new InvalidOperationError(
+                    'You can not archive/delete this feature since other features depend on it.',
+                );
+            }
+        }
+    }
+
+    async validateNoOrphanParents(featureNames: string[]): Promise<void> {
+        if (this.flagResolver.isEnabled('dependentFeatures')) {
+            if (featureNames.length === 0) return;
+            const parents =
+                await this.dependentFeaturesReadModel.getOrphanParents(
+                    featureNames,
+                );
+            if (parents.length > 0) {
                 throw new InvalidOperationError(
                     featureNames.length > 1
                         ? `You can not archive/delete those features since other features depend on them.`
@@ -1460,7 +1474,7 @@ class FeatureToggleService {
             });
         }
 
-        await this.validateNoChildren([featureName]);
+        await this.validateNoChildren(featureName);
 
         await this.featureToggleStore.archive(featureName);
 
@@ -1479,7 +1493,7 @@ class FeatureToggleService {
         projectId: string,
     ): Promise<void> {
         await this.validateFeaturesContext(featureNames, projectId);
-        await this.validateNoChildren(featureNames);
+        await this.validateNoOrphanParents(featureNames);
 
         const features = await this.featureToggleStore.getAllByNames(
             featureNames,
@@ -1780,7 +1794,7 @@ class FeatureToggleService {
 
     // TODO: add project id.
     async deleteFeature(featureName: string, createdBy: string): Promise<void> {
-        await this.validateNoChildren([featureName]);
+        await this.validateNoChildren(featureName);
         const toggle = await this.featureToggleStore.get(featureName);
         const tags = await this.tagStore.getAllTagsForFeature(featureName);
         await this.featureToggleStore.delete(featureName);
@@ -1802,7 +1816,7 @@ class FeatureToggleService {
         createdBy: string,
     ): Promise<void> {
         await this.validateFeaturesContext(featureNames, projectId);
-        await this.validateNoChildren(featureNames);
+        await this.validateNoOrphanParents(featureNames);
 
         const features = await this.featureToggleStore.getAllByNames(
             featureNames,

diff --git a/src/test/e2e/api/admin/project/features.e2e.test.ts b/src/test/e2e/api/admin/project/features.e2e.test.ts
@@ -295,6 +295,41 @@ test('Should not allow to archive/delete feature with children', async () => {
     );
 });
 
+test('Should allow to archive/delete feature with children if no orphans are left', async () => {
+    const parent = uuidv4();
+    const child = uuidv4();
+    await app.createFeature(parent, 'default');
+    await app.createFeature(child, 'default');
+    await app.addDependency(child, parent);
+
+    const { body: deleteBody } = await app.request
+        .post(`/api/admin/projects/default/delete`)
+        .set('Content-Type', 'application/json')
+        .send({ features: [parent, child] })
+        .expect(200);
+});
+
+test('Should not allow to archive/delete feature when orphans are left', async () => {
+    const parent = uuidv4();
+    const child = uuidv4();
+    const orphan = uuidv4();
+    await app.createFeature(parent, 'default');
+    await app.createFeature(child, 'default');
+    await app.createFeature(orphan, 'default');
+    await app.addDependency(child, parent);
+    await app.addDependency(orphan, parent);
+
+    const { body: deleteBody } = await app.request
+        .post(`/api/admin/projects/default/delete`)
+        .set('Content-Type', 'application/json')
+        .send({ features: [parent, child] })
+        .expect(403);
+
+    expect(deleteBody.message).toBe(
+        'You can not archive/delete those features since other features depend on them.',
+    );
+});
+
 test('should clone feature with parent dependencies', async () => {
     const parent = uuidv4();
     const child = uuidv4();