From a9805b312be2af2575d9ec3b11e21ccfe24119f2 Mon Sep 17 00:00:00 2001 From: Mateusz Kwasniewski Date: Mon, 25 Sep 2023 15:50:05 +0200 Subject: [PATCH] feat: Delete dependency api (#4824) --- .../feature/Dependencies/AddDependency.tsx | 4 +- .../dependent-features-controller.ts | 65 +++++++++++++++++-- .../dependent-features-service.ts | 15 ++--- .../dependent-features-store-type.ts | 3 +- .../dependent-features-store.ts | 10 ++- .../dependent-features/dependent-features.ts | 10 +++ .../dependent.features.e2e.test.ts | 14 ++++ .../fake-dependent-features-store.ts | 4 ++ 8 files changed, 105 insertions(+), 20 deletions(-) create mode 100644 src/lib/features/dependent-features/dependent-features.ts diff --git a/frontend/src/component/feature/Dependencies/AddDependency.tsx b/frontend/src/component/feature/Dependencies/AddDependency.tsx index be903f089b..af8af91f20 100644 --- a/frontend/src/component/feature/Dependencies/AddDependency.tsx +++ b/frontend/src/component/feature/Dependencies/AddDependency.tsx @@ -2,7 +2,7 @@ import { Box, styled } from '@mui/material'; import { trim } from '../../common/util'; import React, { FC, useState } from 'react'; import Input from '../../common/Input/Input'; -import { CREATE_FEATURE } from '../../providers/AccessProvider/permissions'; +import { UPDATE_FEATURE } from '../../providers/AccessProvider/permissions'; import PermissionButton from '../../common/PermissionButton/PermissionButton'; import { useDependentFeaturesApi } from 'hooks/api/actions/useDependentFeaturesApi/useDependentFeaturesApi'; @@ -45,7 +45,7 @@ export const AddDependency: FC = ({ onChange={e => setParent(trim(e.target.value))} /> { addDependency(featureId, { feature: parent }); diff --git a/src/lib/features/dependent-features/dependent-features-controller.ts b/src/lib/features/dependent-features/dependent-features-controller.ts index 495d6139c3..e7dc8e023f 100644 --- a/src/lib/features/dependent-features/dependent-features-controller.ts +++ b/src/lib/features/dependent-features/dependent-features-controller.ts @@ -2,10 +2,10 @@ import { Response } from 'express'; import Controller from '../../routes/controller'; import { OpenApiService } from '../../services'; import { - CREATE_FEATURE, IFlagResolver, IUnleashConfig, IUnleashServices, + UPDATE_FEATURE, } from '../../types'; import { Logger } from '../../logger'; import { @@ -20,16 +20,24 @@ import { DependentFeaturesService } from './dependent-features-service'; import { TransactionCreator, UnleashTransaction } from '../../db/transaction'; interface FeatureParams { - featureName: string; + child: string; +} + +interface DeleteDependencyParams { + child: string; + parent: string; } const PATH = '/:projectId/features'; -const PATH_FEATURE = `${PATH}/:featureName`; +const PATH_FEATURE = `${PATH}/:child`; const PATH_DEPENDENCIES = `${PATH_FEATURE}/dependencies`; +const PATH_DEPENDENCY = `${PATH_FEATURE}/dependencies/:parent`; type DependentFeaturesServices = Pick< IUnleashServices, - 'transactionalDependentFeaturesService' | 'openApiService' + | 'transactionalDependentFeaturesService' + | 'dependentFeaturesService' + | 'openApiService' >; export default class DependentFeaturesController extends Controller { @@ -37,6 +45,8 @@ export default class DependentFeaturesController extends Controller { db: UnleashTransaction, ) => DependentFeaturesService; + private dependentFeaturesService: DependentFeaturesService; + private readonly startTransaction: TransactionCreator; private openApiService: OpenApiService; @@ -49,6 +59,7 @@ export default class DependentFeaturesController extends Controller { config: IUnleashConfig, { transactionalDependentFeaturesService, + dependentFeaturesService, openApiService, }: DependentFeaturesServices, startTransaction: TransactionCreator, @@ -56,6 +67,7 @@ export default class DependentFeaturesController extends Controller { super(config); this.transactionalDependentFeaturesService = transactionalDependentFeaturesService; + this.dependentFeaturesService = dependentFeaturesService; this.openApiService = openApiService; this.flagResolver = config.flagResolver; this.startTransaction = startTransaction; @@ -67,7 +79,7 @@ export default class DependentFeaturesController extends Controller { method: 'post', path: PATH_DEPENDENCIES, handler: this.addFeatureDependency, - permission: CREATE_FEATURE, + permission: UPDATE_FEATURE, middleware: [ openApiService.validPath({ tags: ['Features'], @@ -85,20 +97,40 @@ export default class DependentFeaturesController extends Controller { }), ], }); + + this.route({ + method: 'delete', + path: PATH_DEPENDENCY, + handler: this.deleteFeatureDependency, + permission: UPDATE_FEATURE, + acceptAnyContentType: true, + middleware: [ + openApiService.validPath({ + tags: ['Features'], + summary: 'Deletes a feature dependency.', + description: 'Remove a dependency to a parent feature.', + operationId: 'deleteFeatureDependency', + responses: { + 200: emptyResponse, + ...getStandardResponses(401, 403, 404), + }, + }), + ], + }); } async addFeatureDependency( req: IAuthRequest, res: Response, ): Promise { - const { featureName } = req.params; + const { child } = req.params; const { variants, enabled, feature } = req.body; if (this.config.flagResolver.isEnabled('dependentFeatures')) { await this.startTransaction(async (tx) => this.transactionalDependentFeaturesService( tx, - ).upsertFeatureDependency(featureName, { + ).upsertFeatureDependency(child, { variants, enabled, feature, @@ -111,4 +143,23 @@ export default class DependentFeaturesController extends Controller { ); } } + + async deleteFeatureDependency( + req: IAuthRequest, + res: Response, + ): Promise { + const { child, parent } = req.params; + + if (this.config.flagResolver.isEnabled('dependentFeatures')) { + await this.dependentFeaturesService.deleteFeatureDependency({ + parent, + child, + }); + res.status(200).end(); + } else { + throw new InvalidOperationError( + 'Dependent features are not enabled', + ); + } + } } diff --git a/src/lib/features/dependent-features/dependent-features-service.ts b/src/lib/features/dependent-features/dependent-features-service.ts index e68787811c..949659e28f 100644 --- a/src/lib/features/dependent-features/dependent-features-service.ts +++ b/src/lib/features/dependent-features/dependent-features-service.ts @@ -1,15 +1,8 @@ import { InvalidOperationError } from '../../error'; import { CreateDependentFeatureSchema } from '../../openapi'; import { IDependentFeaturesStore } from './dependent-features-store-type'; +import { FeatureDependency, FeatureDependencyId } from './dependent-features'; -export type FeatureDependency = - | { - parent: string; - child: string; - enabled: true; - variants?: string[]; - } - | { parent: string; child: string; enabled: false }; export class DependentFeaturesService { private dependentFeaturesStore: IDependentFeaturesStore; @@ -45,4 +38,10 @@ export class DependentFeaturesService { }; await this.dependentFeaturesStore.upsert(featureDependency); } + + async deleteFeatureDependency( + dependency: FeatureDependencyId, + ): Promise { + await this.dependentFeaturesStore.delete(dependency); + } } diff --git a/src/lib/features/dependent-features/dependent-features-store-type.ts b/src/lib/features/dependent-features/dependent-features-store-type.ts index 0a96144fd9..ed13e5cd71 100644 --- a/src/lib/features/dependent-features/dependent-features-store-type.ts +++ b/src/lib/features/dependent-features/dependent-features-store-type.ts @@ -1,6 +1,7 @@ -import { FeatureDependency } from './dependent-features-service'; +import { FeatureDependency, FeatureDependencyId } from './dependent-features'; export interface IDependentFeaturesStore { upsert(featureDependency: FeatureDependency): Promise; getChildren(parent: string): Promise; + delete(dependency: FeatureDependencyId): Promise; } diff --git a/src/lib/features/dependent-features/dependent-features-store.ts b/src/lib/features/dependent-features/dependent-features-store.ts index db59c192d4..b8ee108aee 100644 --- a/src/lib/features/dependent-features/dependent-features-store.ts +++ b/src/lib/features/dependent-features/dependent-features-store.ts @@ -1,11 +1,10 @@ -import { FeatureDependency } from './dependent-features-service'; import { Db } from '../../db/db'; import { IDependentFeaturesStore } from './dependent-features-store-type'; +import { FeatureDependency, FeatureDependencyId } from './dependent-features'; type SerializableFeatureDependency = Omit & { variants?: string; }; - export class DependentFeaturesStore implements IDependentFeaturesStore { private db: Db; @@ -38,4 +37,11 @@ export class DependentFeaturesStore implements IDependentFeaturesStore { return rows.map((row) => row.child); } + + async delete(dependency: FeatureDependencyId): Promise { + await this.db('dependent_features') + .where('parent', dependency.parent) + .andWhere('child', dependency.child) + .del(); + } } diff --git a/src/lib/features/dependent-features/dependent-features.ts b/src/lib/features/dependent-features/dependent-features.ts new file mode 100644 index 0000000000..4f173731f1 --- /dev/null +++ b/src/lib/features/dependent-features/dependent-features.ts @@ -0,0 +1,10 @@ +export type FeatureDependencyId = { parent: string; child: string }; + +export type FeatureDependency = + | { + parent: string; + child: string; + enabled: true; + variants?: string[]; + } + | { parent: string; child: string; enabled: false }; diff --git a/src/lib/features/dependent-features/dependent.features.e2e.test.ts b/src/lib/features/dependent-features/dependent.features.e2e.test.ts index 501d150f3c..e45cd7aa23 100644 --- a/src/lib/features/dependent-features/dependent.features.e2e.test.ts +++ b/src/lib/features/dependent-features/dependent.features.e2e.test.ts @@ -44,6 +44,18 @@ const addFeatureDependency = async ( .expect(expectedCode); }; +const deleteFeatureDependency = async ( + childFeature: string, + parentFeature: string, + expectedCode = 200, +) => { + return app.request + .delete( + `/api/admin/projects/default/features/${childFeature}/dependencies/${parentFeature}`, + ) + .expect(expectedCode); +}; + test('should add feature dependency', async () => { const parent = uuidv4(); const child = uuidv4(); @@ -60,6 +72,8 @@ test('should add feature dependency', async () => { feature: parent, variants: ['variantB'], }); + + await deleteFeatureDependency(child, parent); }); test('should not allow to add a parent dependency to a feature that already has children', async () => { diff --git a/src/lib/features/dependent-features/fake-dependent-features-store.ts b/src/lib/features/dependent-features/fake-dependent-features-store.ts index c338ccc1b3..46fbaac744 100644 --- a/src/lib/features/dependent-features/fake-dependent-features-store.ts +++ b/src/lib/features/dependent-features/fake-dependent-features-store.ts @@ -8,4 +8,8 @@ export class FakeDependentFeaturesStore implements IDependentFeaturesStore { getChildren(): Promise { return Promise.resolve([]); } + + delete(): Promise { + return Promise.resolve(); + } }