chore: expose an endpoint to really delete a toggle (#808)

* chore: expose an endpoint to really delete a toggle

- To provide a way to run end-to-end tests without cluttering
  our demo instance with way too many feature-toggles, making this
  endpoint available will allow end-to-end tests to clean up properly
  after themselves

src/lib/db/feature-toggle-store.js

@@ -222,6 +222,12 @@ class FeatureToggleStore {
         }
     }
 
+    async deleteFeature(name) {
+        await this.db(TABLE)
+            .where({ name, archived: 1 }) // Feature toggle must be archived to allow deletion
+            .del();
+    }
+
     async reviveFeature({ name }) {
         try {
             await this.db(TABLE)

src/lib/routes/admin-api/archive.test.js

@@ -44,6 +44,23 @@ test('should get empty getFeatures via admin', t => {
         });
 });
 
+test('should be allowed to reuse deleted toggle name', async t => {
+    t.plan(0);
+    const { request, archiveStore, base } = getSetup();
+    await archiveStore.createFeature({
+        name: 'ts.really.delete',
+        strategies: [{ name: 'default' }],
+    });
+    await request
+        .delete(`${base}/api/admin/archive/ts.really.delete`)
+        .expect(200);
+    return request
+        .post(`${base}/api/admin/features/validate`)
+        .send({ name: 'ts.really.delete' })
+        .set('Content-Type', 'application/json')
+        .expect(409);
+});
+
 test('should get archived toggles via admin', t => {
     t.plan(1);
     const { request, base, archiveStore } = getSetup();

src/lib/routes/admin-api/archive.ts

@@ -1,15 +1,20 @@
-'use strict';
+import { Request, Response } from 'express';
-
 import { handleErrors } from './util';
 import { IUnleashConfig } from '../../types/option';
 import { IUnleashServices } from '../../types/services';
+import { Logger } from '../../logger';
 
-const Controller = require('../controller');
+import Controller from '../controller';
 
-const extractUser = require('../../extract-user');
+import extractUser from '../../extract-user';
-const { UPDATE_FEATURE } = require('../../types/permissions');
+import { UPDATE_FEATURE, DELETE_FEATURE } from '../../types/permissions';
+import FeatureToggleService from '../../services/feature-toggle-service';
 
 export default class ArchiveController extends Controller {
+    private readonly logger: Logger;
+
+    private featureService: FeatureToggleService;
+
     constructor(
         config: IUnleashConfig,
         {
@@ -21,6 +26,7 @@ export default class ArchiveController extends Controller {
         this.featureService = featureToggleService;
 
         this.get('/features', this.getArchivedFeatures);
+        this.delete('/:featureName', this.deleteFeature, DELETE_FEATURE);
         this.post(
             '/revive/:featureName',
             this.reviveFeatureToggle,
@@ -38,6 +44,19 @@ export default class ArchiveController extends Controller {
         }
     }
 
+    async deleteFeature(
+        req: Request<any, { featureName: string }, any, any>,
+        res: Response,
+    ): Promise<void> {
+        const { featureName } = req.params;
+        try {
+            await this.featureService.deleteFeature(featureName);
+            res.status(200).end();
+        } catch (error) {
+            handleErrors(res, this.logger, error);
+        }
+    }
+
     // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
     async reviveFeatureToggle(req, res): Promise<void> {
         const userName = extractUser(req);

src/lib/routes/admin-api/feature.ts

@@ -47,7 +47,7 @@ class FeatureController extends Controller {
         this.post('/', this.createToggle, CREATE_FEATURE);
         this.get('/:featureName', this.getToggle);
         this.put('/:featureName', this.updateToggle, UPDATE_FEATURE);
-        this.delete('/:featureName', this.deleteToggle, DELETE_FEATURE);
+        this.delete('/:featureName', this.archiveToggle, DELETE_FEATURE);
         this.post('/validate', this.validate);
         this.post('/:featureName/toggle', this.toggle, UPDATE_FEATURE);
         this.post('/:featureName/toggle/on', this.toggleOn, UPDATE_FEATURE);
@@ -233,12 +233,11 @@ class FeatureController extends Controller {
         }
     }
 
-    async deleteToggle(req: Request, res: Response): Promise<void> {
+    async archiveToggle(req: Request, res: Response): Promise<void> {
         const { featureName } = req.params;
-        const userName = extractUser(req);
 
         try {
-            await this.featureService.archiveToggle(featureName, userName);
+            await this.featureService.archiveToggle(featureName);
             res.status(200).end();
         } catch (error) {
             handleErrors(res, this.logger, error);

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

@@ -58,6 +58,10 @@ export default class FeatureToggleService {
         await this.featureToggleStore.addArchivedFeature(feature);
     }
 
+    async deleteFeature(name) {
+        await this.featureToggleStore.deleteFeature(name);
+    }
+
     async getFeature(name) {
         return this.featureToggleStore.getFeature(name);
     }

src/test/e2e/api/admin/feature-archive.e2e.test.js

@@ -56,3 +56,59 @@ test.serial('must set name when reviving toggle', async t => {
     const request = await setupApp(stores);
     return request.post('/api/admin/archive/revive/').expect(404);
 });
+
+test.serial('should be allowed to reuse deleted toggle name', async t => {
+    t.plan(3);
+    const request = await setupApp(stores);
+    await request
+        .post('/api/admin/features')
+        .send({
+            name: 'really.delete.feature',
+            enabled: false,
+            strategies: [{ name: 'default' }],
+        })
+        .set('Content-Type', 'application/json')
+        .expect(201)
+        .expect(res => {
+            t.is(res.body.name, 'really.delete.feature');
+            t.is(res.body.enabled, false);
+            t.truthy(res.body.createdAt);
+        });
+    await request
+        .delete(`/api/admin/features/really.delete.feature`)
+        .expect(200);
+    await request
+        .delete(`/api/admin/archive/really.delete.feature`)
+        .expect(200);
+    return request
+        .post(`/api/admin/features/validate`)
+        .send({ name: 'really.delete.feature' })
+        .set('Content-Type', 'application/json')
+        .expect(200);
+});
+test.serial('Deleting an unarchived toggle should not take effect', async t => {
+    t.plan(3);
+    const request = await setupApp(stores);
+    await request
+        .post('/api/admin/features')
+        .send({
+            name: 'really.delete.feature',
+            enabled: false,
+            strategies: [{ name: 'default' }],
+        })
+        .set('Content-Type', 'application/json')
+        .expect(201)
+        .expect(res => {
+            t.is(res.body.name, 'really.delete.feature');
+            t.is(res.body.enabled, false);
+            t.truthy(res.body.createdAt);
+        });
+    await request
+        .delete(`/api/admin/archive/really.delete.feature`)
+        .expect(200);
+    return request
+        .post(`/api/admin/features/validate`)
+        .send({ name: 'really.delete.feature' })
+        .set('Content-Type', 'application/json')
+        .expect(409); // because it still exists
+});

src/test/fixtures/fake-feature-toggle-store.js

@@ -64,6 +64,13 @@ module.exports = (databaseIsUp = true) => {
             _features.splice(0, _features.length);
             _archive.splice(0, _archive.length);
         },
+        deleteFeature: featureName => {
+            const archivedIdx = _archive.findIndex(f => f.name === featureName);
+            if (archivedIdx > -1) {
+                _archive.splice(archivedIdx, 1);
+            }
+            return Promise.resolve();
+        },
         importFeature: feat => Promise.resolve(_features.push(feat)),
         getFeatures: query => {
             if (!databaseIsUp) {