From ee660c8eef9192536bdfe1ac303bfc368be25245 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= <ivarconr@gmail.com>
Date: Fri, 1 Oct 2021 12:27:05 +0200
Subject: [PATCH] fix: client api should return feature toggles for disabled
 environments (#995)

* fix: client api should return feature toggles for disabled environments

* fix: add test

* lint
---
 src/lib/db/feature-toggle-client-store.ts     | 62 ++++++++--------
 .../client/feature.env.disabled.e2e.test.ts   | 70 +++++++++++++++++++
 2 files changed, 104 insertions(+), 28 deletions(-)
 create mode 100644 src/test/e2e/api/client/feature.env.disabled.e2e.test.ts

diff --git a/src/lib/db/feature-toggle-client-store.ts b/src/lib/db/feature-toggle-client-store.ts
index fcb5e48740..b853f45250 100644
--- a/src/lib/db/feature-toggle-client-store.ts
+++ b/src/lib/db/feature-toggle-client-store.ts
@@ -45,16 +45,20 @@ export default class FeatureToggleClientStore
         r: any,
         includeId: boolean = true,
     ): IStrategyConfig {
-        const strategy = {
-            name: r.strategy_name,
-            constraints: r.constraints || [],
-            parameters: r.parameters,
-            id: r.strategy_id,
-        };
-        if (!includeId) {
-            delete strategy.id;
+        if (includeId) {
+            return {
+                name: r.strategy_name,
+                constraints: r.constraints || [],
+                parameters: r.parameters,
+                id: r.strategy_id,
+            };
+        } else {
+            return {
+                name: r.strategy_name,
+                constraints: r.constraints || [],
+                parameters: r.parameters,
+            };
         }
-        return strategy;
     }
 
     private async getAll(
@@ -74,28 +78,29 @@ export default class FeatureToggleClientStore
                 'features.variants as variants',
                 'features.created_at as created_at',
                 'features.last_seen_at as last_seen_at',
-                'feature_environments.enabled as enabled',
-                'feature_environments.environment as environment',
-                'feature_strategies.id as strategy_id',
-                'feature_strategies.strategy_name as strategy_name',
-                'feature_strategies.parameters as parameters',
-                'feature_strategies.constraints as constraints',
+                'fe.enabled as enabled',
+                'fe.environment as environment',
+                'fs.id as strategy_id',
+                'fs.strategy_name as strategy_name',
+                'fs.parameters as parameters',
+                'fs.constraints as constraints',
             )
             .fullOuterJoin(
-                'feature_environments',
-                'feature_environments.feature_name',
+                this.db('feature_strategies')
+                    .select('*')
+                    .where({ environment })
+                    .as('fs'),
+                'fs.feature_name',
+                'features.name',
+            )
+            .fullOuterJoin(
+                this.db('feature_environments')
+                    .select('feature_name', 'enabled', 'environment')
+                    .where({ environment })
+                    .as('fe'),
+                'fe.feature_name',
                 'features.name',
             )
-            .fullOuterJoin('feature_strategies', function () {
-                this.on(
-                    'feature_strategies.feature_name',
-                    'features.name',
-                ).andOn(
-                    'feature_strategies.environment',
-                    'feature_environments.environment',
-                );
-            })
-            .where('feature_environments.environment', environment)
             .where({ archived });
 
         if (featureQuery) {
@@ -117,6 +122,7 @@ export default class FeatureToggleClientStore
                 );
             }
         }
+
         const rows = await query;
         stopTimer();
         const featureToggles = rows.reduce((acc, r) => {
@@ -132,7 +138,7 @@ export default class FeatureToggleClientStore
             if (r.strategy_name) {
                 feature.strategies.push(this.getAdminStrategy(r, isAdmin));
             }
-            feature.enabled = r.enabled;
+            feature.enabled = !!r.enabled;
             feature.name = r.name;
             feature.description = r.description;
             feature.project = r.project;
diff --git a/src/test/e2e/api/client/feature.env.disabled.e2e.test.ts b/src/test/e2e/api/client/feature.env.disabled.e2e.test.ts
new file mode 100644
index 0000000000..b28d49bd1b
--- /dev/null
+++ b/src/test/e2e/api/client/feature.env.disabled.e2e.test.ts
@@ -0,0 +1,70 @@
+import { IUnleashTest, setupApp } from '../../helpers/test-helper';
+import dbInit, { ITestDb } from '../../helpers/database-init';
+import getLogger from '../../../fixtures/no-logger';
+
+let app: IUnleashTest;
+let db: ITestDb;
+
+const featureName = 'feature.default.1';
+
+beforeAll(async () => {
+    db = await dbInit('feature_api_client', getLogger);
+    app = await setupApp(db.stores);
+
+    await app.services.featureToggleServiceV2.createFeatureToggle(
+        'default',
+        {
+            name: featureName,
+            description: 'the #1 feature',
+        },
+        'test',
+    );
+
+    await app.services.featureToggleServiceV2.createStrategy(
+        { name: 'default', constraints: [], parameters: {} },
+        'default',
+        featureName,
+        'test',
+    );
+});
+
+afterAll(async () => {
+    await app.destroy();
+    await db.destroy();
+});
+
+test('returns feature toggle for default env', async () => {
+    await app.services.featureToggleServiceV2.updateEnabled(
+        'default',
+        'feature.default.1',
+        'default',
+        true,
+        'test',
+    );
+    await app.request
+        .get('/api/client/features')
+        .expect('Content-Type', /json/)
+        .expect(200)
+        .expect((res) => {
+            expect(res.body.features).toHaveLength(1);
+            expect(res.body.features[0].enabled).toBe(true);
+            expect(res.body.features[0].strategies).toHaveLength(1);
+        });
+});
+
+test('returns feature toggle for default env even if it is removed from project', async () => {
+    await app.services.environmentService.removeEnvironmentFromProject(
+        'default',
+        'default',
+    );
+
+    await app.request
+        .get('/api/client/features')
+        .expect('Content-Type', /json/)
+        .expect(200)
+        .expect((res) => {
+            expect(res.body.features).toHaveLength(1);
+            expect(res.body.features[0].enabled).toBe(false);
+            expect(res.body.features[0].strategies).toHaveLength(1);
+        });
+});