From cf42a829f4c4be573539ba5856f2fe0a184369c3 Mon Sep 17 00:00:00 2001 From: andreas-unleash Date: Tue, 17 Oct 2023 12:35:07 +0300 Subject: [PATCH] feat: add option to return disabled strategies (#5059) Adds the option to include disabled strategies (behind the playgroundImprovements flag Closes # [1-1505](https://linear.app/unleash/issue/1-1505/return-disabled-strategies-in-the-playground-features-request) --------- Signed-off-by: andreas-unleash --- .../feature-toggle-row-converter.ts | 23 ++++++++-- .../fakes/fake-feature-toggle-store.ts | 1 + .../feature-toggle/feature-toggle-service.ts | 1 + .../feature-toggle/feature-toggle-store.ts | 4 ++ .../types/feature-toggle-store-type.ts | 1 + .../playground/advanced-playground.test.ts | 46 ++++++++++++++++++- .../playground/feature-evaluator/client.ts | 1 - .../feature-evaluator/strategy/strategy.ts | 13 +++++- .../spec/playground-strategy-schema.ts | 8 +++- src/server-dev.ts | 1 + 10 files changed, 91 insertions(+), 8 deletions(-) diff --git a/src/lib/features/feature-toggle/converters/feature-toggle-row-converter.ts b/src/lib/features/feature-toggle/converters/feature-toggle-row-converter.ts index 354b049d8861..14181b93f7e3 100644 --- a/src/lib/features/feature-toggle/converters/feature-toggle-row-converter.ts +++ b/src/lib/features/feature-toggle/converters/feature-toggle-row-converter.ts @@ -70,6 +70,7 @@ export class FeatureToggleRowConverter { constraints: row.constraints || [], parameters: mapValues(row.parameters || {}, ensureStringValue), sortOrder: row.sort_order, + disabled: row.strategy_disabled, }; strategy.variants = row.strategy_variants || []; return strategy; @@ -111,6 +112,7 @@ export class FeatureToggleRowConverter { row: any, feature: PartialDeep, featureQuery?: IFeatureToggleQuery, + includeDisabledStrategies?: boolean, ) => { feature.impressionData = row.impression_data; feature.enabled = !!row.enabled; @@ -123,7 +125,10 @@ export class FeatureToggleRowConverter { feature.variants = row.variants || []; feature.project = row.project; - if (this.isUnseenStrategyRow(feature, row) && !row.strategy_disabled) { + if ( + this.isUnseenStrategyRow(feature, row) && + (includeDisabledStrategies ? true : !row.strategy_disabled) + ) { feature.strategies?.push(this.rowToStrategy(row)); } if (this.isNewTag(feature, row)) { @@ -141,13 +146,19 @@ export class FeatureToggleRowConverter { buildFeatureToggleListFromRows = ( rows: any[], featureQuery?: IFeatureToggleQuery, + includeDisabledStrategies?: boolean, ): FeatureToggle[] => { const result = rows.reduce((acc, r) => { let feature: PartialDeep = acc[r.name] ?? { strategies: [], }; - feature = this.createBaseFeature(r, feature, featureQuery); + feature = this.createBaseFeature( + r, + feature, + featureQuery, + includeDisabledStrategies, + ); feature.createdAt = r.created_at; feature.favorite = r.favorite; @@ -162,6 +173,7 @@ export class FeatureToggleRowConverter { buildPlaygroundFeaturesFromRows = ( rows: any[], dependentFeaturesEnabled: boolean, + includeDisabledStrategies: boolean, featureQuery?: IFeatureToggleQuery, ): FeatureConfigurationClient[] => { const result = rows.reduce((acc, r) => { @@ -169,7 +181,12 @@ export class FeatureToggleRowConverter { strategies: [], }; - feature = this.createBaseFeature(r, feature, featureQuery); + feature = this.createBaseFeature( + r, + feature, + featureQuery, + includeDisabledStrategies, + ); if (r.parent && dependentFeaturesEnabled) { feature.dependencies = feature.dependencies || []; diff --git a/src/lib/features/feature-toggle/fakes/fake-feature-toggle-store.ts b/src/lib/features/feature-toggle/fakes/fake-feature-toggle-store.ts index 935df3c332aa..979931cc1e45 100644 --- a/src/lib/features/feature-toggle/fakes/fake-feature-toggle-store.ts +++ b/src/lib/features/feature-toggle/fakes/fake-feature-toggle-store.ts @@ -165,6 +165,7 @@ export default class FakeFeatureToggleStore implements IFeatureToggleStore { async getPlaygroundFeatures( dependentFeaturesEnabled: boolean, + includeDisabledStrategies: boolean, query?: IFeatureToggleQuery, ): Promise { return this.features.filter( diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index 0e0b318665f2..a3fb77fee179 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/src/lib/features/feature-toggle/feature-toggle-service.ts @@ -1056,6 +1056,7 @@ class FeatureToggleService { await this.clientFeatureToggleStore.getPlayground(query || {}), await this.featureToggleStore.getPlaygroundFeatures( this.flagResolver.isEnabled('dependentFeatures'), + this.flagResolver.isEnabled('playgroundImprovements'), query, ), ]); diff --git a/src/lib/features/feature-toggle/feature-toggle-store.ts b/src/lib/features/feature-toggle/feature-toggle-store.ts index 97da22b60336..322b292e29d2 100644 --- a/src/lib/features/feature-toggle/feature-toggle-store.ts +++ b/src/lib/features/feature-toggle/feature-toggle-store.ts @@ -123,6 +123,7 @@ export default class FeatureToggleStore implements IFeatureToggleStore { featureQuery?: IFeatureToggleQuery, userId?: number, archived: boolean = false, + includeDisabledStrategies: boolean = false, ): Promise { const environment = featureQuery?.environment || DEFAULT_ENV; @@ -150,11 +151,13 @@ export default class FeatureToggleStore implements IFeatureToggleStore { return this.featureToggleRowConverter.buildFeatureToggleListFromRows( rows, featureQuery, + includeDisabledStrategies, ); } async getPlaygroundFeatures( dependentFeaturesEnabled: boolean, + includeDisabledStrategies: boolean, featureQuery: IFeatureToggleQuery, ): Promise { const environment = featureQuery?.environment || DEFAULT_ENV; @@ -177,6 +180,7 @@ export default class FeatureToggleStore implements IFeatureToggleStore { return this.featureToggleRowConverter.buildPlaygroundFeaturesFromRows( rows, dependentFeaturesEnabled, + includeDisabledStrategies, featureQuery, ); } diff --git a/src/lib/features/feature-toggle/types/feature-toggle-store-type.ts b/src/lib/features/feature-toggle/types/feature-toggle-store-type.ts index b3b8b5fca2e5..b21395dc2439 100644 --- a/src/lib/features/feature-toggle/types/feature-toggle-store-type.ts +++ b/src/lib/features/feature-toggle/types/feature-toggle-store-type.ts @@ -39,6 +39,7 @@ export interface IFeatureToggleStore extends Store { ): Promise; getPlaygroundFeatures( dependentFeaturesEnabled: boolean, + includeDisabledStrategies: boolean, featureQuery?: IFeatureToggleQuery, ): Promise; countByDate(queryModifiers: { diff --git a/src/lib/features/playground/advanced-playground.test.ts b/src/lib/features/playground/advanced-playground.test.ts index 0eb9b9975248..cc44509c4fa2 100644 --- a/src/lib/features/playground/advanced-playground.test.ts +++ b/src/lib/features/playground/advanced-playground.test.ts @@ -11,7 +11,9 @@ let db: ITestDb; beforeAll(async () => { db = await dbInit('advanced_playground', getLogger, { - experimental: { flags: { dependentFeatures: true } }, + experimental: { + flags: { dependentFeatures: true, playgroundImprovements: true }, + }, }); app = await setupAppWithCustomConfig( db.stores, @@ -23,6 +25,9 @@ beforeAll(async () => { strategyVariant: true, privateProjects: true, dependentFeatures: true, + playgroundImprovements: true, + useLastSeenRefactor: true, + separateAdminClientApi: true, }, }, }, @@ -327,6 +332,7 @@ test('show matching variant from variants selection only for enabled toggles', a variants, }, ); + await enableToggle('test-playground-feature-with-variants'); const { body: result } = await app.request @@ -357,3 +363,41 @@ test('show matching variant from variants selection only for enabled toggles', a expect(feature.variants).toMatchObject([]); }); }); + +test('should return disabled strategies with unevaluated result', async () => { + await createFeatureToggleWithStrategy( + 'test-playground-feature-with-disabled-strategy', + { + name: 'flexibleRollout', + constraints: [], + disabled: true, + parameters: { + rollout: '50', + stickiness: 'random', + groupId: 'test-playground-feature-with-variants', + }, + }, + ); + + const { body: result } = await app.request + .post('/api/admin/playground/advanced') + .send({ + environments: ['default'], + projects: ['default'], + context: { appName: 'playground' }, + }) + .set('Content-Type', 'application/json') + .expect(200); + + const typedResult: AdvancedPlaygroundResponseSchema = result; + + const feature = typedResult.features.find( + (feature) => + feature.name === 'test-playground-feature-with-disabled-strategy', + ); + + expect( + feature?.environments.default[0].strategies.data[0].result + .evaluationStatus, + ).toBe('unevaluated'); +}); diff --git a/src/lib/features/playground/feature-evaluator/client.ts b/src/lib/features/playground/feature-evaluator/client.ts index 6735095e5ce8..92bd84966430 100644 --- a/src/lib/features/playground/feature-evaluator/client.ts +++ b/src/lib/features/playground/feature-evaluator/client.ts @@ -44,7 +44,6 @@ export default class UnleashClient { !strategy || !strategy.name || typeof strategy.name !== 'string' || - !strategy.isEnabled || typeof strategy.isEnabled !== 'function' ) { throw new Error('Invalid strategy data / interface'); diff --git a/src/lib/features/playground/feature-evaluator/strategy/strategy.ts b/src/lib/features/playground/feature-evaluator/strategy/strategy.ts index 41855be54e00..c7275bf3bae9 100644 --- a/src/lib/features/playground/feature-evaluator/strategy/strategy.ts +++ b/src/lib/features/playground/feature-evaluator/strategy/strategy.ts @@ -145,9 +145,20 @@ export class Strategy { } : undefined; + if (disabled) { + return { + result: { + enabled: 'unknown', + evaluationStatus: 'unevaluated', + }, + constraints: constraintResults.constraints, + segments: segmentResults.segments, + }; + } + return { result: { - enabled: disabled ? false : overallResult, + enabled: overallResult, evaluationStatus: 'complete', variant, variants: variant ? variantDefinitions : undefined, diff --git a/src/lib/openapi/spec/playground-strategy-schema.ts b/src/lib/openapi/spec/playground-strategy-schema.ts index c039766a8560..cd510e527ca8 100644 --- a/src/lib/openapi/spec/playground-strategy-schema.ts +++ b/src/lib/openapi/spec/playground-strategy-schema.ts @@ -8,6 +8,7 @@ import { overrideSchema } from './override-schema'; export const playgroundStrategyEvaluation = { evaluationComplete: 'complete', evaluationIncomplete: 'incomplete', + unevaluated: 'unevaluated', unknownResult: 'unknown', } as const; @@ -21,8 +22,11 @@ export const strategyEvaluationResults = { evaluationStatus: { type: 'string', description: - "Signals that this strategy could not be evaluated. This is most likely because you're using a custom strategy that Unleash doesn't know about.", - enum: [playgroundStrategyEvaluation.evaluationIncomplete], + "Signals that this strategy could not be evaluated. This is most likely because you're using a custom strategy that Unleash doesn't know about. The `unevaluated` result is also returned if the strategy is disabled.", + enum: [ + playgroundStrategyEvaluation.evaluationIncomplete, + playgroundStrategyEvaluation.unevaluated, + ], }, enabled: { description: diff --git a/src/server-dev.ts b/src/server-dev.ts index 8a8ab7d90223..63d3c6eed832 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -48,6 +48,7 @@ process.nextTick(async () => { dependentFeatures: true, useLastSeenRefactor: true, separateAdminClientApi: true, + playgroundImprovements: true, }, }, authentication: {