fix: only send FEATURE_UPDATED for legacy (#1054)

Co-authored-by: Christopher Kolstad <chriswk@getunleash.ai>

src/lib/event-differ.js

@@ -148,7 +148,7 @@ function eachConsecutiveEvent(events, callback) {
     });
 }
 
-const ignoredProps = ['createdAt', 'lastSeenAt', 'environments', 'id'];
+const ignoredProps = ['createdAt', 'lastSeenAt', 'id'];
 
 const filterProps = (path, key) => {
     return ignoredProps.includes(key);

src/lib/routes/admin-api/feature.ts

@@ -104,22 +104,10 @@ class FeatureController extends Controller {
         res: Response,
     ): Promise<void> {
         const name = req.params.featureName;
-        const feature = await this.getLegacyFeatureToggle(name);
+        const feature = await this.featureService2.getFeatureToggleLegacy(name);
         res.json(feature).end();
     }
 
-    private async getLegacyFeatureToggle(name: string): Promise<any> {
-        const feature = await this.featureService2.getFeatureToggle(name);
-        const defaultEnv = feature.environments.find(
-            (e) => e.name === DEFAULT_ENV,
-        );
-        const strategies = defaultEnv?.strategies || [];
-        const enabled = defaultEnv?.enabled || false;
-        delete feature.environments;
-
-        return { ...feature, enabled, strategies };
-    }
-
     async listTags(req: Request, res: Response): Promise<void> {
         const tags = await this.featureTagService.listTags(
             req.params.featureName,
@@ -233,7 +221,12 @@ class FeatureController extends Controller {
             userName,
         );
 
-        const feature = await this.getLegacyFeatureToggle(featureName);
+        const feature =
+            await this.featureService2.storeFeatureUpdatedEventLegacy(
+                featureName,
+                userName,
+            );
+
         res.status(200).json(feature);
     }
 
@@ -249,6 +242,10 @@ class FeatureController extends Controller {
             DEFAULT_ENV,
             userName,
         );
+        await this.featureService2.storeFeatureUpdatedEventLegacy(
+            featureName,
+            userName,
+        );
         res.status(200).json(feature);
     }
 
@@ -263,6 +260,10 @@ class FeatureController extends Controller {
             true,
             userName,
         );
+        await this.featureService2.storeFeatureUpdatedEventLegacy(
+            featureName,
+            userName,
+        );
         res.json(feature);
     }
 
@@ -277,6 +278,10 @@ class FeatureController extends Controller {
             false,
             userName,
         );
+        await this.featureService2.storeFeatureUpdatedEventLegacy(
+            featureName,
+            userName,
+        );
         res.json(feature);
     }
 
@@ -284,7 +289,11 @@ class FeatureController extends Controller {
         const { featureName } = req.params;
         const userName = extractUsername(req);
         await this.featureService2.updateStale(featureName, true, userName);
-        const feature = await this.getLegacyFeatureToggle(featureName);
+        const feature =
+            await this.featureService2.storeFeatureUpdatedEventLegacy(
+                featureName,
+                userName,
+            );
         res.json(feature).end();
     }
 
@@ -292,7 +301,11 @@ class FeatureController extends Controller {
         const { featureName } = req.params;
         const userName = extractUsername(req);
         await this.featureService2.updateStale(featureName, false, userName);
-        const feature = await this.getLegacyFeatureToggle(featureName);
+        const feature =
+            await this.featureService2.storeFeatureUpdatedEventLegacy(
+                featureName,
+                userName,
+            );
         res.json(feature).end();
     }
 

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

@@ -34,7 +34,7 @@ import {
     FeatureToggle,
     FeatureToggleDTO,
     FeatureToggleWithEnvironment,
-    FeatureToggleWithEnvironmentLegacy,
+    FeatureToggleLegacy,
     IEnvironmentDetail,
     IFeatureEnvironmentInfo,
     IFeatureOverview,
@@ -152,7 +152,11 @@ class FeatureToggleServiceV2 {
                 project: projectId,
                 createdBy: userName,
                 environment,
-                data: { ...data, featureName: newFeatureStrategy.featureName },
+                data: {
+                    ...data,
+                    name: featureName, // Done like this since we use data as our return object.
+                    strategyName: newFeatureStrategy.strategyName,
+                },
             });
             return data;
         } catch (e) {
@@ -480,13 +484,6 @@ class FeatureToggleServiceV2 {
             project: projectId,
             tags,
         });
-        await this.eventStore.store({
-            type: FEATURE_UPDATED,
-            createdBy: userName,
-            data: featureToggle,
-            project: projectId,
-            tags,
-        });
         return featureToggle;
     }
 
