bug(#3545): include strategy titles on playground evaluation results (#4084)

This PR adds strategy titles as an optional bit of data added to client
features. It's only added when prompted.


![image](https://github.com/Unleash/unleash/assets/17786332/99509679-2aab-4c2a-abff-c6e6f27d8074)

## Discussion points:

### getPlaygroundFeatures

The optional `includeStrategyId` parameter has been replaced by a
`getPlaygroundFeatures` in the service (and in the underlying store).
The playground was the only place that used this specific include, so
instead of adding more and making the interface for that method more
complex, I created a new method that deals specifically with the
playground.

The underlying store still uses an `optionalIncludes` parameter,
however. I have a plan to make that interface more fluid, but I'd like
to propose that in a follow-up PR.

src/lib/db/feature-toggle-client-store.ts

@@ -16,13 +16,14 @@ import FeatureToggleStore from './feature-toggle-store';
 import { Db } from './db';
 import Raw = Knex.Raw;
 
+type OptionalClientFeatures = Set<'strategy IDs' | 'strategy titles'>;
+
 export interface IGetAllFeatures {
     featureQuery?: IFeatureToggleQuery;
     archived: boolean;
     isAdmin: boolean;
-    includeStrategyIds?: boolean;
-    includeDisabledStrategies?: boolean;
     userId?: number;
+    optionalIncludes?: OptionalClientFeatures;
 }
 
 export interface IGetAdminFeatures {
@@ -54,8 +55,8 @@ export default class FeatureToggleClientStore
         featureQuery,
         archived,
         isAdmin,
-        includeStrategyIds,
         userId,
+        optionalIncludes,
     }: IGetAllFeatures): Promise<IFeatureToggleClient[]> {
         const environment = featureQuery?.environment || DEFAULT_ENV;
         const stopTimer = this.timer('getFeatureAdmin');
@@ -74,6 +75,7 @@ export default class FeatureToggleClientStore
             'fe.environment as environment',
             'fs.id as strategy_id',
             'fs.strategy_name as strategy_name',
+            'fs.title as strategy_title',
             'fs.disabled as strategy_disabled',
             'fs.parameters as parameters',
             'fs.constraints as constraints',
@@ -198,19 +200,32 @@ export default class FeatureToggleClientStore
                 feature.lastSeenAt = r.last_seen_at;
                 feature.createdAt = r.created_at;
             }
+
             acc[r.name] = feature;
             return acc;
         }, {});
 
         const features: IFeatureToggleClient[] = Object.values(featureToggles);
 
-        if (!isAdmin && !includeStrategyIds) {
-            // We should not send strategy IDs from the client API,
-            // as this breaks old versions of the Go SDK (at least).
-            FeatureToggleClientStore.removeIdsFromStrategies(features);
-        }
+        // strip away unwanted properties
+        const cleanedFeatures = features.map(({ strategies, ...rest }) => ({
+            ...rest,
+            strategies: strategies?.map(({ id, title, ...strategy }) => ({
+                ...strategy,
 
-        return features;
+                ...(optionalIncludes?.has('strategy titles') && title
+                    ? { title }
+                    : {}),
+
+                // We should not send strategy IDs from the client API,
+                // as this breaks old versions of the Go SDK (at least).
+                ...(isAdmin || optionalIncludes?.has('strategy IDs')
+                    ? { id }
+                    : {}),
+            })),
+        }));
+
+        return cleanedFeatures;
     }
 
     private static rowToStrategy(row: Record<string, any>): IStrategyConfig {
@@ -230,14 +245,6 @@ export default class FeatureToggleClientStore
         };
     }
 
-    private static removeIdsFromStrategies(features: IFeatureToggleClient[]) {
-        features.forEach((feature) => {
-            feature.strategies.forEach((strategy) => {
-                delete strategy.id;
-            });
-        });
-    }
-
     private isUnseenStrategyRow(
         feature: PartialDeep<IFeatureToggleClient>,
         row: Record<string, any>,
@@ -298,15 +305,22 @@ export default class FeatureToggleClientStore
 
     async getClient(
         featureQuery?: IFeatureToggleQuery,
-        includeStrategyIds?: boolean,
-        includeDisabledStrategies?: boolean,
     ): Promise<IFeatureToggleClient[]> {
         return this.getAll({
             featureQuery,
             archived: false,
             isAdmin: false,
-            includeStrategyIds,
-            includeDisabledStrategies,
+        });
+    }
+
+    async getPlayground(
+        featureQuery?: IFeatureToggleQuery,
+    ): Promise<IFeatureToggleClient[]> {
+        return this.getAll({
+            featureQuery,
+            archived: false,
+            isAdmin: false,
+            optionalIncludes: new Set(['strategy titles', 'strategy IDs']),
         });
     }
 
@@ -320,7 +334,6 @@ export default class FeatureToggleClientStore
             archived: Boolean(archived),
             isAdmin: true,
             userId,
-            includeDisabledStrategies: true,
         });
     }
 }

