feat: prevent delete and archive on parent feature (#4913)

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 (
+        <Dialogue
+            title="You can't archive a feature that other features depend on"
+            open={isOpen}
+            primaryButtonText='OK'
+            onClick={onClose}
+        >
+            <p>The following features depend on your feature:</p>
+            <ul>
+                {features.map((feature) => (
+                    <li key={feature}>
+                        <StyledLink
+                            to={`/projects/${project}/features/${feature}`}
+                            target='_blank'
+                            rel='noopener noreferrer'
+                        >
+                            {feature}
+                        </StyledLink>
+                    </li>
+                ))}
+            </ul>
+        </Dialogue>
+    );
+};

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 = () => {
                 <Route path='settings' element={<FeatureSettings />} />
                 <Route path='*' element={<FeatureOverview />} />
             </Routes>
-            <FeatureArchiveDialog
-                isOpen={showDelDialog}
-                onConfirm={() => {
-                    projectRefetch();
-                    navigate(`/projects/${projectId}`);
-                }}
-                onClose={() => setShowDelDialog(false)}
-                projectId={projectId}
-                featureIds={[featureId]}
+            <ConditionallyRender
+                condition={feature.children.length > 0}
+                show={
+                    <FeatureArchiveNotAllowedDialog
+                        features={feature.children}
+                        project={projectId}
+                        isOpen={showDelDialog}
+                        onClose={() => setShowDelDialog(false)}
+                    />
+                }
+                elseShow={
+                    <FeatureArchiveDialog
+                        isOpen={showDelDialog}
+                        onConfirm={() => {
+                            projectRefetch();
+                            navigate(`/projects/${projectId}`);
+                        }}
+                        onClose={() => setShowDelDialog(false)}
+                        projectId={projectId}
+                        featureIds={[featureId]}
+                    />
+                }
             />
+
             <FeatureStaleDialog
                 isStale={feature.stale}
                 isOpen={openStaleDialog}

src/lib/features/dependent-features/dependent-features-read-model-type.ts

@@ -1,7 +1,7 @@
 import { IDependency } from '../../types';
 
 export interface IDependentFeaturesReadModel {
-    getChildren(parent: string): Promise<string[]>;
+    getChildren(parents: string[]): Promise<string[]>;
     getParents(child: string): Promise<IDependency[]>;
     getParentOptions(child: string): Promise<string[]>;
 }

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<string[]> {
-        const rows = await this.db('dependent_features').where(
+    async getChildren(parents: string[]): Promise<string[]> {
+        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<IDependency[]> {

src/lib/features/dependent-features/dependent-features-service.ts

@@ -29,9 +29,9 @@ export class DependentFeaturesService {
     ): Promise<void> {
         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.',

src/lib/services/feature-toggle-service.ts

@@ -251,6 +251,22 @@ class FeatureToggleService {
         }
     }
 
+    async validateNoChildren(featureNames: string[]): Promise<void> {
+        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<void> {
         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<void> {
+        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<void> {
         await this.validateFeaturesContext(featureNames, projectId);
+        await this.validateNoChildren(featureNames);
 
         const features = await this.featureToggleStore.getAllByNames(
             featureNames,

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')