@@ -653,16 +650,6 @@ class FeatureToggleServiceV2 {
             const tags = await this.featureTagStore.getAllTagsForFeature(
                 featureName,
             );
-            const data = await this.getFeatureToggleLegacy(featureName);
-
-            await this.eventStore.store({
-                type: FEATURE_UPDATED,
-                createdBy: userName,
-                data,
-                tags,
-                project: projectId,
-                environment,
-            });
             await this.eventStore.store({
                 type: enabled
                     ? FEATURE_ENVIRONMENT_ENABLED
@@ -680,6 +667,25 @@ class FeatureToggleServiceV2 {
         );
     }
 
+    async storeFeatureUpdatedEventLegacy(
+        featureName: string,
+        userName: string,
+    ): Promise<FeatureToggleLegacy> {
+        const tags = await this.featureTagStore.getAllTagsForFeature(
+            featureName,
+        );
+        const feature = await this.getFeatureToggleLegacy(featureName);
+
+        await this.eventStore.store({
+            type: FEATURE_UPDATED,
+            createdBy: userName,
+            data: feature,
+            tags,
+            project: feature.project,
+        });
+        return feature;
+    }
+
     // @deprecated
     async toggle(
         projectId: string,
@@ -704,28 +710,28 @@ class FeatureToggleServiceV2 {
 
     async getFeatureToggleLegacy(
         featureName: string,
-    ): Promise<FeatureToggleWithEnvironmentLegacy> {
+    ): Promise<FeatureToggleLegacy> {
         const feature =
             await this.featureStrategiesStore.getFeatureToggleWithEnvs(
                 featureName,
             );
-        const defaultEnv = feature.environments.find(
+        const { environments, ...legacyFeature } = feature;
-            (e) => e.name === DEFAULT_ENV,
+        const defaultEnv = environments.find((e) => e.name === DEFAULT_ENV);
-        );
         const strategies = defaultEnv?.strategies || [];
         const enabled = defaultEnv?.enabled || false;
 
-        return { ...feature, enabled, strategies };
+        return { ...legacyFeature, enabled, strategies };
     }
 
     // @deprecated
+    // TODO: move to projectService
     async updateField(
         featureName: string,
         field: string,
         // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
         value: any,
         userName: string,
-        event?: string,
+        event: string,
     ): Promise<any> {
         const feature = await this.featureToggleStore.get(featureName);
         feature[field] = value;
@@ -738,7 +744,7 @@ class FeatureToggleServiceV2 {
         const data = await this.getFeatureToggleLegacy(featureName);
 
         await this.eventStore.store({
-            type: event || FEATURE_UPDATED,
+            type: event,
             createdBy: userName,
             data,
             project: data.project,

src/lib/types/model.ts

@@ -67,8 +67,7 @@ export interface FeatureToggleWithEnvironment extends FeatureToggle {
 }
 
 // @deprecated
-export interface FeatureToggleWithEnvironmentLegacy
+export interface FeatureToggleLegacy extends FeatureToggle {
-    extends FeatureToggleWithEnvironment {
     strategies: IStrategyConfig[];
     enabled: boolean;
 }

src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts

@@ -1,5 +1,3 @@
-import EventService from '../../../lib/services/event-service';
-import { FEATURE_UPDATED } from '../../../lib/types/events';
 import FeatureToggleServiceV2 from '../../../lib/services/feature-toggle-service-v2';
 import { IStrategyConfig } from '../../../lib/types/model';
 import { createTestConfig } from '../../config/test-config';
@@ -9,7 +7,6 @@ import { DEFAULT_ENV } from '../../../lib/util/constants';
 let stores;
 let db;
 let service: FeatureToggleServiceV2;
-let eventService: EventService;
 
 beforeAll(async () => {
     const config = createTestConfig();
@@ -19,7 +16,6 @@ beforeAll(async () => {
     );
     stores = db.stores;
     service = new FeatureToggleServiceV2(stores, config);
-    eventService = new EventService(stores, config);
 });
 
 afterAll(async () => {
@@ -91,39 +87,6 @@ test('Should be able to update existing strategy configuration', async () => {
     expect(updatedConfig.parameters).toEqual({ b2b: true });
 });
 
-test('Should include legacy props in event log when updating strategy configuration', async () => {
-    const userName = 'event-tester';
-    const featureName = 'update-existing-strategy-events';
-    const config: Omit<IStrategyConfig, 'id'> = {
-        name: 'default',
-        constraints: [],
-        parameters: {},
-    };
-
-    await service.createFeatureToggle(
-        'default',
-        {
-            name: featureName,
-        },
-        userName,
-    );
-
-    await service.createStrategy(config, 'default', featureName, userName);
-    await service.updateEnabled(
-        'default',
-        featureName,
-        DEFAULT_ENV,
-        true,
-        userName,
-    );
-
-    const events = await eventService.getEventsForToggle(featureName);
-    const updatedEvent = events.find((e) => e.type === FEATURE_UPDATED);
-    expect(updatedEvent.type).toBe(FEATURE_UPDATED);
-    expect(updatedEvent.data.enabled).toBe(true);
-    expect(updatedEvent.data.strategies).toBeDefined();
-});
-
 test('Should be able to get strategy by id', async () => {
     const userName = 'strategy';
     const config: Omit<IStrategyConfig, 'id'> = {