task: Disables feature_environments without strategies (#1003)

src/lib/db/feature-environment-store.ts

@@ -10,7 +10,10 @@ import { DB_TIME } from '../metric-events';
 import { IFeatureEnvironment } from '../types/model';
 import NotFoundError from '../error/notfound-error';
 
-const T = { featureEnvs: 'feature_environments' };
+const T = {
+    featureEnvs: 'feature_environments',
+    featureStrategies: 'feature_strategies',
+};
 
 interface IFeatureEnvironmentRow {
     environment: string;
@@ -92,6 +95,22 @@ export class FeatureEnvironmentStore implements IFeatureEnvironmentStore {
         }));
     }
 
+    async disableEnvironmentIfNoStrategies(
+        featureName: string,
+        environment: string,
+    ): Promise<void> {
+        const result = await this.db.raw(
+            `SELECT EXISTS (SELECT 1 FROM ${T.featureStrategies} WHERE feature_name = ? AND environment = ?) AS enabled`,
+            [featureName, environment],
+        );
+        const { enabled } = result.rows[0];
+        if (!enabled) {
+            await this.db(T.featureEnvs)
+                .update({ enabled: false })
+                .where({ feature_name: featureName, environment });
+        }
+    }
+
     async addEnvironmentToFeature(
         featureName: string,
         environment: string,

src/lib/routes/admin-api/project/features.ts

@@ -300,12 +300,13 @@ export default class ProjectFeaturesController extends Controller {
         res: Response,
     ): Promise<void> {
         this.logger.info('Deleting strategy');
-        const { environment, projectId } = req.params;
+        const { environment, projectId, featureName } = req.params;
         const userName = extractUsername(req);
         const { strategyId } = req.params;
         this.logger.info(strategyId);
         const strategy = await this.featureService.deleteStrategy(
             strategyId,
+            featureName,
             userName,
             projectId,
             environment,

src/lib/services/feature-toggle-service-v2.ts

@@ -226,6 +226,7 @@ class FeatureToggleServiceV2 {
      */
     async deleteStrategy(
         id: string,
+        featureName: string,
         userName: string,
         project: string = 'default',
         environment: string = DEFAULT_ENV,
@@ -240,6 +241,11 @@ class FeatureToggleServiceV2 {
                 id,
             },
         });
+        // If there are no strategies left for environment disable it
+        await this.featureEnvironmentStore.disableEnvironmentIfNoStrategies(
+            featureName,
+            environment,
+        );
     }
 
     async getStrategiesForEnvironment(

src/lib/types/stores/feature-environment-store.ts

@@ -34,7 +34,10 @@ export interface IFeatureEnvironmentStore
         environment: string,
         enabled: boolean,
     ): Promise<void>;
-
+    disableEnvironmentIfNoStrategies(
+        featureName: string,
+        environment: string,
+    ): Promise<void>;
     disconnectFeatures(environment: string, project: string): Promise<void>;
     connectFeatures(environment: string, projectId: string): Promise<void>;
 

src/test/e2e/api/admin/project/feature.strategy.e2e.test.ts

@@ -1108,3 +1108,140 @@ test('Feature strategies list should respect strategy sortorders for each enviro
     expect(strategies[1].sortOrder).toBe(sortOrderSecond);
     expect(strategies[2].sortOrder).toBe(sortOrderDefault);
 });
+
+test('Deleting last strategy for feature environment should disable that environment', async () => {
+    const envName = 'last_strategy_delete_env';
+    const featureName = 'last_strategy_delete_feature';
+    // Create environment
+    await db.stores.environmentStore.create({
+        name: envName,
+        type: 'test',
+    });
+    // Connect environment to project
+    await app.request
+        .post('/api/admin/projects/default/environments')
+        .send({
+            environment: envName,
+        })
+        .expect(200);
+    await app.request
+        .post('/api/admin/projects/default/features')
+        .send({ name: featureName })
+        .expect(201);
+    let strategyId;
+    await app.request
+        .post(
+            `/api/admin/projects/default/features/${featureName}/environments/${envName}/strategies`,
+        )
+        .send({
+            name: 'default',
+            parameters: {
+                userId: 'string',
+            },
+        })
+        .expect(200)
+        .expect((res) => {
+            strategyId = res.body.id;
+        });
+    // Enable feature_environment
+    await app.request
+        .post(
+            `/api/admin/projects/default/features/${featureName}/environments/${envName}/on`,
+        )
+        .send({})
+        .expect(200);
+    await app.request
+        .get(
+            `/api/admin/projects/default/features/${featureName}/environments/${envName}`,
+        )
+        .expect(200)
+        .expect((res) => {
+            expect(res.body.enabled).toBeTruthy();
+        });
+    // Delete last strategy, this should also disable the environment
+    await app.request.delete(
+        `/api/admin/projects/default/features/${featureName}/environments/${envName}/strategies/${strategyId}`,
+    );
+    await app.request
+        .get(
+            `/api/admin/projects/default/features/${featureName}/environments/${envName}`,
+        )
+        .expect(200)
+        .expect((res) => {
+            expect(res.body.enabled).toBeFalsy();
+        });
+});
+
+test('Deleting strategy for feature environment should not disable that environment as long as there are other strategies', async () => {
+    const envName = 'any_strategy_delete_env';
+    const featureName = 'any_strategy_delete_feature';
+    // Create environment
+    await db.stores.environmentStore.create({
+        name: envName,
+        type: 'test',
+    });
+    // Connect environment to project
+    await app.request
+        .post('/api/admin/projects/default/environments')
+        .send({
+            environment: envName,
+        })
+        .expect(200);
+    await app.request
+        .post('/api/admin/projects/default/features')
+        .send({ name: featureName })
+        .expect(201);
+    let strategyId;
+    await app.request
+        .post(
+            `/api/admin/projects/default/features/${featureName}/environments/${envName}/strategies`,
+        )
+        .send({
+            name: 'default',
+            parameters: {
+                userId: 'string',
+            },
+        })
+        .expect(200)
+        .expect((res) => {
+            strategyId = res.body.id;
+        });
+    await app.request
+        .post(
+            `/api/admin/projects/default/features/${featureName}/environments/${envName}/strategies`,
+        )
+        .send({
+            name: 'default',
+            parameters: {
+                customerId: 'string',
+            },
+        })
+        .expect(200);
+    // Enable feature_environment
+    await app.request
+        .post(
+            `/api/admin/projects/default/features/${featureName}/environments/${envName}/on`,
+        )
+        .send({})
+        .expect(200);
+    await app.request
+        .get(
+            `/api/admin/projects/default/features/${featureName}/environments/${envName}`,
+        )
+        .expect(200)
+        .expect((res) => {
+            expect(res.body.enabled).toBeTruthy();
+        });
+    // Delete a strategy, this should also disable the environment
+    await app.request.delete(
+        `/api/admin/projects/default/features/${featureName}/environments/${envName}/strategies/${strategyId}`,
+    );
+    await app.request
+        .get(
+            `/api/admin/projects/default/features/${featureName}/environments/${envName}`,
+        )
+        .expect(200)
+        .expect((res) => {
+            expect(res.body.enabled).toBeTruthy();
+        });
+});

src/test/fixtures/fake-feature-environment-store.ts

@@ -148,4 +148,13 @@ export default class FakeFeatureEnvironmentStore
     ): Promise<void> {
         return Promise.reject(new Error('Not implemented'));
     }
+
+    disableEnvironmentIfNoStrategies(
+        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+        featureName: string,
+        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+        environment: string,
+    ): Promise<void> {
+        return Promise.reject(new Error('Not implemented'));
+    }
 }