diff --git a/frontend/src/openapi/models/featureTagSchema.ts b/frontend/src/openapi/models/featureTagSchema.ts index 1782a61ab4ff..bed907cf3d19 100644 --- a/frontend/src/openapi/models/featureTagSchema.ts +++ b/frontend/src/openapi/models/featureTagSchema.ts @@ -24,4 +24,6 @@ export interface FeatureTagSchema { * @deprecated */ value?: string; + /** The id of the user who created this tag */ + createdByUserId?: number; } diff --git a/src/lib/db/feature-tag-store.ts b/src/lib/db/feature-tag-store.ts index 5c07f96a2607..730e53b88e29 100644 --- a/src/lib/db/feature-tag-store.ts +++ b/src/lib/db/feature-tag-store.ts @@ -6,6 +6,7 @@ import { DB_TIME } from '../metric-events'; import { IFeatureAndTag, IFeatureTag, + IFeatureTagInsert, IFeatureTagStore, } from '../types/stores/feature-tag-store'; import { Db } from './db'; @@ -18,6 +19,7 @@ interface FeatureTagTable { feature_name: string; tag_type: string; tag_value: string; + created_by_user_id?: number; } class FeatureTagStore implements IFeatureTagStore { @@ -82,6 +84,7 @@ class FeatureTagStore implements IFeatureTagStore { featureName: row.feature_name, tagType: row.tag_type, tagValue: row.tag_value, + createdByUserId: row.created_by_user_id, }; } @@ -91,6 +94,7 @@ class FeatureTagStore implements IFeatureTagStore { featureName: row.feature_name, tagType: row.tag_type, tagValue: row.tag_value, + createdByUserId: row.created_by_user_id, })); } @@ -138,13 +142,18 @@ class FeatureTagStore implements IFeatureTagStore { featureName: row.feature_name, tagType: row.tag_type, tagValue: row.tag_value, + createdByUserId: row.created_by_user_id, })); } - async tagFeature(featureName: string, tag: ITag): Promise { + async tagFeature( + featureName: string, + tag: ITag, + createdByUserId: number, + ): Promise { const stopTimer = this.timer('tagFeature'); await this.db(TABLE) - .insert(this.featureAndTagToRow(featureName, tag)) + .insert(this.featureAndTagToRow(featureName, tag, createdByUserId)) .onConflict(COLUMNS) .merge(); stopTimer(); @@ -177,6 +186,7 @@ class FeatureTagStore implements IFeatureTagStore { featureName: row.feature_name, tagType: row.tag_type, tagValue: row.tag_value, + createdByUserId: row.created_by_user_id, })); } @@ -186,7 +196,9 @@ class FeatureTagStore implements IFeatureTagStore { stopTimer(); } - async tagFeatures(featureTags: IFeatureTag[]): Promise { + async tagFeatures( + featureTags: IFeatureTagInsert[], + ): Promise { if (featureTags.length !== 0) { const rows = await this.db(TABLE) .insert(featureTags.map(this.featureTagToRow)) @@ -204,7 +216,11 @@ class FeatureTagStore implements IFeatureTagStore { const stopTimer = this.timer('untagFeature'); try { await this.db(TABLE) - .where(this.featureAndTagToRow(featureName, tag)) + .where({ + feature_name: featureName, + tag_type: tag.type, + tag_value: tag.value, + }) .delete(); } catch (err) { this.logger.error(err); @@ -233,11 +249,13 @@ class FeatureTagStore implements IFeatureTagStore { featureName, tagType, tagValue, - }: IFeatureTag): FeatureTagTable { + createdByUserId, + }: IFeatureTagInsert): FeatureTagTable { return { feature_name: featureName, tag_type: tagType, tag_value: tagValue, + created_by_user_id: createdByUserId, }; } @@ -248,11 +266,13 @@ class FeatureTagStore implements IFeatureTagStore { featureAndTagToRow( featureName: string, { type, value }: ITag, + createdByUserId: number, ): FeatureTagTable { return { feature_name: featureName, tag_type: type, tag_value: value, + created_by_user_id: createdByUserId, }; } } diff --git a/src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts b/src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts index 1e25341b2599..1ab6430c914c 100644 --- a/src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts +++ b/src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts @@ -34,6 +34,7 @@ let app: IUnleashTest; let db: ITestDb; const sortOrderFirst = 0; const sortOrderSecond = 10; +const TESTUSERID = 3333; const createSegment = async (segmentName: string) => { const segment = await app.services.segmentService.create( @@ -2991,7 +2992,7 @@ test('Can filter based on tags', async () => { await db.stores.featureToggleStore.create('default', { name: 'not-tagged', }); - await db.stores.featureTagStore.tagFeature('to-be-tagged', tag); + await db.stores.featureTagStore.tagFeature('to-be-tagged', tag, TESTUSERID); await app.request .get('/api/admin/projects/default/features?tag=simple:hello-tags') .expect((res) => { @@ -3028,10 +3029,12 @@ test('Can query for features with namePrefix and tags', async () => { await db.stores.featureTagStore.tagFeature( 'to-be-tagged-nameprefix-and-tags', tag, + TESTUSERID, ); await db.stores.featureTagStore.tagFeature( 'tagged-but-not-hit-nameprefix-and-tags', tag, + TESTUSERID, ); await app.request .get( @@ -3065,13 +3068,26 @@ test('Can query for two tags at the same time. Tags are ORed together', async () name: 'tagged-with-both-tags', }, ); - await db.stores.featureTagStore.tagFeature(taggedWithFirst.name, tag); + await db.stores.featureTagStore.tagFeature( + taggedWithFirst.name, + tag, + TESTUSERID, + ); await db.stores.featureTagStore.tagFeature( taggedWithSecond.name, secondTag, + TESTUSERID, + ); + await db.stores.featureTagStore.tagFeature( + taggedWithBoth.name, + tag, + TESTUSERID, + ); + await db.stores.featureTagStore.tagFeature( + taggedWithBoth.name, + secondTag, + TESTUSERID, ); - await db.stores.featureTagStore.tagFeature(taggedWithBoth.name, tag); - await db.stores.featureTagStore.tagFeature(taggedWithBoth.name, secondTag); await app.request .get( `/api/admin/projects/default/features?tag=${tag.type}:${tag.value}&tag=${secondTag.type}:${secondTag.value}`, diff --git a/src/lib/openapi/spec/feature-tag-schema.ts b/src/lib/openapi/spec/feature-tag-schema.ts index 3ff8be6fb1cf..5be4ecfe5e43 100644 --- a/src/lib/openapi/spec/feature-tag-schema.ts +++ b/src/lib/openapi/spec/feature-tag-schema.ts @@ -35,6 +35,12 @@ export const featureTagSchema = { description: 'The value of the tag. This property is deprecated and will be removed in a future version of Unleash. Superseded by the `tagValue` property.', }, + createdByUserId: { + type: 'number', + nullable: true, + example: 1, + description: 'The id of the user who created this tag', + }, }, components: {}, } as const; diff --git a/src/lib/schema/feature-schema.ts b/src/lib/schema/feature-schema.ts index 29dc66755a0c..d64694afa1da 100644 --- a/src/lib/schema/feature-schema.ts +++ b/src/lib/schema/feature-schema.ts @@ -159,4 +159,5 @@ export const featureTagSchema = joi.object().keys({ tagValue: joi.string(), type: nameType.optional(), value: joi.string().optional(), + createdByUserId: joi.number().optional(), }); diff --git a/src/lib/services/feature-tag-service.ts b/src/lib/services/feature-tag-service.ts index 0205729563cb..8206d0dc9477 100644 --- a/src/lib/services/feature-tag-service.ts +++ b/src/lib/services/feature-tag-service.ts @@ -6,6 +6,7 @@ import { IFeatureToggleStore, IUnleashStores } from '../types/stores'; import { tagSchema } from './tag-schema'; import { IFeatureTag, + IFeatureTagInsert, IFeatureTagStore, } from '../types/stores/feature-tag-store'; import { ITagStore } from '../types/stores/tag-store'; @@ -61,7 +62,11 @@ class FeatureTagService { const featureToggle = await this.featureToggleStore.get(featureName); const validatedTag = await tagSchema.validateAsync(tag); await this.createTagIfNeeded(validatedTag, userName, addedByUserId); - await this.featureTagStore.tagFeature(featureName, validatedTag); + await this.featureTagStore.tagFeature( + featureName, + validatedTag, + addedByUserId, + ); await this.eventService.storeEvent({ type: FEATURE_TAGGED, @@ -88,25 +93,26 @@ class FeatureTagService { this.createTagIfNeeded(tag, userName, updatedByUserId), ), ); - const createdFeatureTags: IFeatureTag[] = featureNames.flatMap( + const createdFeatureTags: IFeatureTagInsert[] = featureNames.flatMap( (featureName) => addedTags.map((addedTag) => ({ featureName, tagType: addedTag.type, tagValue: addedTag.value, + createdByUserId: updatedByUserId, })), ); await this.featureTagStore.tagFeatures(createdFeatureTags); - const removedFeatureTags: IFeatureTag[] = featureNames.flatMap( - (featureName) => + const removedFeatureTags: Omit[] = + featureNames.flatMap((featureName) => removedTags.map((addedTag) => ({ featureName, tagType: addedTag.type, tagValue: addedTag.value, })), - ); + ); await this.featureTagStore.untagFeatures(removedFeatureTags); diff --git a/src/lib/services/state-service.test.ts b/src/lib/services/state-service.test.ts index c068d396eabe..76c3b3ce623f 100644 --- a/src/lib/services/state-service.test.ts +++ b/src/lib/services/state-service.test.ts @@ -16,6 +16,7 @@ import variantsExportV3 from '../../test/examples/variantsexport_v3.json'; import EventService from './event-service'; import { SYSTEM_USER_ID } from '../types'; const oldExportExample = require('./state-service-export-v1.json'); +const TESTUSERID = 3333; function getSetup() { const stores = createStores(); @@ -398,10 +399,14 @@ test('Should not import an existing tag', async () => { }; await stores.tagTypeStore.createTagType(data.tagTypes[0]); await stores.tagStore.createTag(data.tags[0]); - await stores.featureTagStore.tagFeature(data.featureTags[0].featureName, { - type: data.featureTags[0].tagType, - value: data.featureTags[0].tagValue, - }); + await stores.featureTagStore.tagFeature( + data.featureTags[0].featureName, + { + type: data.featureTags[0].tagType, + value: data.featureTags[0].tagValue, + }, + TESTUSERID, + ); await stateService.import({ data, userId: SYSTEM_USER_ID, @@ -466,10 +471,14 @@ test('should export tag, tagtypes but not feature tags if the feature is not exp }; await stores.tagTypeStore.createTagType(data.tagTypes[0]); await stores.tagStore.createTag(data.tags[0]); - await stores.featureTagStore.tagFeature(data.featureTags[0].featureName, { - type: data.featureTags[0].tagType, - value: data.featureTags[0].tagValue, - }); + await stores.featureTagStore.tagFeature( + data.featureTags[0].featureName, + { + type: data.featureTags[0].tagType, + value: data.featureTags[0].tagValue, + }, + TESTUSERID, + ); const exported = await stateService.export({ includeFeatureToggles: false, @@ -504,10 +513,14 @@ test('should export tag, tagtypes, featureTags and features', async () => { }; await stores.tagTypeStore.createTagType(data.tagTypes[0]); await stores.tagStore.createTag(data.tags[0]); - await stores.featureTagStore.tagFeature(data.featureTags[0].featureName, { - type: data.featureTags[0].tagType, - value: data.featureTags[0].tagValue, - }); + await stores.featureTagStore.tagFeature( + data.featureTags[0].featureName, + { + type: data.featureTags[0].tagType, + value: data.featureTags[0].tagValue, + }, + TESTUSERID, + ); const exported = await stateService.export({ includeFeatureToggles: true, @@ -667,10 +680,14 @@ test('exporting to new format works', async () => { parameters: {}, constraints: [], }); - await stores.featureTagStore.tagFeature('Some-feature', { - type: 'simple', - value: 'Test', - }); + await stores.featureTagStore.tagFeature( + 'Some-feature', + { + type: 'simple', + value: 'Test', + }, + TESTUSERID, + ); const exported = await stateService.export({}); expect(exported.featureStrategies).toHaveLength(1); }); @@ -725,10 +742,14 @@ test('featureStrategies can keep existing', async () => { parameters: {}, constraints: [], }); - await stores.featureTagStore.tagFeature('Some-feature', { - type: 'simple', - value: 'Test', - }); + await stores.featureTagStore.tagFeature( + 'Some-feature', + { + type: 'simple', + value: 'Test', + }, + TESTUSERID, + ); const exported = await stateService.export({}); await stateService.import({ @@ -776,10 +797,14 @@ test('featureStrategies should not keep existing if dropBeforeImport', async () parameters: {}, constraints: [], }); - await stores.featureTagStore.tagFeature('Some-feature', { - type: 'simple', - value: 'Test', - }); + await stores.featureTagStore.tagFeature( + 'Some-feature', + { + type: 'simple', + value: 'Test', + }, + TESTUSERID, + ); const exported = await stateService.export({}); exported.featureStrategies = []; diff --git a/src/lib/services/state-service.ts b/src/lib/services/state-service.ts index fa562f2a62d1..fed2cfcc4781 100644 --- a/src/lib/services/state-service.ts +++ b/src/lib/services/state-service.ts @@ -616,13 +616,18 @@ export default class StateService { userName: string, userId: number, ): Promise { - const featureTagsToInsert = featureTags.filter((tag) => - keepExisting - ? !oldFeatureTags.some((old) => - this.compareFeatureTags(old, tag), - ) - : true, - ); + const featureTagsToInsert = featureTags + .filter((tag) => + keepExisting + ? !oldFeatureTags.some((old) => + this.compareFeatureTags(old, tag), + ) + : true, + ) + .map((tag) => ({ + createdByUserId: userId, + ...tag, + })); if (featureTagsToInsert.length > 0) { const importedFeatureTags = await this.featureTagStore.tagFeatures(featureTagsToInsert); diff --git a/src/lib/types/stores/feature-tag-store.ts b/src/lib/types/stores/feature-tag-store.ts index b64e9ca01a80..74142bebf162 100644 --- a/src/lib/types/stores/feature-tag-store.ts +++ b/src/lib/types/stores/feature-tag-store.ts @@ -5,6 +5,12 @@ export interface IFeatureTag { featureName: string; tagType: string; tagValue: string; + createdByUserId?: number; +} + +export interface IFeatureTagInsert + extends Omit { + createdByUserId: number; } export interface IFeatureAndTag { @@ -15,8 +21,14 @@ export interface IFeatureTagStore extends Store { getAllTagsForFeature(featureName: string): Promise; getAllFeaturesForTag(tagValue: string): Promise; getAllByFeatures(features: string[]): Promise; - tagFeature(featureName: string, tag: ITag): Promise; - tagFeatures(featureTags: IFeatureTag[]): Promise; + tagFeature( + featureName: string, + tag: ITag, + createdByUserId: number, + ): Promise; + tagFeatures(featureTags: IFeatureTagInsert[]): Promise; untagFeature(featureName: string, tag: ITag): Promise; - untagFeatures(featureTags: IFeatureTag[]): Promise; + untagFeatures( + featureTags: Omit[], + ): Promise; } diff --git a/src/test/e2e/stores/feature-tag-store.e2e.test.ts b/src/test/e2e/stores/feature-tag-store.e2e.test.ts index 0728b9b8ff9b..018e2b5a70c3 100644 --- a/src/test/e2e/stores/feature-tag-store.e2e.test.ts +++ b/src/test/e2e/stores/feature-tag-store.e2e.test.ts @@ -11,6 +11,7 @@ let featureToggleStore: IFeatureToggleStore; const featureName = 'test-tag'; const tag = { type: 'simple', value: 'test' }; +const TESTUSERID = 3333; beforeAll(async () => { db = await dbInit('feature_tag_store_serial', getLogger); @@ -31,12 +32,13 @@ afterEach(async () => { }); test('should tag feature', async () => { - await featureTagStore.tagFeature(featureName, tag); + await featureTagStore.tagFeature(featureName, tag, TESTUSERID); const featureTags = await featureTagStore.getAllTagsForFeature(featureName); const featureTag = await featureTagStore.get({ featureName, tagType: tag.type, tagValue: tag.value, + createdByUserId: TESTUSERID, }); expect(featureTags).toHaveLength(1); expect(featureTags[0]).toStrictEqual(tag); @@ -45,39 +47,41 @@ test('should tag feature', async () => { }); test('feature tag exists', async () => { - await featureTagStore.tagFeature(featureName, tag); + await featureTagStore.tagFeature(featureName, tag, TESTUSERID); const exists = await featureTagStore.exists({ featureName, tagType: tag.type, tagValue: tag.value, + createdByUserId: TESTUSERID, }); expect(exists).toBe(true); }); test('should delete feature tag', async () => { - await featureTagStore.tagFeature(featureName, tag); + await featureTagStore.tagFeature(featureName, tag, TESTUSERID); await featureTagStore.delete({ featureName, tagType: tag.type, tagValue: tag.value, + createdByUserId: TESTUSERID, }); const featureTags = await featureTagStore.getAllTagsForFeature(featureName); expect(featureTags).toHaveLength(0); }); test('should untag feature', async () => { - await featureTagStore.tagFeature(featureName, tag); + await featureTagStore.tagFeature(featureName, tag, TESTUSERID); await featureTagStore.untagFeature(featureName, tag); const featureTags = await featureTagStore.getAllTagsForFeature(featureName); expect(featureTags).toHaveLength(0); }); test('get all feature tags', async () => { - await featureTagStore.tagFeature(featureName, tag); + await featureTagStore.tagFeature(featureName, tag, TESTUSERID); await featureToggleStore.create('default', { name: 'some-other-toggle', }); - await featureTagStore.tagFeature('some-other-toggle', tag); + await featureTagStore.tagFeature('some-other-toggle', tag, TESTUSERID); const all = await featureTagStore.getAll(); expect(all).toHaveLength(2); }); @@ -87,11 +91,17 @@ test('should import feature tags', async () => { name: 'some-other-toggle-import', }); await featureTagStore.tagFeatures([ - { featureName, tagType: tag.type, tagValue: tag.value }, + { + featureName, + tagType: tag.type, + tagValue: tag.value, + createdByUserId: TESTUSERID, + }, { featureName: 'some-other-toggle-import', tagType: tag.type, tagValue: tag.value, + createdByUserId: TESTUSERID, }, ]); diff --git a/src/test/fixtures/fake-feature-tag-store.ts b/src/test/fixtures/fake-feature-tag-store.ts index 34e1227c6c61..f4054b824ae1 100644 --- a/src/test/fixtures/fake-feature-tag-store.ts +++ b/src/test/fixtures/fake-feature-tag-store.ts @@ -46,11 +46,16 @@ export default class FakeFeatureTagStore implements IFeatureTagStore { return this.featureTags; } - async tagFeature(featureName: string, tag: ITag): Promise { + async tagFeature( + featureName: string, + tag: ITag, + createdByUserId: number, + ): Promise { this.featureTags.push({ featureName, tagType: tag.type, tagValue: tag.value, + createdByUserId, }); return Promise.resolve(tag); } @@ -67,10 +72,14 @@ export default class FakeFeatureTagStore implements IFeatureTagStore { async tagFeatures(featureTags: IFeatureTag[]): Promise { return Promise.all( featureTags.map(async (fT) => { - const saved = await this.tagFeature(fT.featureName, { - value: fT.tagValue, - type: fT.tagType, - }); + const saved = await this.tagFeature( + fT.featureName, + { + value: fT.tagValue, + type: fT.tagType, + }, + fT.createdByUserId, + ); return { featureName: fT.featureName, tag: saved,