From 6914bd79084229e8be028944368966cd6955ef62 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Thu, 21 Oct 2021 22:33:50 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20Only=20trigger=20environment=20enabled/d?= =?UTF-8?q?isabled=20events=20if=20different=20f=E2=80=A6=20(#1053)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ivar Conradi Østhus --- src/lib/db/feature-environment-store.ts | 11 +-- src/lib/services/feature-toggle-service-v2.ts | 40 ++++++----- .../types/stores/feature-environment-store.ts | 2 +- .../api/admin/project/features.e2e.test.ts | 7 ++ .../feature-environment-store.e2e.test.ts | 70 +++++++++++++++++++ .../fake-feature-environment-store.ts | 10 ++- 6 files changed, 113 insertions(+), 27 deletions(-) create mode 100644 src/test/e2e/stores/feature-environment-store.e2e.test.ts diff --git a/src/lib/db/feature-environment-store.ts b/src/lib/db/feature-environment-store.ts index 3467db7ca2..dbf27a4aaf 100644 --- a/src/lib/db/feature-environment-store.ts +++ b/src/lib/db/feature-environment-store.ts @@ -196,11 +196,12 @@ export class FeatureEnvironmentStore implements IFeatureEnvironmentStore { environment: string, featureName: string, enabled: boolean, - ): Promise { - await this.db(T.featureEnvs) - .update({ enabled }) - .where({ environment, feature_name: featureName }); - return enabled; + ): Promise { + return this.db(T.featureEnvs).update({ enabled }).where({ + environment, + feature_name: featureName, + enabled: !enabled, + }); } async connectProject( diff --git a/src/lib/services/feature-toggle-service-v2.ts b/src/lib/services/feature-toggle-service-v2.ts index b7abcfb128..f1441a5228 100644 --- a/src/lib/services/feature-toggle-service-v2.ts +++ b/src/lib/services/feature-toggle-service-v2.ts @@ -642,25 +642,29 @@ class FeatureToggleServiceV2 { ); } } - await this.featureEnvironmentStore.setEnvironmentEnabledStatus( - environment, - featureName, - enabled, - ); + const updatedEnvironmentStatus = + await this.featureEnvironmentStore.setEnvironmentEnabledStatus( + environment, + featureName, + enabled, + ); const feature = await this.featureToggleStore.get(featureName); - const tags = await this.featureTagStore.getAllTagsForFeature( - featureName, - ); - await this.eventStore.store({ - type: enabled - ? FEATURE_ENVIRONMENT_ENABLED - : FEATURE_ENVIRONMENT_DISABLED, - createdBy: userName, - data: { name: featureName }, - tags, - project: projectId, - environment, - }); + + if (updatedEnvironmentStatus > 0) { + const tags = await this.featureTagStore.getAllTagsForFeature( + featureName, + ); + await this.eventStore.store({ + type: enabled + ? FEATURE_ENVIRONMENT_ENABLED + : FEATURE_ENVIRONMENT_DISABLED, + createdBy: userName, + data: { name: featureName }, + tags, + project: projectId, + environment, + }); + } return feature; } throw new NotFoundError( diff --git a/src/lib/types/stores/feature-environment-store.ts b/src/lib/types/stores/feature-environment-store.ts index a5ff4c2fef..dc57c9762b 100644 --- a/src/lib/types/stores/feature-environment-store.ts +++ b/src/lib/types/stores/feature-environment-store.ts @@ -20,7 +20,7 @@ export interface IFeatureEnvironmentStore environment: string, featureName: string, enabled: boolean, - ): Promise; + ): Promise; getEnvironmentMetaData( environment: string, featureName: string, 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 835742813a..03ef78a384 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/src/test/e2e/api/admin/project/features.e2e.test.ts @@ -1087,6 +1087,13 @@ test('Disabling environment creates a FEATURE_ENVIRONMENT_DISABLED event', async .send({ name: 'default', constraints: [], properties: {} }) .expect(200); + await app.request + .post( + `/api/admin/projects/default/features/${featureName}/environments/${environment}/on`, + ) + .set('Content-Type', 'application/json') + .expect(200); + await app.request .post( `/api/admin/projects/default/features/${featureName}/environments/${environment}/off`, diff --git a/src/test/e2e/stores/feature-environment-store.e2e.test.ts b/src/test/e2e/stores/feature-environment-store.e2e.test.ts new file mode 100644 index 0000000000..0d21ed71d3 --- /dev/null +++ b/src/test/e2e/stores/feature-environment-store.e2e.test.ts @@ -0,0 +1,70 @@ +import { IUnleashStores } from '../../../lib/types'; +import dbInit from '../helpers/database-init'; +import getLogger from '../../fixtures/no-logger'; +import { IFeatureEnvironmentStore } from '../../../lib/types/stores/feature-environment-store'; +import { IFeatureToggleStore } from '../../../lib/types/stores/feature-toggle-store'; +import { IEnvironmentStore } from '../../../lib/types/stores/environment-store'; + +let db; +let stores: IUnleashStores; +let featureEnvironmentStore: IFeatureEnvironmentStore; +let featureStore: IFeatureToggleStore; +let environmentStore: IEnvironmentStore; + +beforeEach(async () => { + db = await dbInit('feature_environment_store_serial', getLogger); + stores = db.stores; + featureEnvironmentStore = stores.featureEnvironmentStore; + environmentStore = stores.environmentStore; + featureStore = stores.featureToggleStore; +}); + +afterEach(async () => { + await db.destroy(); +}); + +test('Setting enabled to same as existing value returns 0', async () => { + let envName = 'enabled-to-true'; + let featureName = 'enabled-to-true-feature'; + await environmentStore.create({ + name: envName, + enabled: true, + type: 'test', + }); + await featureStore.create('default', { name: featureName }); + await featureEnvironmentStore.connectProject(envName, 'default'); + await featureEnvironmentStore.connectFeatures(envName, 'default'); + const enabledStatus = await featureEnvironmentStore.isEnvironmentEnabled( + featureName, + envName, + ); + const changed = await featureEnvironmentStore.setEnvironmentEnabledStatus( + envName, + featureName, + enabledStatus, + ); + expect(changed).toBe(0); +}); + +test('Setting enabled to not existing value returns 1', async () => { + let envName = 'enabled-toggle'; + let featureName = 'enabled-toggle-feature'; + await environmentStore.create({ + name: envName, + enabled: true, + type: 'test', + }); + await featureStore.create('default', { name: featureName }); + await featureEnvironmentStore.connectProject(envName, 'default'); + await featureEnvironmentStore.connectFeatures(envName, 'default'); + const enabledStatus = await featureEnvironmentStore.isEnvironmentEnabled( + featureName, + envName, + ); + const changed = await featureEnvironmentStore.setEnvironmentEnabledStatus( + envName, + featureName, + !enabledStatus, + ); + expect(changed).toBe(1); +}); diff --git a/src/test/fixtures/fake-feature-environment-store.ts b/src/test/fixtures/fake-feature-environment-store.ts index 1efc17db5d..d14db3aba0 100644 --- a/src/test/fixtures/fake-feature-environment-store.ts +++ b/src/test/fixtures/fake-feature-environment-store.ts @@ -107,10 +107,14 @@ export default class FakeFeatureEnvironmentStore environment: string, featureName: string, enabled: boolean, - ): Promise { + ): Promise { const fE = await this.get({ environment, featureName }); - fE.enabled = enabled; - return enabled; + if (fE.enabled !== enabled) { + fE.enabled = enabled; + return 1; + } else { + return 0; + } } async connectProject(