From 6520aa1b0cb22d539229f509100ecfc3c2c7a7c1 Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Thu, 3 Feb 2022 11:06:51 +0100 Subject: [PATCH] Feat/impression data (#1310) * feat: add impression data column * fix: add default value to impressionData * fix: allow client api to return impressionData * fix: add tests for impressionData * fix: reset server-dev * fix: add test for adding a toggle with impression data on a different project * fix: update tests --- src/lib/db/feature-strategy-store.ts | 2 + src/lib/db/feature-toggle-client-store.ts | 2 + src/lib/db/feature-toggle-store.ts | 4 + src/lib/schema/feature-schema.test.ts | 10 +++ src/lib/schema/feature-schema.ts | 12 +++ src/lib/types/model.ts | 1 + ...28081242-add-impressiondata-to-features.js | 19 +++++ .../api/admin/project/features.e2e.test.ts | 85 ++++++++++++++++++- src/test/e2e/api/client/feature.e2e.test.ts | 55 ++++++++++++ 9 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 src/migrations/20220128081242-add-impressiondata-to-features.js diff --git a/src/lib/db/feature-strategy-store.ts b/src/lib/db/feature-strategy-store.ts index d2bd270df52..54dd709b0a5 100644 --- a/src/lib/db/feature-strategy-store.ts +++ b/src/lib/db/feature-strategy-store.ts @@ -206,6 +206,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { 'features.project as project', 'features.stale as stale', 'features.variants as variants', + 'features.impression_data as impression_data', 'features.created_at as created_at', 'features.last_seen_at as last_seen_at', 'feature_environments.enabled as enabled', @@ -249,6 +250,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { acc.environments = {}; } acc.name = r.name; + acc.impressionData = r.impression_data; acc.description = r.description; acc.project = r.project; acc.stale = r.stale; diff --git a/src/lib/db/feature-toggle-client-store.ts b/src/lib/db/feature-toggle-client-store.ts index 4435c574a3e..14dfe4843b7 100644 --- a/src/lib/db/feature-toggle-client-store.ts +++ b/src/lib/db/feature-toggle-client-store.ts @@ -74,6 +74,7 @@ export default class FeatureToggleClientStore 'features.type as type', 'features.project as project', 'features.stale as stale', + 'features.impression_data as impression_data', 'features.variants as variants', 'features.created_at as created_at', 'features.last_seen_at as last_seen_at', @@ -137,6 +138,7 @@ export default class FeatureToggleClientStore if (r.strategy_name) { feature.strategies.push(this.getAdminStrategy(r, isAdmin)); } + feature.impressionData = r.impression_data; feature.enabled = !!r.enabled; feature.name = r.name; feature.description = r.description; diff --git a/src/lib/db/feature-toggle-store.ts b/src/lib/db/feature-toggle-store.ts index 4331be1258b..6fc3fd04852 100644 --- a/src/lib/db/feature-toggle-store.ts +++ b/src/lib/db/feature-toggle-store.ts @@ -15,6 +15,7 @@ const FEATURE_COLUMNS = [ 'stale', 'variants', 'created_at', + 'impression_data', 'last_seen_at', ]; @@ -27,6 +28,7 @@ export interface FeaturesTable { project: string; last_seen_at?: Date; created_at?: Date; + impression_data: boolean; } const TABLE = 'features'; @@ -166,6 +168,7 @@ export default class FeatureToggleStore implements IFeatureToggleStore { variants: sortedVariants, createdAt: row.created_at, lastSeenAt: row.last_seen_at, + impressionData: row.impression_data, }; } @@ -188,6 +191,7 @@ export default class FeatureToggleStore implements IFeatureToggleStore { archived: data.archived || false, stale: data.stale, created_at: data.createdAt, + impression_data: data.impressionData, }; if (!row.created_at) { delete row.created_at; diff --git a/src/lib/schema/feature-schema.test.ts b/src/lib/schema/feature-schema.test.ts index c5f7eaaf38e..236fdb047ed 100644 --- a/src/lib/schema/feature-schema.test.ts +++ b/src/lib/schema/feature-schema.test.ts @@ -4,6 +4,7 @@ test('should require URL firendly name', () => { const toggle = { name: 'io`dasd', enabled: false, + impressionData: false, strategies: [{ name: 'default' }], }; @@ -15,6 +16,7 @@ test('should be valid toggle name', () => { const toggle = { name: 'app.name', enabled: false, + impressionData: false, strategies: [{ name: 'default' }], }; @@ -28,6 +30,7 @@ test('should strip extra variant fields', () => { type: 'release', enabled: false, stale: false, + impressionData: false, strategies: [{ name: 'default' }], variants: [ { @@ -49,6 +52,7 @@ test('should allow weightType=fix', () => { type: 'release', project: 'default', enabled: false, + impressionData: false, stale: false, archived: false, strategies: [{ name: 'default' }], @@ -71,6 +75,7 @@ test('should disallow weightType=unknown', () => { name: 'app.name', type: 'release', enabled: false, + impressionData: false, stale: false, archived: false, strategies: [{ name: 'default' }], @@ -95,6 +100,7 @@ test('should be possible to define variant overrides', () => { type: 'release', project: 'some', enabled: false, + impressionData: false, stale: false, archived: false, strategies: [{ name: 'default' }], @@ -125,6 +131,7 @@ test('variant overrides must have corect shape', async () => { name: 'app.name', type: 'release', enabled: false, + impressionData: false, stale: false, strategies: [{ name: 'default' }], variants: [ @@ -154,6 +161,7 @@ test('should keep constraints', () => { type: 'release', project: 'default', enabled: false, + impressionData: false, stale: false, archived: false, strategies: [ @@ -180,6 +188,7 @@ test('should not accept empty constraint values', () => { name: 'app.constraints.empty.value', type: 'release', enabled: false, + impressionData: false, stale: false, strategies: [ { @@ -206,6 +215,7 @@ test('should not accept empty list of constraint values', () => { name: 'app.constraints.empty.value.list', type: 'release', enabled: false, + impressionData: false, stale: false, strategies: [ { diff --git a/src/lib/schema/feature-schema.ts b/src/lib/schema/feature-schema.ts index 21fbb5c3281..395d2c8e970 100644 --- a/src/lib/schema/feature-schema.ts +++ b/src/lib/schema/feature-schema.ts @@ -56,6 +56,12 @@ export const featureMetadataSchema = joi archived: joi.boolean().default(false), type: joi.string().default('release'), description: joi.string().allow('').allow(null).optional(), + impressionData: joi + .boolean() + .allow(true) + .allow(false) + .default(false) + .optional(), createdAt: joi.date().optional().allow(null), }) .options({ allowUnknown: false, stripUnknown: true, abortEarly: false }); @@ -70,6 +76,12 @@ export const featureSchema = joi type: joi.string().default('release'), project: joi.string().default('default'), description: joi.string().allow('').allow(null).optional(), + impressionData: joi + .boolean() + .allow(true) + .allow(false) + .default(false) + .optional(), strategies: joi .array() .min(0) diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index 0f0e3c2060f..57641040a1c 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -38,6 +38,7 @@ export interface FeatureToggleDTO { stale?: boolean; archived?: boolean; createdAt?: Date; + impressionData?: boolean; } export interface FeatureToggle extends FeatureToggleDTO { diff --git a/src/migrations/20220128081242-add-impressiondata-to-features.js b/src/migrations/20220128081242-add-impressiondata-to-features.js new file mode 100644 index 00000000000..a7b079c9ff7 --- /dev/null +++ b/src/migrations/20220128081242-add-impressiondata-to-features.js @@ -0,0 +1,19 @@ +'use strict'; + +exports.up = function (db, cb) { + db.runSql( + ` + ALTER TABLE features ADD COLUMN "impression_data" BOOLEAN DEFAULT FALSE; + `, + cb, + ); +}; + +exports.down = function (db, cb) { + db.runSql( + ` + ALTER TABLE features DROP COLUMN "impression_data"; + `, + cb, + ); +}; diff --git a/src/test/e2e/api/admin/project/features.e2e.test.ts b/src/test/e2e/api/admin/project/features.e2e.test.ts index 449a5d87f7c..d350182bb6b 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/src/test/e2e/api/admin/project/features.e2e.test.ts @@ -512,13 +512,19 @@ test('Should patch feature toggle', async () => { const name = 'new.toggle.patch'; await app.request .post(url) - .send({ name, description: 'some', type: 'release' }) + .send({ + name, + description: 'some', + type: 'release', + impressionData: true, + }) .expect(201); await app.request .patch(`${url}/${name}`) .send([ { op: 'replace', path: '/description', value: 'New desc' }, { op: 'replace', path: '/type', value: 'kill-switch' }, + { op: 'replace', path: '/impressionData', value: false }, ]) .expect(200); @@ -527,6 +533,7 @@ test('Should patch feature toggle', async () => { expect(toggle.name).toBe(name); expect(toggle.description).toBe('New desc'); expect(toggle.type).toBe('kill-switch'); + expect(toggle.impressionData).toBe(false); expect(toggle.archived).toBeFalsy(); const events = await db.stores.eventStore.getAll({ type: FEATURE_METADATA_UPDATED, @@ -1983,3 +1990,79 @@ test('should not update project with PATCH', async () => { }) .expect(200); }); + +test('Can create a feature with impression data', async () => { + await app.request + .post('/api/admin/projects/default/features') + .send({ + name: 'new.toggle.with.impressionData', + impressionData: true, + }) + .expect(201) + .expect((res) => { + expect(res.body.impressionData).toBe(true); + }); +}); + +test('Can create a feature without impression data', async () => { + await app.request + .post('/api/admin/projects/default/features') + .send({ + name: 'new.toggle.without.impressionData', + }) + .expect(201) + .expect((res) => { + expect(res.body.impressionData).toBe(false); + }); +}); + +test('Can update impression data with PUT', async () => { + const toggle = { + name: 'update.toggle.with.impressionData', + impressionData: true, + }; + await app.request + .post('/api/admin/projects/default/features') + .send(toggle) + .expect(201) + .expect((res) => { + expect(res.body.impressionData).toBe(true); + }); + + await app.request + .put(`/api/admin/projects/default/features/${toggle.name}`) + .send({ ...toggle, impressionData: false }) + .expect(200) + .expect((res) => { + expect(res.body.impressionData).toBe(false); + }); +}); + +test('Can create toggle with impression data on different project', async () => { + db.stores.projectStore.create({ + id: 'impression-data', + name: 'ImpressionData', + description: '', + }); + + const toggle = { + name: 'project.impression.data', + impressionData: true, + }; + + await app.request + .post('/api/admin/projects/impression-data/features') + .send(toggle) + .expect(201) + .expect((res) => { + expect(res.body.impressionData).toBe(true); + }); + + await app.request + .put(`/api/admin/projects/impression-data/features/${toggle.name}`) + .send({ ...toggle, impressionData: false }) + .expect(200) + .expect((res) => { + expect(res.body.impressionData).toBe(false); + }); +}); diff --git a/src/test/e2e/api/client/feature.e2e.test.ts b/src/test/e2e/api/client/feature.e2e.test.ts index d1ffe228862..a37178f54f9 100644 --- a/src/test/e2e/api/client/feature.e2e.test.ts +++ b/src/test/e2e/api/client/feature.e2e.test.ts @@ -14,6 +14,7 @@ beforeAll(async () => { { name: 'featureX', description: 'the #1 feature', + impressionData: true, }, 'test', ); @@ -134,6 +135,24 @@ test('gets a feature by name', async () => { .expect(200); }); +test('returns a feature toggles impression data', async () => { + return app.request + .get('/api/client/features/featureX') + .expect('Content-Type', /json/) + .expect((res) => { + expect(res.body.impressionData).toBe(true); + }); +}); + +test('returns a false for impression data when not specified', async () => { + return app.request + .get('/api/client/features/featureZ') + .expect('Content-Type', /json/) + .expect((res) => { + expect(res.body.impressionData).toBe(false); + }); +}); + test('cant get feature that does not exist', async () => { return app.request .get('/api/client/features/myfeature') @@ -255,3 +274,39 @@ test('Can use multiple filters', async () => { expect(res.body.features[0].name).toBe('test.feature'); }); }); + +test('returns a feature toggles impression data for a different project', async () => { + const project = { + id: 'impression-data-client', + name: 'ImpressionData', + description: '', + }; + + db.stores.projectStore.create(project); + + const toggle = { + name: 'project-client.impression.data', + impressionData: true, + }; + + await app.request + .post('/api/admin/projects/impression-data-client/features') + .send(toggle) + .expect(201) + .expect((res) => { + expect(res.body.impressionData).toBe(true); + }); + + return app.request + .get('/api/client/features') + .expect('Content-Type', /json/) + .expect((res) => { + const projectToggle = res.body.features.find( + (resToggle) => resToggle.project === project.id, + ); + + expect(projectToggle.name).toBe(toggle.name); + expect(projectToggle.project).toBe(project.id); + expect(projectToggle.impressionData).toBe(true); + }); +});