From 5141d9db676a7fcc6a82cc127df94012d9f6f25b Mon Sep 17 00:00:00 2001 From: Mateusz Kwasniewski Date: Wed, 4 Oct 2023 12:16:52 +0200 Subject: [PATCH] feat: change project with feature dependencies (#4915) --- .../FeatureSettingsProjectConfirm.test.tsx | 63 +++++++++++++++++++ .../FeatureSettingsProjectConfirm.tsx | 23 +++++++ .../dependent-features-read-model-type.ts | 1 + .../dependent-features-read-model.ts | 9 +++ .../fake-dependent-features-read-model.ts | 4 ++ src/lib/services/feature-toggle-service.ts | 7 +++ .../api/admin/project/features.e2e.test.ts | 31 +++++++++ 7 files changed, 138 insertions(+) create mode 100644 frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.test.tsx diff --git a/frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.test.tsx b/frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.test.tsx new file mode 100644 index 000000000000..90a06225c299 --- /dev/null +++ b/frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.test.tsx @@ -0,0 +1,63 @@ +import { screen, waitFor } from '@testing-library/react'; +import { render } from 'utils/testRenderer'; +import { testServerRoute, testServerSetup } from 'utils/testServer'; +import FeatureSettingsProjectConfirm from './FeatureSettingsProjectConfirm'; +import { IFeatureToggle } from 'interfaces/featureToggle'; +import { UIProviderContainer } from '../../../../../providers/UIProvider/UIProviderContainer'; +import { Route, Routes } from 'react-router-dom'; +import React from 'react'; +import userEvent from '@testing-library/user-event'; + +const server = testServerSetup(); + +const setupApi = () => { + testServerRoute(server, '/api/admin/ui-config', { + flags: { + dependentFeatures: true, + }, + }); +}; + +test('Cannot change project for feature with dependencies', async () => { + let closed = false; + setupApi(); + render( + + + { + closed = true; + }} + onClick={() => {}} + open={true} + changeRequests={[]} + /> + } + /> + + , + { + route: 'projects/default/features/parent/settings', + }, + ); + + await screen.findByText('Please remove feature dependencies first.'); + + const closeButton = await screen.findByText('Close'); + userEvent.click(closeButton); + + await waitFor(() => { + expect(closed).toBe(true); + }); +}); diff --git a/frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.tsx b/frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.tsx index 6e5e11d910ef..619108c44c29 100644 --- a/frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.tsx @@ -9,6 +9,7 @@ import { Link } from 'react-router-dom'; import { IChangeRequest } from 'component/changeRequest/changeRequest.types'; import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled'; +import { useUiFlag } from 'hooks/useUiFlag'; const StyledContainer = styled('div')(({ theme }) => ({ display: 'grid', @@ -40,6 +41,7 @@ const FeatureSettingsProjectConfirm = ({ feature, changeRequests, }: IFeatureSettingsProjectConfirm) => { + const dependentFeatures = useUiFlag('dependentFeatures'); const currentProjectId = useRequiredPathParam('projectId'); const { project } = useProject(projectId); @@ -58,10 +60,15 @@ const FeatureSettingsProjectConfirm = ({ ? changeRequests.length > 0 : false; + const hasDependencies = + dependentFeatures && + (feature.dependencies.length > 0 || feature.children.length > 0); + return ( + + + The feature toggle must not have any + dependencies. + {' '} +
+ + Please remove feature dependencies + first. + +

+ } + /> ; getParents(child: string): Promise; getParentOptions(child: string): Promise; + hasDependencies(feature: string): Promise; } diff --git a/src/lib/features/dependent-features/dependent-features-read-model.ts b/src/lib/features/dependent-features/dependent-features-read-model.ts index dcc0366831de..b52b4484c8a4 100644 --- a/src/lib/features/dependent-features/dependent-features-read-model.ts +++ b/src/lib/features/dependent-features/dependent-features-read-model.ts @@ -49,4 +49,13 @@ export class DependentFeaturesReadModel implements IDependentFeaturesReadModel { return rows.map((item) => item.name); } + + async hasDependencies(feature: string): Promise { + const parents = await this.db('dependent_features') + .where('parent', feature) + .orWhere('child', feature) + .limit(1); + + return parents.length > 0; + } } 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 index 80311c58934a..6f7ca3cc6e65 100644 --- a/src/lib/features/dependent-features/fake-dependent-features-read-model.ts +++ b/src/lib/features/dependent-features/fake-dependent-features-read-model.ts @@ -15,4 +15,8 @@ export class FakeDependentFeaturesReadModel getParentOptions(): Promise { return Promise.resolve([]); } + + hasDependencies(): Promise { + return Promise.resolve(false); + } } diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 872cedb86f81..7dec5526fb93 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -1732,6 +1732,13 @@ class FeatureToggleService { `Changing project not allowed. Project ${newProject} has change requests enabled.`, ); } + if ( + await this.dependentFeaturesReadModel.hasDependencies(featureName) + ) { + throw new ForbiddenError( + 'Changing project not allowed. Feature has dependencies.', + ); + } const feature = await this.featureToggleStore.get(featureName); const oldProject = feature.project; feature.project = newProject; diff --git a/src/test/e2e/api/admin/project/features.e2e.test.ts b/src/test/e2e/api/admin/project/features.e2e.test.ts index 961292686d44..da3635e723c2 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/src/test/e2e/api/admin/project/features.e2e.test.ts @@ -27,6 +27,7 @@ import supertest from 'supertest'; import { randomId } from '../../../../../lib/util/random-id'; import { DEFAULT_PROJECT } from '../../../../../lib/types'; import { FeatureStrategySchema, SetStrategySortOrderSchema } from 'lib/openapi'; +import { ForbiddenError } from '../../../../../lib/error'; let app: IUnleashTest; let db: ITestDb; @@ -241,6 +242,36 @@ test('should list dependencies and children', async () => { }); }); +test('should not allow to change project with dependencies', async () => { + const parent = uuidv4(); + const child = uuidv4(); + await app.createFeature(parent, 'default'); + await app.createFeature(child, 'default'); + await app.addDependency(child, parent); + const user = new ApiUser({ + tokenName: 'project-changer', + permissions: ['ADMIN'], + project: '*', + type: ApiTokenType.ADMIN, + environment: '*', + secret: 'a', + }); + + await expect(async () => + app.services.projectService.changeProject( + 'default', + child, + // @ts-ignore + user, + 'default', + ), + ).rejects.toThrow( + new ForbiddenError( + 'Changing project not allowed. Feature has dependencies.', + ), + ); +}); + test('Should not allow to archive/delete feature with children', async () => { const parent = uuidv4(); const child = uuidv4();