Skip to content

Commit

Permalink
feat: implement column created_by_user_id in feature_tag (#5695)
Browse files Browse the repository at this point in the history
## About the changes

Adds the new nullable column created_by_user_id to the data used by
feature-tag-store and feature-tag-service. Also updates openapi schemas.
  • Loading branch information
daveleek authored Dec 21, 2023
1 parent e0f8334 commit 4e56d1d
Show file tree
Hide file tree
Showing 11 changed files with 172 additions and 60 deletions.
2 changes: 2 additions & 0 deletions frontend/src/openapi/models/featureTagSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,6 @@ export interface FeatureTagSchema {
* @deprecated
*/
value?: string;
/** The id of the user who created this tag */
createdByUserId?: number;
}
30 changes: 25 additions & 5 deletions src/lib/db/feature-tag-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -18,6 +19,7 @@ interface FeatureTagTable {
feature_name: string;
tag_type: string;
tag_value: string;
created_by_user_id?: number;
}

class FeatureTagStore implements IFeatureTagStore {
Expand Down Expand Up @@ -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,
};
}

Expand All @@ -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,
}));
}

Expand Down Expand Up @@ -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<ITag> {
async tagFeature(
featureName: string,
tag: ITag,
createdByUserId: number,
): Promise<ITag> {
const stopTimer = this.timer('tagFeature');
await this.db<FeatureTagTable>(TABLE)
.insert(this.featureAndTagToRow(featureName, tag))
.insert(this.featureAndTagToRow(featureName, tag, createdByUserId))
.onConflict(COLUMNS)
.merge();
stopTimer();
Expand Down Expand Up @@ -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,
}));
}

Expand All @@ -186,7 +196,9 @@ class FeatureTagStore implements IFeatureTagStore {
stopTimer();
}

async tagFeatures(featureTags: IFeatureTag[]): Promise<IFeatureAndTag[]> {
async tagFeatures(
featureTags: IFeatureTagInsert[],
): Promise<IFeatureAndTag[]> {
if (featureTags.length !== 0) {
const rows = await this.db(TABLE)
.insert(featureTags.map(this.featureTagToRow))
Expand All @@ -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);
Expand Down Expand Up @@ -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,
};
}

Expand All @@ -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,
};
}
}
Expand Down
24 changes: 20 additions & 4 deletions src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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}`,
Expand Down
6 changes: 6 additions & 0 deletions src/lib/openapi/spec/feature-tag-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/lib/schema/feature-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,5 @@ export const featureTagSchema = joi.object().keys({
tagValue: joi.string(),
type: nameType.optional(),
value: joi.string().optional(),
createdByUserId: joi.number().optional(),
});
16 changes: 11 additions & 5 deletions src/lib/services/feature-tag-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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<IFeatureTag, 'createdByUserId'>[] =
featureNames.flatMap((featureName) =>
removedTags.map((addedTag) => ({
featureName,
tagType: addedTag.type,
tagValue: addedTag.value,
})),
);
);

await this.featureTagStore.untagFeatures(removedFeatureTags);

Expand Down
73 changes: 49 additions & 24 deletions src/lib/services/state-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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 = [];
Expand Down
19 changes: 12 additions & 7 deletions src/lib/services/state-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -616,13 +616,18 @@ export default class StateService {
userName: string,
userId: number,
): Promise<void> {
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);
Expand Down
Loading

0 comments on commit 4e56d1d

Please sign in to comment.