diff --git a/frontend/src/component/common/FeatureArchiveDialog/FeatureArchiveNotAllowedDialog.tsx b/frontend/src/component/common/FeatureArchiveDialog/FeatureArchiveNotAllowedDialog.tsx new file mode 100644 index 000000000000..34a5a88b474b --- /dev/null +++ b/frontend/src/component/common/FeatureArchiveDialog/FeatureArchiveNotAllowedDialog.tsx @@ -0,0 +1,44 @@ +import { FC } from 'react'; +import { Dialogue } from '../Dialogue/Dialogue'; +import { styled } from '@mui/material'; +import { Link } from 'react-router-dom'; + +interface IFeatureArchiveNotAllowedDialogProps { + isOpen: boolean; + onClose: () => void; + features: string[]; + project: string; +} + +const StyledLink = styled(Link)(({ theme }) => ({ + textDecoration: 'none', + color: theme.palette.primary.main, + fontWeight: theme.fontWeight.bold, +})); +export const FeatureArchiveNotAllowedDialog: FC< + IFeatureArchiveNotAllowedDialogProps +> = ({ isOpen, onClose, features, project }) => { + return ( + +

The following features depend on your feature:

+ +
+ ); +}; diff --git a/frontend/src/component/feature/FeatureView/FeatureView.tsx b/frontend/src/component/feature/FeatureView/FeatureView.tsx index 33045ba326b6..8358ed0a3898 100644 --- a/frontend/src/component/feature/FeatureView/FeatureView.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureView.tsx @@ -1,5 +1,5 @@ import { useState } from 'react'; -import { styled, Tab, Tabs, useMediaQuery, Box, Card } from '@mui/material'; +import { styled, Tab, Tabs, useMediaQuery } from '@mui/material'; import { Archive, FileCopy, Label, WatchLater } from '@mui/icons-material'; import { Link, @@ -29,13 +29,13 @@ import { FeatureStatusChip } from 'component/common/FeatureStatusChip/FeatureSta import { FeatureNotFound } from 'component/feature/FeatureView/FeatureNotFound/FeatureNotFound'; import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; import { FeatureArchiveDialog } from 'component/common/FeatureArchiveDialog/FeatureArchiveDialog'; +import { FeatureArchiveNotAllowedDialog } from 'component/common/FeatureArchiveDialog/FeatureArchiveNotAllowedDialog'; import { useFavoriteFeaturesApi } from 'hooks/api/actions/useFavoriteFeaturesApi/useFavoriteFeaturesApi'; import { FavoriteIconButton } from 'component/common/FavoriteIconButton/FavoriteIconButton'; import { ReactComponent as ChildLinkIcon } from 'assets/icons/link-child.svg'; import { ReactComponent as ParentLinkIcon } from 'assets/icons/link-parent.svg'; -import { TooltipLink } from '../../common/TooltipLink/TooltipLink'; import { ChildrenTooltip } from './FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanelDetails/ChildrenTooltip'; -import { useUiFlag } from '../../../hooks/useUiFlag'; +import { useUiFlag } from 'hooks/useUiFlag'; const StyledHeader = styled('div')(({ theme }) => ({ backgroundColor: theme.palette.background.paper, @@ -321,16 +321,30 @@ export const FeatureView = () => { } /> } /> - { - projectRefetch(); - navigate(`/projects/${projectId}`); - }} - onClose={() => setShowDelDialog(false)} - projectId={projectId} - featureIds={[featureId]} + 0} + show={ + setShowDelDialog(false)} + /> + } + elseShow={ + { + projectRefetch(); + navigate(`/projects/${projectId}`); + }} + onClose={() => setShowDelDialog(false)} + projectId={projectId} + featureIds={[featureId]} + /> + } /> + ; + getChildren(parents: string[]): Promise; getParents(child: string): Promise; getParentOptions(child: 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 69dfab1e8337..dcc0366831de 100644 --- a/src/lib/features/dependent-features/dependent-features-read-model.ts +++ b/src/lib/features/dependent-features/dependent-features-read-model.ts @@ -9,13 +9,13 @@ export class DependentFeaturesReadModel implements IDependentFeaturesReadModel { this.db = db; } - async getChildren(parent: string): Promise { - const rows = await this.db('dependent_features').where( + async getChildren(parents: string[]): Promise { + const rows = await this.db('dependent_features').whereIn( 'parent', - parent, + parents, ); - return rows.map((row) => row.child); + return [...new Set(rows.map((row) => row.child))]; } async getParents(child: string): Promise { diff --git a/src/lib/features/dependent-features/dependent-features-service.ts b/src/lib/features/dependent-features/dependent-features-service.ts index 8d29e26ffbb2..1afc08d770c9 100644 --- a/src/lib/features/dependent-features/dependent-features-service.ts +++ b/src/lib/features/dependent-features/dependent-features-service.ts @@ -29,9 +29,9 @@ export class DependentFeaturesService { ): Promise { const { enabled, feature: parent, variants } = dependentFeature; - const children = await this.dependentFeaturesReadModel.getChildren( + const children = await this.dependentFeaturesReadModel.getChildren([ child, - ); + ]); if (children.length > 0) { throw new InvalidOperationError( 'Transitive dependency detected. Cannot add a dependency to the feature that other features depend on.', diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 711496826458..872cedb86f81 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -251,6 +251,22 @@ class FeatureToggleService { } } + async validateNoChildren(featureNames: string[]): Promise { + if (this.flagResolver.isEnabled('dependentFeatures')) { + if (featureNames.length === 0) return; + const children = await this.dependentFeaturesReadModel.getChildren( + featureNames, + ); + if (children.length > 0) { + throw new InvalidOperationError( + featureNames.length > 1 + ? `You can not archive/delete those features since other features depend on them.` + : `You can not archive/delete this feature since other features depend on it.`, + ); + } + } + } + validateUpdatedProperties( { featureName, projectId }: IFeatureContext, existingStrategy: IFeatureStrategy, @@ -925,7 +941,7 @@ class FeatureToggleService { if (this.flagResolver.isEnabled('dependentFeatures')) { [dependencies, children] = await Promise.all([ this.dependentFeaturesReadModel.getParents(featureName), - this.dependentFeaturesReadModel.getChildren(featureName), + this.dependentFeaturesReadModel.getChildren([featureName]), ]); } @@ -1424,6 +1440,8 @@ class FeatureToggleService { }); } + await this.validateNoChildren([featureName]); + await this.featureToggleStore.archive(featureName); await this.eventService.storeEvent( @@ -1441,6 +1459,7 @@ class FeatureToggleService { projectId: string, ): Promise { await this.validateFeaturesContext(featureNames, projectId); + await this.validateNoChildren(featureNames); const features = await this.featureToggleStore.getAllByNames( featureNames, @@ -1734,6 +1753,7 @@ class FeatureToggleService { // TODO: add project id. async deleteFeature(featureName: string, createdBy: string): Promise { + await this.validateNoChildren([featureName]); const toggle = await this.featureToggleStore.get(featureName); const tags = await this.tagStore.getAllTagsForFeature(featureName); await this.featureToggleStore.delete(featureName); @@ -1755,6 +1775,7 @@ class FeatureToggleService { createdBy: string, ): Promise { await this.validateFeaturesContext(featureNames, projectId); + await this.validateNoChildren(featureNames); const features = await this.featureToggleStore.getAllByNames( featureNames, 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 1f33738bf654..961292686d44 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/src/test/e2e/api/admin/project/features.e2e.test.ts @@ -241,6 +241,29 @@ test('should list dependencies and children', async () => { }); }); +test('Should not allow to archive/delete feature with children', async () => { + const parent = uuidv4(); + const child = uuidv4(); + await app.createFeature(parent, 'default'); + await app.createFeature(child, 'default'); + await app.addDependency(child, parent); + + const { body: archiveBody } = await app.request + .delete(`/api/admin/projects/default/features/${parent}`) + .expect(403); + const { body: deleteBody } = await app.request + .post(`/api/admin/projects/default/delete`) + .set('Content-Type', 'application/json') + .send({ features: [parent] }) + .expect(403); + expect(archiveBody.message).toBe( + 'You can not archive/delete this feature since other features depend on it.', + ); + expect(deleteBody.message).toBe( + 'You can not archive/delete this feature since other features depend on it.', + ); +}); + test('Can get features for project', async () => { await app.request .post('/api/admin/projects/default/features')