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

feat: show warning about dependencies removed on archive (#5104)

This commit is contained in:
Mateusz Kwasniewski 2023-10-20 08:58:03 +02:00 committed by GitHub
parent d212917fd0
commit b890df6e12
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 143 additions and 33 deletions

View File

@ -38,7 +38,10 @@ const setupArchiveValidation = (orphanParents: string[]) => {
testServerRoute(
server,
'/api/admin/projects/projectId/archive/validate',
orphanParents,
{
hasDeletedDependencies: true,
parentsWithChildFeatures: orphanParents,
},
'post',
);
};
@ -60,6 +63,9 @@ test('Add single archive feature change to change request', async () => {
);
expect(screen.getByText('Archive feature toggle')).toBeInTheDocument();
await screen.findByText(
'Archiving features with dependencies will also remove those dependencies.',
);
const button = await screen.findByText('Add change to draft');
button.click();
@ -87,6 +93,9 @@ test('Add multiple archive feature changes to change request', async () => {
);
await screen.findByText('Archive feature toggles');
await screen.findByText(
'Archiving features with dependencies will also remove those dependencies.',
);
const button = await screen.findByText('Add to change request');
button.click();
@ -144,6 +153,11 @@ test('Show error message when multiple parents of orphaned children are archived
await screen.findByText(
'have child features that depend on them and are not part of the archive operation. These parent features can not be archived:',
);
expect(
screen.queryByText(
'Archiving features with dependencies will also remove those dependencies.',
),
).not.toBeInTheDocument();
});
test('Show error message when 1 parent of orphaned children is archived', async () => {
@ -165,4 +179,9 @@ test('Show error message when 1 parent of orphaned children is archived', async
await screen.findByText(
'has child features that depend on it and are not part of the archive operation.',
);
expect(
screen.queryByText(
'Archiving features with dependencies will also remove those dependencies.',
),
).not.toBeInTheDocument();
});

View File

@ -23,6 +23,15 @@ interface IFeatureArchiveDialogProps {
featuresWithUsage?: string[];
}
const RemovedDependenciesAlert = () => {
return (
<Alert severity='warning' sx={{ m: (theme) => theme.spacing(2, 0) }}>
Archiving features with dependencies will also remove those
dependencies.
</Alert>
);
};
const UsageWarning = ({
ids,
projectId,
@ -228,21 +237,25 @@ const useVerifyArchive = (
) => {
const [disableArchive, setDisableArchive] = useState(true);
const [offendingParents, setOffendingParents] = useState<string[]>([]);
const [hasDeletedDependencies, setHasDeletedDependencies] = useState(false);
const { verifyArchiveFeatures } = useProjectApi();
useEffect(() => {
if (isOpen) {
verifyArchiveFeatures(projectId, featureIds)
.then((res) => res.json())
.then((offendingParents) => {
if (offendingParents.length === 0) {
.then(
({ hasDeletedDependencies, parentsWithChildFeatures }) => {
if (parentsWithChildFeatures.length === 0) {
setDisableArchive(false);
setOffendingParents(offendingParents);
setOffendingParents(parentsWithChildFeatures);
} else {
setDisableArchive(true);
setOffendingParents(offendingParents);
setOffendingParents(parentsWithChildFeatures);
}
});
setHasDeletedDependencies(hasDeletedDependencies);
},
);
}
}, [
JSON.stringify(featureIds),
@ -250,9 +263,10 @@ const useVerifyArchive = (
projectId,
setOffendingParents,
setDisableArchive,
setHasDeletedDependencies,
]);
return { disableArchive, offendingParents };
return { disableArchive, offendingParents, hasDeletedDependencies };
};
export const FeatureArchiveDialog: VFC<IFeatureArchiveDialogProps> = ({
@ -285,14 +299,16 @@ export const FeatureArchiveDialog: VFC<IFeatureArchiveDialogProps> = ({
},
});
const { disableArchive, offendingParents } = useVerifyArchive(
featureIds,
projectId,
isOpen,
);
const { disableArchive, offendingParents, hasDeletedDependencies } =
useVerifyArchive(featureIds, projectId, isOpen);
const dependentFeatures = useUiFlag('dependentFeatures');
const removeDependenciesWarning =
dependentFeatures &&
offendingParents.length === 0 &&
hasDeletedDependencies;
return (
<Dialogue
onClick={archiveAction}
@ -312,6 +328,7 @@ export const FeatureArchiveDialog: VFC<IFeatureArchiveDialogProps> = ({
<strong>{featureIds?.length}</strong> feature
toggles?
</p>
<ConditionallyRender
condition={Boolean(
uiConfig.flags.lastSeenByEnvironment &&
@ -336,6 +353,10 @@ export const FeatureArchiveDialog: VFC<IFeatureArchiveDialogProps> = ({
/>
}
/>
<ConditionallyRender
condition={removeDependenciesWarning}
show={<RemovedDependenciesAlert />}
/>
<ConditionallyRender
condition={featureIds?.length <= 5}
show={
@ -368,6 +389,10 @@ export const FeatureArchiveDialog: VFC<IFeatureArchiveDialogProps> = ({
/>
}
/>
<ConditionallyRender
condition={removeDependenciesWarning}
show={<RemovedDependenciesAlert />}
/>
</>
}
/>

View File

@ -9,6 +9,6 @@ export interface IDependentFeaturesReadModel {
getParents(child: string): Promise<IDependency[]>;
getDependencies(children: string[]): Promise<IFeatureDependency[]>;
getParentOptions(child: string): Promise<string[]>;
hasDependencies(feature: string): Promise<boolean>;
haveDependencies(features: string[]): Promise<boolean>;
hasAnyDependencies(): Promise<boolean>;
}

View File

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

View File

@ -20,7 +20,7 @@ export class FakeDependentFeaturesReadModel
return Promise.resolve([]);
}
hasDependencies(): Promise<boolean> {
haveDependencies(): Promise<boolean> {
return Promise.resolve(false);
}

View File

@ -1587,8 +1587,22 @@ class FeatureToggleService {
);
}
async validateArchiveToggles(featureNames: string[]): Promise<string[]> {
return this.dependentFeaturesReadModel.getOrphanParents(featureNames);
async validateArchiveToggles(featureNames: string[]): Promise<{
hasDeletedDependencies: boolean;
parentsWithChildFeatures: string[];
}> {
const hasDeletedDependencies =
await this.dependentFeaturesReadModel.haveDependencies(
featureNames,
);
const parentsWithChildFeatures =
await this.dependentFeaturesReadModel.getOrphanParents(
featureNames,
);
return {
hasDeletedDependencies,
parentsWithChildFeatures,
};
}
async unprotectedArchiveToggles(
@ -1880,7 +1894,9 @@ class FeatureToggleService {
);
}
if (
await this.dependentFeaturesReadModel.hasDependencies(featureName)
await this.dependentFeaturesReadModel.haveDependencies([
featureName,
])
) {
throw new ForbiddenError(
'Changing project not allowed. Feature has dependencies.',

View File

@ -165,6 +165,7 @@ import {
createDependentFeatureSchema,
parentFeatureOptionsSchema,
dependenciesExistSchema,
validateArchiveFeaturesSchema,
} from './spec';
import { IServerOption } from '../types';
import { mapValues, omitKeys } from '../util';
@ -393,6 +394,7 @@ export const schemas: UnleashSchemas = {
parentFeatureOptionsSchema,
featureDependenciesSchema,
dependenciesExistSchema,
validateArchiveFeaturesSchema,
};
// Remove JSONSchema keys that would result in an invalid OpenAPI spec.

View File

@ -165,3 +165,4 @@ export * from './create-dependent-feature-schema';
export * from './parent-feature-options-schema';
export * from './feature-dependencies-schema';
export * from './dependencies-exist-schema';
export * from './validate-archive-features-schema';

View File

@ -0,0 +1,33 @@
import { FromSchema } from 'json-schema-to-ts';
export const validateArchiveFeaturesSchema = {
$id: '#/components/schemas/validateArchiveFeaturesSchema',
type: 'object',
additionalProperties: false,
description: 'Validation details for features archive operation',
required: ['parentsWithChildFeatures', 'hasDeletedDependencies'],
properties: {
parentsWithChildFeatures: {
type: 'array',
items: {
type: 'string',
},
description:
'List of parent features that would orphan child features that are not part of the archive operation',
example: ['my-feature-4', 'my-feature-5', 'my-feature-6'],
},
hasDeletedDependencies: {
type: 'boolean',
description:
'Whether any dependencies will be deleted as part of archive',
example: true,
},
},
components: {
schemas: {},
},
} as const;
export type ValidateArchiveFeaturesSchema = FromSchema<
typeof validateArchiveFeaturesSchema
>;

View File

@ -124,11 +124,13 @@ export default class ProjectArchiveController extends Controller {
tags: ['Features'],
operationId: 'validateArchiveFeatures',
description:
'This endpoint validated if a list of features can be archived. Returns a list of parent features that would orphan some child features. If archive can process then empty list is returned.',
summary: 'Validates if a list of features can be archived',
'This endpoint return info about the archive features impact.',
summary: 'Validates archive features',
requestBody: createRequestSchema('batchFeaturesSchema'),
responses: {
200: createResponseSchema('batchFeaturesSchema'),
200: createResponseSchema(
'validateArchiveFeaturesSchema',
),
...getStandardResponses(400, 401, 403, 415),
},
}),
@ -209,10 +211,10 @@ export default class ProjectArchiveController extends Controller {
): Promise<void> {
const { features } = req.body;
const offendingParents =
const { parentsWithChildFeatures, hasDeletedDependencies } =
await this.featureService.validateArchiveToggles(features);
res.send(offendingParents);
res.send({ parentsWithChildFeatures, hasDeletedDependencies });
}
}

View File

@ -290,8 +290,20 @@ test('Should validate if a list of features with dependencies can be archived',
})
.expect(200);
expect(allChildrenAndParent).toEqual([]);
expect(allChildren).toEqual([]);
expect(onlyParent).toEqual([parent]);
expect(oneChildAndParent).toEqual([parent]);
expect(allChildrenAndParent).toEqual({
hasDeletedDependencies: true,
parentsWithChildFeatures: [],
});
expect(allChildren).toEqual({
hasDeletedDependencies: true,
parentsWithChildFeatures: [],
});
expect(onlyParent).toEqual({
hasDeletedDependencies: true,
parentsWithChildFeatures: [parent],
});
expect(oneChildAndParent).toEqual({
hasDeletedDependencies: true,
parentsWithChildFeatures: [parent],
});
});