1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-06-04 01:18:20 +02:00

feat: change project with feature dependencies (#4915)

This commit is contained in:
Mateusz Kwasniewski 2023-10-04 12:16:52 +02:00 committed by GitHub
parent 1c4897da4d
commit 5141d9db67
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 138 additions and 0 deletions

View File

@ -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(
<UIProviderContainer>
<Routes>
<Route
path={'projects/:projectId/features/:featureId/settings'}
element={
<FeatureSettingsProjectConfirm
projectId={'newProjectId'}
feature={
{
environments: [],
dependencies: [],
children: ['child'],
} as unknown as IFeatureToggle
}
onClose={() => {
closed = true;
}}
onClick={() => {}}
open={true}
changeRequests={[]}
/>
}
/>
</Routes>
</UIProviderContainer>,
{
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);
});
});

View File

@ -9,6 +9,7 @@ import { Link } from 'react-router-dom';
import { IChangeRequest } from 'component/changeRequest/changeRequest.types'; import { IChangeRequest } from 'component/changeRequest/changeRequest.types';
import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; import { useRequiredPathParam } from 'hooks/useRequiredPathParam';
import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled'; import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled';
import { useUiFlag } from 'hooks/useUiFlag';
const StyledContainer = styled('div')(({ theme }) => ({ const StyledContainer = styled('div')(({ theme }) => ({
display: 'grid', display: 'grid',
@ -40,6 +41,7 @@ const FeatureSettingsProjectConfirm = ({
feature, feature,
changeRequests, changeRequests,
}: IFeatureSettingsProjectConfirm) => { }: IFeatureSettingsProjectConfirm) => {
const dependentFeatures = useUiFlag('dependentFeatures');
const currentProjectId = useRequiredPathParam('projectId'); const currentProjectId = useRequiredPathParam('projectId');
const { project } = useProject(projectId); const { project } = useProject(projectId);
@ -58,10 +60,15 @@ const FeatureSettingsProjectConfirm = ({
? changeRequests.length > 0 ? changeRequests.length > 0
: false; : false;
const hasDependencies =
dependentFeatures &&
(feature.dependencies.length > 0 || feature.children.length > 0);
return ( return (
<ConditionallyRender <ConditionallyRender
condition={ condition={
hasSameEnvironments && hasSameEnvironments &&
!hasDependencies &&
!hasPendingChangeRequests && !hasPendingChangeRequests &&
!targetProjectHasChangeRequestsEnabled !targetProjectHasChangeRequestsEnabled
} }
@ -98,6 +105,22 @@ const FeatureSettingsProjectConfirm = ({
Cannot proceed with the move Cannot proceed with the move
</StyledAlert> </StyledAlert>
<ConditionallyRender
condition={hasDependencies}
show={
<p>
<span>
The feature toggle must not have any
dependencies.
</span>{' '}
<br />
<span>
Please remove feature dependencies
first.
</span>
</p>
}
/>
<ConditionallyRender <ConditionallyRender
condition={!hasSameEnvironments} condition={!hasSameEnvironments}
show={ show={

View File

@ -4,4 +4,5 @@ export interface IDependentFeaturesReadModel {
getChildren(parents: string[]): Promise<string[]>; getChildren(parents: string[]): Promise<string[]>;
getParents(child: string): Promise<IDependency[]>; getParents(child: string): Promise<IDependency[]>;
getParentOptions(child: string): Promise<string[]>; getParentOptions(child: string): Promise<string[]>;
hasDependencies(feature: string): Promise<boolean>;
} }

View File

@ -49,4 +49,13 @@ export class DependentFeaturesReadModel implements IDependentFeaturesReadModel {
return rows.map((item) => item.name); return rows.map((item) => item.name);
} }
async hasDependencies(feature: string): Promise<boolean> {
const parents = await this.db('dependent_features')
.where('parent', feature)
.orWhere('child', feature)
.limit(1);
return parents.length > 0;
}
} }

View File

@ -15,4 +15,8 @@ export class FakeDependentFeaturesReadModel
getParentOptions(): Promise<string[]> { getParentOptions(): Promise<string[]> {
return Promise.resolve([]); return Promise.resolve([]);
} }
hasDependencies(): Promise<boolean> {
return Promise.resolve(false);
}
} }

View File

@ -1732,6 +1732,13 @@ class FeatureToggleService {
`Changing project not allowed. Project ${newProject} has change requests enabled.`, `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 feature = await this.featureToggleStore.get(featureName);
const oldProject = feature.project; const oldProject = feature.project;
feature.project = newProject; feature.project = newProject;

View File

@ -27,6 +27,7 @@ import supertest from 'supertest';
import { randomId } from '../../../../../lib/util/random-id'; import { randomId } from '../../../../../lib/util/random-id';
import { DEFAULT_PROJECT } from '../../../../../lib/types'; import { DEFAULT_PROJECT } from '../../../../../lib/types';
import { FeatureStrategySchema, SetStrategySortOrderSchema } from 'lib/openapi'; import { FeatureStrategySchema, SetStrategySortOrderSchema } from 'lib/openapi';
import { ForbiddenError } from '../../../../../lib/error';
let app: IUnleashTest; let app: IUnleashTest;
let db: ITestDb; 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 () => { test('Should not allow to archive/delete feature with children', async () => {
const parent = uuidv4(); const parent = uuidv4();
const child = uuidv4(); const child = uuidv4();