diff --git a/src/lib/db/feature-toggle-client-store.ts b/src/lib/db/feature-toggle-client-store.ts index 2870449680..e3ece08327 100644 --- a/src/lib/db/feature-toggle-client-store.ts +++ b/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 { 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): 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, row: Record, @@ -298,15 +305,22 @@ export default class FeatureToggleClientStore async getClient( featureQuery?: IFeatureToggleQuery, - includeStrategyIds?: boolean, - includeDisabledStrategies?: boolean, ): Promise { return this.getAll({ featureQuery, archived: false, isAdmin: false, - includeStrategyIds, - includeDisabledStrategies, + }); + } + + async getPlayground( + featureQuery?: IFeatureToggleQuery, + ): Promise { + 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, }); } } diff --git a/src/lib/features/playground/playground-service.ts b/src/lib/features/playground/playground-service.ts index 6a44c83c79..b1e4e410d3 100644 --- a/src/lib/features/playground/playground-service.ts +++ b/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 = features.reduce( (obj, feature) => { obj[feature.name] = feature.project; diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 0d4cd8a11d..ce94261df8 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -828,13 +828,9 @@ class FeatureToggleService { async getClientFeatures( query?: IFeatureToggleQuery, - includeIds?: boolean, - includeDisabledStrategies?: boolean, ): Promise { 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 { + const result = await this.featureToggleClientStore.getPlayground( + query || {}, + ); + return result; + } + /** * @deprecated Legacy! * diff --git a/src/lib/types/stores/feature-toggle-client-store.ts b/src/lib/types/stores/feature-toggle-client-store.ts index 824429075f..090ec8fa61 100644 --- a/src/lib/types/stores/feature-toggle-client-store.ts +++ b/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, - includeStrategyIds?: boolean, - includeDisabledStrategies?: boolean, + ): Promise; + + getPlayground( + featureQuery: Partial, ): Promise; // @Deprecated diff --git a/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts b/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts index 25b4370ad3..52bd074b08 100644 --- a/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts +++ b/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 = { + 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(); + } +}); diff --git a/src/test/fixtures/fake-feature-toggle-client-store.ts b/src/test/fixtures/fake-feature-toggle-client-store.ts index d765b8bcfd..c2d3e7c713 100644 --- a/src/test/fixtures/fake-feature-toggle-client-store.ts +++ b/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 { + 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,