diff --git a/frontend/src/component/feature/Dependencies/AddDependencyDialogue.tsx b/frontend/src/component/feature/Dependencies/AddDependencyDialogue.tsx index 00a2ca8408..1a61ca3b08 100644 --- a/frontend/src/component/feature/Dependencies/AddDependencyDialogue.tsx +++ b/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) { diff --git a/frontend/src/hooks/api/actions/useChangeRequestApi/useChangeRequestApi.ts b/frontend/src/hooks/api/actions/useChangeRequestApi/useChangeRequestApi.ts index 58e352d207..38f7ca7f0f 100644 --- a/frontend/src/hooks/api/actions/useChangeRequestApi/useChangeRequestApi.ts +++ b/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; } diff --git a/src/lib/features/dependent-features/createDependentFeaturesService.ts b/src/lib/features/dependent-features/createDependentFeaturesService.ts index 016511720f..f00fa202b8 100644 --- a/src/lib/features/dependent-features/createDependentFeaturesService.ts +++ b/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, ); }; diff --git a/src/lib/features/dependent-features/dependent-features-controller.ts b/src/lib/features/dependent-features/dependent-features-controller.ts index b6439f66db..dd1b6585e1 100644 --- a/src/lib/features/dependent-features/dependent-features-controller.ts +++ b/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 { diff --git a/src/lib/features/dependent-features/dependent-features-service.ts b/src/lib/features/dependent-features/dependent-features-service.ts index 7c891aeb77..b33bf73c44 100644 --- a/src/lib/features/dependent-features/dependent-features-service.ts +++ b/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 { + 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 { + 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 { + 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 { 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); + } + } } diff --git a/src/test/e2e/api/client/feature.e2e.test.ts b/src/test/e2e/api/client/feature.e2e.test.ts index ce150b078a..aee5bdf75c 100644 --- a/src/test/e2e/api/client/feature.e2e.test.ts +++ b/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',