feat: do not allow to manage dependencies directly with cr enabled (#4971)

frontend/src/component/feature/Dependencies/AddDependencyDialogue.tsx

@@ -76,7 +76,7 @@ const useManageDependency = (
         useDependentFeaturesApi(project);
 
     const handleAddChange = async (
-        actionType: 'addDependency' | 'deleteDependencies',
+        actionType: 'addDependency' | 'deleteDependency',
     ) => {
         if (!environment) {
             console.error('No change request environment');
@@ -91,7 +91,7 @@ const useManageDependency = (
                 },
             ]);
         }
-        if (actionType === 'deleteDependencies') {
+        if (actionType === 'deleteDependency') {
             await addChange(project, environment, [
                 { action: actionType, feature: featureId, payload: undefined },
             ]);
@@ -112,7 +112,7 @@ const useManageDependency = (
             if (isChangeRequestConfiguredInAnyEnv()) {
                 await handleAddChange(
                     parent === REMOVE_DEPENDENCY_OPTION.key
-                        ? 'deleteDependencies'
+                        ? 'deleteDependency'
                         : 'addDependency',
                 );
             } else if (parent === REMOVE_DEPENDENCY_OPTION.key) {

frontend/src/hooks/api/actions/useChangeRequestApi/useChangeRequestApi.ts

@@ -13,7 +13,7 @@ export interface IChangeSchema {
         | 'archiveFeature'
         | 'updateSegment'
         | 'addDependency'
-        | 'deleteDependencies';
+        | 'deleteDependency';
     payload: string | boolean | object | number | undefined;
 }
 

src/lib/features/dependent-features/createDependentFeaturesService.ts

@@ -10,6 +10,10 @@ import { EventService } from '../../services';
 import FeatureTagStore from '../../db/feature-tag-store';
 import FakeEventStore from '../../../test/fixtures/fake-event-store';
 import FakeFeatureTagStore from '../../../test/fixtures/fake-feature-tag-store';
+import {
+    createChangeRequestAccessReadModel,
+    createFakeChangeRequestAccessService,
+} from '../change-request-access-service/createChangeRequestAccessReadModel';
 
 export const createDependentFeaturesService = (
     db: Db,
@@ -27,9 +31,14 @@ export const createDependentFeaturesService = (
     );
     const dependentFeaturesStore = new DependentFeaturesStore(db);
     const dependentFeaturesReadModel = new DependentFeaturesReadModel(db);
+    const changeRequestAccessReadModel = createChangeRequestAccessReadModel(
+        db,
+        config,
+    );
     return new DependentFeaturesService(
         dependentFeaturesStore,
         dependentFeaturesReadModel,
+        changeRequestAccessReadModel,
         eventService,
     );
 };
@@ -48,9 +57,12 @@ export const createFakeDependentFeaturesService = (
     );
     const dependentFeaturesStore = new FakeDependentFeaturesStore();
     const dependentFeaturesReadModel = new FakeDependentFeaturesReadModel();
+    const changeRequestAccessReadModel = createFakeChangeRequestAccessService();
+
     return new DependentFeaturesService(
         dependentFeaturesStore,
         dependentFeaturesReadModel,
+        changeRequestAccessReadModel,
         eventService,
     );
 };

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

@@ -186,7 +186,7 @@ export default class DependentFeaturesController extends Controller {
                         enabled,
                         feature,
                     },
-                    extractUsernameFromUser(req.user),
+                    req.user,
                 ),
             );
             res.status(200).end();
@@ -210,7 +210,7 @@ export default class DependentFeaturesController extends Controller {
                     child,
                 },
                 projectId,
-                extractUsernameFromUser(req.user),
+                req.user,
             );
             res.status(200).end();
         } else {
@@ -230,7 +230,7 @@ export default class DependentFeaturesController extends Controller {
             await this.dependentFeaturesService.deleteFeatureDependencies(
                 child,
                 projectId,
-                extractUsernameFromUser(req.user),
+                req.user,
             );
             res.status(200).end();
         } else {

src/lib/features/dependent-features/dependent-features-service.ts

@@ -1,24 +1,32 @@
-import { InvalidOperationError } from '../../error';
+import { InvalidOperationError, PermissionError } from '../../error';
 import { CreateDependentFeatureSchema } from '../../openapi';
 import { IDependentFeaturesStore } from './dependent-features-store-type';
 import { FeatureDependency, FeatureDependencyId } from './dependent-features';
 import { IDependentFeaturesReadModel } from './dependent-features-read-model-type';
 import { EventService } from '../../services';
+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';
 
 export class DependentFeaturesService {
     private dependentFeaturesStore: IDependentFeaturesStore;
 
     private dependentFeaturesReadModel: IDependentFeaturesReadModel;
 
+    private changeRequestAccessReadModel: IChangeRequestAccessReadModel;
+
     private eventService: EventService;
 
     constructor(
         dependentFeaturesStore: IDependentFeaturesStore,
         dependentFeaturesReadModel: IDependentFeaturesReadModel,
+        changeRequestAccessReadModel: IChangeRequestAccessReadModel,
         eventService: EventService,
     ) {
         this.dependentFeaturesStore = dependentFeaturesStore;
         this.dependentFeaturesReadModel = dependentFeaturesReadModel;
+        this.changeRequestAccessReadModel = changeRequestAccessReadModel;
         this.eventService = eventService;
     }
 
@@ -35,7 +43,7 @@ export class DependentFeaturesService {
         );
         await Promise.all(
             parents.map((parent) =>
-                this.upsertFeatureDependency(
+                this.unprotectedUpsertFeatureDependency(
                     { child: newFeatureName, projectId },
                     {
                         feature: parent.feature,
@@ -49,6 +57,20 @@ export class DependentFeaturesService {
     }
 
     async upsertFeatureDependency(
+        { child, projectId }: { child: string; projectId: string },
+        dependentFeature: CreateDependentFeatureSchema,
+        user: User,
+    ): Promise<void> {
+        await this.stopWhenChangeRequestsEnabled(projectId, user);
+
+        return this.unprotectedUpsertFeatureDependency(
+            { child, projectId },
+            dependentFeature,
+            extractUsernameFromUser(user),
+        );
+    }
+
+    async unprotectedUpsertFeatureDependency(
         { child, projectId }: { child: string; projectId: string },
         dependentFeature: CreateDependentFeatureSchema,
         user: string,
@@ -92,6 +114,20 @@ export class DependentFeaturesService {
     }
 
     async deleteFeatureDependency(
+        dependency: FeatureDependencyId,
+        projectId: string,
+        user: User,
+    ): Promise<void> {
+        await this.stopWhenChangeRequestsEnabled(projectId, user);
+
+        return this.unprotectedDeleteFeatureDependency(
+            dependency,
+            projectId,
+            extractUsernameFromUser(user),
+        );
+    }
+
+    async unprotectedDeleteFeatureDependency(
         dependency: FeatureDependencyId,
         projectId: string,
         user: string,
@@ -107,6 +143,20 @@ export class DependentFeaturesService {
     }
 
     async deleteFeatureDependencies(
+        feature: string,
+        projectId: string,
+        user: User,
+    ): Promise<void> {
+        await this.stopWhenChangeRequestsEnabled(projectId, user);
+
+        return this.unprotectedDeleteFeatureDependencies(
+            feature,
+            projectId,
+            extractUsernameFromUser(user),
+        );
+    }
+
+    async unprotectedDeleteFeatureDependencies(
         feature: string,
         projectId: string,
         user: string,
@@ -123,4 +173,15 @@ export class DependentFeaturesService {
     async getParentOptions(feature: string): Promise<string[]> {
         return this.dependentFeaturesReadModel.getParentOptions(feature);
     }
+
+    private async stopWhenChangeRequestsEnabled(project: string, user?: User) {
+        const canBypass =
+            await this.changeRequestAccessReadModel.canBypassChangeRequestForProject(
+                project,
+                user,
+            );
+        if (!canBypass) {
+            throw new PermissionError(SKIP_CHANGE_REQUEST);
+        }
+    }
 }

src/test/e2e/api/client/feature.e2e.test.ts

@@ -61,7 +61,7 @@ beforeAll(async () => {
         'test',
     );
     // depend on enabled feature with variant
-    await app.services.dependentFeaturesService.upsertFeatureDependency(
+    await app.services.dependentFeaturesService.unprotectedUpsertFeatureDependency(
         { child: 'featureY', projectId: 'default' },
         { feature: 'featureX', variants: ['featureXVariant'] },
         'test',