mirror of
https://github.com/Unleash/unleash.git
synced 2025-01-20 00:08:02 +01:00
feat: prevent delete and archive on parent feature (#4913)
This commit is contained in:
parent
296cc9a9f2
commit
88305a6388
@ -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>
|
||||
);
|
||||
};
|
@ -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}
|
||||
|
@ -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[]>;
|
||||
}
|
||||
|
@ -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[]> {
|
||||
|
@ -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.',
|
||||
|
@ -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,
|
||||
|
@ -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')
|
||||
|
Loading…
Reference in New Issue
Block a user