feat: bulk update tags (#3274)

frontend/src/interfaces/uiConfig.ts

@@ -48,6 +48,7 @@ export interface IFlags {
     proPlanAutoCharge?: boolean;
     notifications?: boolean;
     loginHistory?: boolean;
+    bulkOperations?: boolean;
     projectScopedSegments?: boolean;
 }
 

src/lib/__snapshots__/create-config.test.ts.snap

@@ -69,6 +69,7 @@ exports[`should create default config 1`] = `
     "flags": {
       "ENABLE_DARK_MODE_SUPPORT": false,
       "anonymiseEventLog": false,
+      "bulkOperations": false,
       "caseInsensitiveInOperators": false,
       "crOnVariants": false,
       "embedProxy": true,
@@ -92,6 +93,7 @@ exports[`should create default config 1`] = `
     "experiments": {
       "ENABLE_DARK_MODE_SUPPORT": false,
       "anonymiseEventLog": false,
+      "bulkOperations": false,
       "caseInsensitiveInOperators": false,
       "crOnVariants": false,
       "embedProxy": true,

src/lib/db/feature-tag-store.ts

@@ -1,10 +1,8 @@
-import { EventEmitter } from 'stream';
 import { Logger, LogProvider } from '../logger';
-import { ITag } from '../types/model';
+import { ITag } from '../types';
+import EventEmitter from 'events';
 import metricsHelper from '../util/metrics-helper';
 import { DB_TIME } from '../metric-events';
-import { UNIQUE_CONSTRAINT_VIOLATION } from '../error/db-error';
-import FeatureHasTagError from '../error/feature-has-tag-error';
 import {
     IFeatureAndTag,
     IFeatureTag,
@@ -123,34 +121,22 @@ class FeatureTagStore implements IFeatureTagStore {
         const stopTimer = this.timer('tagFeature');
         await this.db<FeatureTagTable>(TABLE)
             .insert(this.featureAndTagToRow(featureName, tag))
-            .catch((err) => {
-                if (err.code === UNIQUE_CONSTRAINT_VIOLATION) {
-                    throw new FeatureHasTagError(
-                        `${featureName} already has the tag: [${tag.type}:${tag.value}]`,
-                    );
-                } else {
-                    throw err;
-                }
-            });
+            .onConflict(COLUMNS)
+            .merge();
         stopTimer();
         return tag;
     }
 
-    async tagFeatures(featureNames: string[], tag: ITag): Promise<ITag> {
-        const stopTimer = this.timer('tagFeatures');
-        await this.db<FeatureTagTable>(TABLE)
-            .insert(this.featuresAndTagToRow(featureNames, tag))
-            .catch((err) => {
-                if (err.code === UNIQUE_CONSTRAINT_VIOLATION) {
-                    throw new FeatureHasTagError(
-                        `Some of the features already have the tag: [${tag.type}:${tag.value}]`,
-                    );
-                } else {
-                    throw err;
-                }
-            });
+    async untagFeatures(featureTags: IFeatureTag[]): Promise<void> {
+        const stopTimer = this.timer('untagFeatures');
+        try {
+            await this.db(TABLE)
+                .whereIn(COLUMNS, featureTags.map(this.featureTagArray))
+                .delete();
+        } catch (err) {
+            this.logger.error(err);
+        }
         stopTimer();
-        return tag;
     }
 
     /**
@@ -176,11 +162,9 @@ class FeatureTagStore implements IFeatureTagStore {
         stopTimer();
     }
 
-    async importFeatureTags(
-        featureTags: IFeatureTag[],
-    ): Promise<IFeatureAndTag[]> {
+    async tagFeatures(featureTags: IFeatureTag[]): Promise<IFeatureAndTag[]> {
         const rows = await this.db(TABLE)
-            .insert(featureTags.map(this.importToRow))
+            .insert(featureTags.map(this.featureTagToRow))
             .returning(COLUMNS)
             .onConflict(COLUMNS)
             .ignore();
@@ -222,7 +206,7 @@ class FeatureTagStore implements IFeatureTagStore {
         };
     }
 
-    importToRow({
+    featureTagToRow({
         featureName,
         tagType,
         tagValue,
@@ -234,6 +218,10 @@ class FeatureTagStore implements IFeatureTagStore {
         };
     }
 
+    featureTagArray({ featureName, tagType, tagValue }: IFeatureTag): string[] {
+        return [featureName, tagType, tagValue];
+    }
+
     featureAndTagToRow(
         featureName: string,
         { type, value }: ITag,
@@ -244,17 +232,6 @@ class FeatureTagStore implements IFeatureTagStore {
             tag_value: value,
         };
     }
-
-    featuresAndTagToRow(
-        featureNames: string[],
-        { type, value }: ITag,
-    ): FeatureTagTable[] {
-        return featureNames.map((featureName) => ({
-            feature_name: featureName,
-            tag_type: type,
-            tag_value: value,
-        }));
-    }
 }
 
 module.exports = FeatureTagStore;

src/lib/openapi/spec/tags-bulk-add-schema.test.ts

@@ -4,9 +4,19 @@ import { TagsBulkAddSchema } from './tags-bulk-add-schema';
 test('tagsBulkAddSchema', () => {
     const data: TagsBulkAddSchema = {
         features: ['my-feature'],
-        tag: {
-            type: 'simple',
-            value: 'besttag',
+        tags: {
+            addedTags: [
+                {
+                    type: 'simple',
+                    value: 'besttag',
+                },
+            ],
+            removedTags: [
+                {
+                    type: 'simple2',
+                    value: 'besttag2',
+                },
+            ],
         },
     };
 

src/lib/openapi/spec/tags-bulk-add-schema.ts

@@ -1,11 +1,12 @@
 import { FromSchema } from 'json-schema-to-ts';
+import { updateTagsSchema } from './update-tags-schema';
 import { tagSchema } from './tag-schema';
 
 export const tagsBulkAddSchema = {
     $id: '#/components/schemas/tagsBulkAddSchema',
     type: 'object',
     additionalProperties: false,
-    required: ['features', 'tag'],
+    required: ['features', 'tags'],
     properties: {
         features: {
             type: 'array',
@@ -14,12 +15,13 @@ export const tagsBulkAddSchema = {
                 minLength: 1,
             },
         },
-        tag: {
-            $ref: '#/components/schemas/tagSchema',
+        tags: {
+            $ref: '#/components/schemas/updateTagsSchema',
         },
     },
     components: {
         schemas: {
+            updateTagsSchema,
             tagSchema,
         },
     },

src/lib/openapi/spec/update-tags-schema.test.ts

@@ -0,0 +1,23 @@
+import { validateSchema } from '../validate';
+import { UpdateTagsSchema } from './update-tags-schema';
+
+test('updateTagsSchema', () => {
+    const data: UpdateTagsSchema = {
+        addedTags: [
+            {
+                type: 'simple',
+                value: 'besttag',
+            },
+        ],
+        removedTags: [
+            {
+                type: 'simple2',
+                value: 'besttag2',
+            },
+        ],
+    };
+
+    expect(
+        validateSchema('#/components/schemas/updateTagsSchema', data),
+    ).toBeUndefined();
+});

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

@@ -24,6 +24,8 @@ import {
 import { emptyResponse } from '../../openapi/util/standard-responses';
 import FeatureTagService from 'lib/services/feature-tag-service';
 import { TagsBulkAddSchema } from '../../openapi/spec/tags-bulk-add-schema';
+import NotFoundError from '../../error/notfound-error';
+import { IFlagResolver } from '../../types';
 
 const version = 1;
 
@@ -36,6 +38,8 @@ class TagController extends Controller {
 
     private openApiService: OpenApiService;
 
+    private flagResolver: IFlagResolver;
+
     constructor(
         config: IUnleashConfig,
         {
@@ -52,6 +56,7 @@ class TagController extends Controller {
         this.openApiService = openApiService;
         this.featureTagService = featureTagService;
         this.logger = config.getLogger('/admin-api/tag.js');
+        this.flagResolver = config.flagResolver;
 
         this.route({
             method: 'get',
@@ -85,18 +90,16 @@ class TagController extends Controller {
             ],
         });
         this.route({
-            method: 'post',
+            method: 'put',
             path: '/features',
-            handler: this.addTagToFeatures,
+            handler: this.updateFeaturesTags,
             permission: UPDATE_FEATURE,
             middleware: [
                 openApiService.validPath({
                     tags: ['Tags'],
                     operationId: 'addTagToFeatures',
                     requestBody: createRequestSchema('tagsBulkAddSchema'),
-                    responses: {
-                        201: resourceCreatedResponseSchema('tagSchema'),
-                    },
+                    responses: { 200: emptyResponse },
                 }),
             ],
         });
@@ -207,18 +210,22 @@ class TagController extends Controller {
         res.status(200).end();
     }
 
-    async addTagToFeatures(
+    async updateFeaturesTags(
         req: IAuthRequest<void, void, TagsBulkAddSchema>,
         res: Response<TagSchema>,
     ): Promise<void> {
-        const { features, tag } = req.body;
+        if (!this.flagResolver.isEnabled('bulkOperations')) {
+            throw new NotFoundError('Bulk operations are not enabled');
+        }
+        const { features, tags } = req.body;
         const userName = extractUsername(req);
-        const addedTag = await this.featureTagService.addTags(
+        await this.featureTagService.updateTags(
             features,
-            tag,
+            tags.addedTags,
+            tags.removedTags,
             userName,
         );
-        res.status(201).json(addedTag);
+        res.status(200).end();
     }
 }
 export default TagController;

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

@@ -4,7 +4,10 @@ import { FEATURE_TAGGED, FEATURE_UNTAGGED, TAG_CREATED } from '../types/events';
 import { IUnleashConfig } from '../types/option';
 import { IFeatureToggleStore, IUnleashStores } from '../types/stores';
 import { tagSchema } from './tag-schema';
-import { IFeatureTagStore } from '../types/stores/feature-tag-store';
+import {
+    IFeatureTag,
+    IFeatureTagStore,
+} from '../types/stores/feature-tag-store';
 import { IEventStore } from '../types/stores/event-store';
 import { ITagStore } from '../types/stores/tag-store';
 import { ITag } from '../types/model';
@@ -64,30 +67,61 @@ class FeatureTagService {
         return validatedTag;
     }
 
-    async addTags(
+    async updateTags(
         featureNames: string[],
-        tag: ITag,
+        addedTags: ITag[],
+        removedTags: ITag[],
         userName: string,
-    ): Promise<ITag> {
+    ): Promise<void> {
         const featureToggles = await this.featureToggleStore.getAllByNames(
             featureNames,
         );
-        const validatedTag = await tagSchema.validateAsync(tag);
-        await this.createTagIfNeeded(validatedTag, userName);
-        await this.featureTagStore.tagFeatures(featureNames, validatedTag);
-
         await Promise.all(
-            featureToggles.map((featureToggle) =>
-                this.eventStore.store({
-                    type: FEATURE_TAGGED,
-                    createdBy: userName,
-                    featureName: featureToggle.name,
-                    project: featureToggle.project,
-                    data: validatedTag,
-                }),
-            ),
+            addedTags.map((tag) => this.createTagIfNeeded(tag, userName)),
         );
-        return validatedTag;
+        const createdFeatureTags: IFeatureTag[] = featureNames.flatMap(
+            (featureName) =>
+                addedTags.map((addedTag) => ({
+                    featureName,
+                    tagType: addedTag.type,
+                    tagValue: addedTag.value,
+                })),
+        );
+
+        await this.featureTagStore.tagFeatures(createdFeatureTags);
+
+        const removedFeatureTags: IFeatureTag[] = featureNames.flatMap(
+            (featureName) =>
+                removedTags.map((addedTag) => ({
+                    featureName,
+                    tagType: addedTag.type,
+                    tagValue: addedTag.value,
+                })),
+        );
+
+        await this.featureTagStore.untagFeatures(removedFeatureTags);
+
+        const creationEvents = featureToggles.flatMap((featureToggle) =>
+            addedTags.map((addedTag) => ({
+                type: FEATURE_TAGGED,
+                createdBy: userName,
+                featureName: featureToggle.name,
+                project: featureToggle.project,
+                data: addedTag,
+            })),
+        );
+
+        const removalEvents = featureToggles.flatMap((featureToggle) =>
+            removedTags.map((removedTag) => ({
+                type: FEATURE_UNTAGGED,
+                createdBy: userName,
+                featureName: featureToggle.name,
+                project: featureToggle.project,
+                data: removedTag,
+            })),
+        );
+
+        await this.eventStore.batchStore([...creationEvents, ...removalEvents]);
     }
 
     async createTagIfNeeded(tag: ITag, userName: string): Promise<void> {

src/lib/services/state-service.ts

@@ -600,10 +600,9 @@ export default class StateService {
                 : true,
         );
         if (featureTagsToInsert.length > 0) {
-            const importedFeatureTags =
-                await this.featureTagStore.importFeatureTags(
-                    featureTagsToInsert,
-                );
+            const importedFeatureTags = await this.featureTagStore.tagFeatures(
+                featureTagsToInsert,
+            );
             const importedFeatureTagEvents = importedFeatureTags.map((tag) => ({
                 type: FEATURE_TAG_IMPORT,
                 createdBy: userName,

src/lib/types/experimental.ts

@@ -64,6 +64,10 @@ const flags = {
     ),
     notifications: parseEnvVarBoolean(process.env.NOTIFICATIONS, false),
     loginHistory: parseEnvVarBoolean(process.env.UNLEASH_LOGIN_HISTORY, false),
+    bulkOperations: parseEnvVarBoolean(
+        process.env.UNLEASH_BULK_OPERATIONS,
+        false,
+    ),
     projectScopedSegments: parseEnvVarBoolean(
         process.env.PROJECT_SCOPED_SEGMENTS,
         false,

src/lib/types/stores/feature-tag-store.ts

@@ -15,7 +15,7 @@ export interface IFeatureTagStore extends Store<IFeatureTag, IFeatureTag> {
     getAllTagsForFeature(featureName: string): Promise<ITag[]>;
     getAllByFeatures(features: string[]): Promise<IFeatureTag[]>;
     tagFeature(featureName: string, tag: ITag): Promise<ITag>;
-    tagFeatures(featureNames: string[], tag: ITag): Promise<ITag>;
-    importFeatureTags(featureTags: IFeatureTag[]): Promise<IFeatureAndTag[]>;
+    tagFeatures(featureTags: IFeatureTag[]): Promise<IFeatureAndTag[]>;
     untagFeature(featureName: string, tag: ITag): Promise<void>;
+    untagFeatures(featureTags: IFeatureTag[]): Promise<void>;
 }

src/test/e2e/api/admin/feature.e2e.test.ts

@@ -739,31 +739,6 @@ test('Querying with multiple filters ANDs the filters', async () => {
         });
 });
 
-test('Tagging a feature with a tag it already has should return 409', async () => {
-    const feature1Name = `test.${randomId()}`;
-    await app.request.post('/api/admin/features').send({
-        name: feature1Name,
-        type: 'killswitch',
-        enabled: true,
-        strategies: [{ name: 'default' }],
-    });
-
-    const tag = { value: randomId(), type: 'simple' };
-    await app.request
-        .post(`/api/admin/features/${feature1Name}/tags`)
-        .send(tag)
-        .expect(201);
-    return app.request
-        .post(`/api/admin/features/${feature1Name}/tags`)
-        .send(tag)
-        .expect(409)
-        .expect((res) => {
-            expect(res.body.details[0].message).toBe(
-                `${feature1Name} already has the tag: [${tag.type}:${tag.value}]`,
-            );
-        });
-});
-
 test('marks feature toggle as stale', async () => {
     expect.assertions(1);
     await app.request

src/test/e2e/api/admin/tags.e2e.test.ts

@@ -11,6 +11,7 @@ beforeAll(async () => {
         experimental: {
             flags: {
                 strictSchemaValidation: true,
+                bulkOperations: true,
             },
         },
     });
@@ -112,16 +113,30 @@ test('Can delete a tag', async () => {
 test('Can tag features', async () => {
     const featureName = 'test.feature';
     const featureName2 = 'test.feature2';
-    const tag = {
+    const addedTag = {
         value: 'TeamRed',
         type: 'simple',
     };
+    const removedTag = {
+        value: 'remove_me',
+        type: 'simple',
+    };
     await app.request.post('/api/admin/features').send({
         name: featureName,
         type: 'killswitch',
         enabled: true,
         strategies: [{ name: 'default' }],
     });
+
+    await db.stores.tagStore.createTag(removedTag);
+    await db.stores.featureTagStore.tagFeature(featureName, removedTag);
+
+    const initialTagState = await app.request.get(
+        `/api/admin/features/${featureName}/tags`,
+    );
+
+    expect(initialTagState.body).toMatchObject({ tags: [removedTag] });
+
     await app.request.post('/api/admin/features').send({
         name: featureName2,
         type: 'killswitch',
@@ -129,9 +144,12 @@ test('Can tag features', async () => {
         strategies: [{ name: 'default' }],
     });
 
-    await app.request.post('/api/admin/tags/features').send({
+    await app.request.put('/api/admin/tags/features').send({
         features: [featureName, featureName2],
-        tag: tag,
+        tags: {
+            addedTags: [addedTag],
+            removedTags: [removedTag],
+        },
     });
     const res = await app.request.get(
         `/api/admin/features/${featureName}/tags`,
@@ -141,6 +159,6 @@ test('Can tag features', async () => {
         `/api/admin/features/${featureName2}/tags`,
     );
 
-    expect(res.body).toMatchObject({ tags: [tag] });
-    expect(res2.body).toMatchObject({ tags: [tag] });
+    expect(res.body).toMatchObject({ tags: [addedTag] });
+    expect(res2.body).toMatchObject({ tags: [addedTag] });
 });

src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap

@@ -3652,13 +3652,13 @@ Stats are divided into current and previous **windows**.
             },
             "type": "array",
           },
-          "tag": {
-            "$ref": "#/components/schemas/tagSchema",
+          "tags": {
+            "$ref": "#/components/schemas/updateTagsSchema",
           },
         },
         "required": [
           "features",
-          "tag",
+          "tags",
         ],
         "type": "object",
       },
@@ -7960,7 +7960,7 @@ If the provided project does not exist, the list of events will be empty.",
       },
     },
     "/api/admin/tags/features": {
-      "post": {
+      "put": {
         "operationId": "addTagToFeatures",
         "requestBody": {
           "content": {
@@ -7974,24 +7974,8 @@ If the provided project does not exist, the list of events will be empty.",
           "required": true,
         },
         "responses": {
-          "201": {
-            "content": {
-              "application/json": {
-                "schema": {
-                  "$ref": "#/components/schemas/tagSchema",
-                },
-              },
-            },
-            "description": "The resource was successfully created.",
-            "headers": {
-              "location": {
-                "description": "The location of the newly created resource.",
-                "schema": {
-                  "format": "uri",
-                  "type": "string",
-                },
-              },
-            },
+          "200": {
+            "description": "This response has no body.",
           },
         },
         "tags": [

src/test/e2e/stores/feature-tag-store.e2e.test.ts

@@ -71,16 +71,6 @@ test('should untag feature', async () => {
     expect(featureTags).toHaveLength(0);
 });
 
-test('should throw if feature have tag', async () => {
-    expect.assertions(1);
-    await featureTagStore.tagFeature(featureName, tag);
-    try {
-        await featureTagStore.tagFeature(featureName, tag);
-    } catch (e) {
-        expect(e.message).toContain('already has the tag');
-    }
-});
-
 test('get all feature tags', async () => {
     await featureTagStore.tagFeature(featureName, tag);
     await featureToggleStore.create('default', {
@@ -95,7 +85,7 @@ test('should import feature tags', async () => {
     await featureToggleStore.create('default', {
         name: 'some-other-toggle-import',
     });
-    await featureTagStore.importFeatureTags([
+    await featureTagStore.tagFeatures([
         { featureName, tagType: tag.type, tagValue: tag.value },
         {
             featureName: 'some-other-toggle-import',

src/test/fixtures/fake-feature-tag-store.ts

@@ -57,9 +57,7 @@ export default class FakeFeatureTagStore implements IFeatureTagStore {
         return Promise.resolve();
     }
 
-    async importFeatureTags(
-        featureTags: IFeatureTag[],
-    ): Promise<IFeatureAndTag[]> {
+    async tagFeatures(featureTags: IFeatureTag[]): Promise<IFeatureAndTag[]> {
         return Promise.all(
             featureTags.map(async (fT) => {
                 const saved = await this.tagFeature(fT.featureName, {
@@ -92,14 +90,9 @@ export default class FakeFeatureTagStore implements IFeatureTagStore {
         );
     }
 
-    async tagFeatures(featureNames: string[], tag: ITag): Promise<ITag> {
-        const featureTags = featureNames.map((featureName) => ({
-            featureName,
-            tagType: tag.type,
-            tagValue: tag.value,
-        }));
-        this.featureTags.push(...featureTags);
-        return Promise.resolve(tag);
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    untagFeatures(featureTags: IFeatureTag[]): Promise<void> {
+        throw new Error('Method not implemented.');
     }
 }