src/lib/features/playground/playground-service.ts

@@ -194,14 +194,11 @@ export class PlaygroundService {
             environment: string;
         }
     > {
-        const features = await this.featureToggleService.getClientFeatures(
-            {
-                project: projects === ALL ? undefined : projects,
-                environment,
-            },
-            true,
-            false,
-        );
+        const features = await this.featureToggleService.getPlaygroundFeatures({
+            project: projects === ALL ? undefined : projects,
+            environment,
+        });
+
         const featureProject: Record<string, string> = features.reduce(
             (obj, feature) => {
                 obj[feature.name] = feature.project;

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

@@ -828,13 +828,9 @@ class FeatureToggleService {
 
     async getClientFeatures(
         query?: IFeatureToggleQuery,
-        includeIds?: boolean,
-        includeDisabledStrategies?: boolean,
     ): Promise<FeatureConfigurationClient[]> {
         const result = await this.featureToggleClientStore.getClient(
             query || {},
-            includeIds,
-            includeDisabledStrategies,
         );
         if (this.flagResolver.isEnabled('cleanClientApi')) {
             return result.map(
@@ -869,6 +865,15 @@ class FeatureToggleService {
         }
     }
 
+    async getPlaygroundFeatures(
+        query?: IFeatureToggleQuery,
+    ): Promise<FeatureConfigurationClient[]> {
+        const result = await this.featureToggleClientStore.getPlayground(
+            query || {},
+        );
+        return result;
+    }
+
     /**
      * @deprecated Legacy!
      *

src/lib/types/stores/feature-toggle-client-store.ts

@@ -4,8 +4,10 @@ import { IGetAdminFeatures } from '../../db/feature-toggle-client-store';
 export interface IFeatureToggleClientStore {
     getClient(
         featureQuery: Partial<IFeatureToggleQuery>,
-        includeStrategyIds?: boolean,
-        includeDisabledStrategies?: boolean,
+    ): Promise<IFeatureToggleClient[]>;
+
+    getPlayground(
+        featureQuery: Partial<IFeatureToggleQuery>,
     ): Promise<IFeatureToggleClient[]>;
 
     // @Deprecated

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

@@ -592,3 +592,44 @@ test('If CRs are protected for any environment in the project stops bulk update
         ),
     ).rejects.toThrowError(new NoAccessError(SKIP_CHANGE_REQUEST));
 });
+
+test('getPlaygroundFeatures should return ids and titles (if they exist) on client strategies', async () => {
+    const featureName = 'check-returned-strategy-configuration';
+    const projectId = 'default';
+
+    const title = 'custom strategy title';
+    const userName = 'strategy';
+    const config: Omit<FeatureStrategySchema, 'id'> = {
+        name: 'default',
+        constraints: [],
+        parameters: {},
+        title,
+    };
+    await service.createFeatureToggle(
+        projectId,
+        {
+            name: featureName,
+        },
+        userName,
+    );
+
+    await service.createStrategy(
+        config,
+        { projectId, featureName, environment: DEFAULT_ENV },
+        userName,
+    );
+
+    const playgroundFeatures = await service.getPlaygroundFeatures();
+
+    const strategyWithTitle = playgroundFeatures.find(
+        (feature) => feature.name === featureName,
+    )!.strategies[0];
+
+    expect(strategyWithTitle.title).toStrictEqual(title);
+
+    for (const strategy of playgroundFeatures.flatMap(
+        (feature) => feature.strategies,
+    )) {
+        expect(strategy.id).not.toBeUndefined();
+    }
+});

src/test/fixtures/fake-feature-toggle-client-store.ts

@@ -53,6 +53,19 @@ export default class FakeFeatureToggleClientStore
         return this.getFeatures(query);
     }
 
+    async getPlayground(
+        query?: IFeatureToggleQuery,
+    ): Promise<IFeatureToggleClient[]> {
+        const features = await this.getFeatures(query);
+        return features.map(({ strategies, ...rest }) => ({
+            ...rest,
+            strategies: strategies.map((strategy, index) => ({
+                ...strategy,
+                id: `strategy#${index}`,
+            })),
+        }));
+    }
+
     async getAdmin({
         featureQuery: query,
         archived,