Skip to content

Commit

Permalink
feat: change project with feature dependencies (#4915)
Browse files Browse the repository at this point in the history
  • Loading branch information
kwasniew authored Oct 4, 2023
1 parent 1c4897d commit 5141d9d
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -40,6 +41,7 @@ const FeatureSettingsProjectConfirm = ({
feature,
changeRequests,
}: IFeatureSettingsProjectConfirm) => {
const dependentFeatures = useUiFlag('dependentFeatures');
const currentProjectId = useRequiredPathParam('projectId');
const { project } = useProject(projectId);

Expand All @@ -58,10 +60,15 @@ const FeatureSettingsProjectConfirm = ({
? changeRequests.length > 0
: false;

const hasDependencies =
dependentFeatures &&
(feature.dependencies.length > 0 || feature.children.length > 0);

return (
<ConditionallyRender
condition={
hasSameEnvironments &&
!hasDependencies &&
!hasPendingChangeRequests &&
!targetProjectHasChangeRequestsEnabled
}
Expand Down Expand Up @@ -98,6 +105,22 @@ const FeatureSettingsProjectConfirm = ({
Cannot proceed with the move
</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
condition={!hasSameEnvironments}
show={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ export interface IDependentFeaturesReadModel {
getChildren(parents: string[]): Promise<string[]>;
getParents(child: string): Promise<IDependency[]>;
getParentOptions(child: string): Promise<string[]>;
hasDependencies(feature: string): Promise<boolean>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,13 @@ export class DependentFeaturesReadModel implements IDependentFeaturesReadModel {

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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,8 @@ export class FakeDependentFeaturesReadModel
getParentOptions(): Promise<string[]> {
return Promise.resolve([]);
}

hasDependencies(): Promise<boolean> {
return Promise.resolve(false);
}
}
7 changes: 7 additions & 0 deletions src/lib/services/feature-toggle-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
31 changes: 31 additions & 0 deletions src/test/e2e/api/admin/project/features.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down

1 comment on commit 5141d9d

@vercel
Copy link

@vercel vercel bot commented on 5141d9d Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.