From 097dd8ae560641e3b5ab0d9b4b5277ac2be3c6d7 Mon Sep 17 00:00:00 2001 From: andreas-unleash Date: Fri, 21 Apr 2023 12:09:07 +0300 Subject: [PATCH] Feat/enable disable strategies (#3566) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds enabled field to feature strategies Filter out disabled strategies when returning/evaluating ## About the changes Closes # [1-865](https://linear.app/unleash/issue/1-865/allow-for-enablingdisabling-strategies-in-place-backend) ### Important files ## Discussion points --------- Signed-off-by: andreas-unleash --- src/lib/db/feature-strategy-store.ts | 9 ++ src/lib/db/feature-toggle-client-store.ts | 71 +++++++----- .../spec/create-feature-strategy-schema.ts | 7 ++ .../openapi/spec/feature-strategy-schema.ts | 7 ++ .../spec/playground-feature-schema.test.ts | 1 + .../spec/playground-strategy-schema.ts | 17 ++- .../spec/update-feature-strategy-schema.ts | 13 +++ src/lib/services/feature-toggle-service.ts | 54 +++++----- src/lib/services/playground-service.ts | 1 + src/lib/services/strategy-schema.ts | 1 + src/lib/types/model.ts | 2 + .../stores/feature-toggle-client-store.ts | 1 + src/lib/util/feature-evaluator/client.ts | 2 + .../feature-evaluator/strategy/strategy.ts | 13 ++- ...-add-disabled-field-to-feature-strategy.js | 85 +++++++++++++++ src/test/e2e/api/admin/playground.e2e.test.ts | 3 +- .../api/admin/project/features.e2e.test.ts | 99 +++++++++++++++++ .../__snapshots__/openapi.e2e.test.ts.snap | 31 ++++++ src/test/e2e/api/proxy/proxy.e2e.test.ts | 101 ++++++++++++++++++ .../e2e/services/playground-service.test.ts | 2 + 20 files changed, 462 insertions(+), 58 deletions(-) create mode 100644 src/migrations/20230419104126-add-disabled-field-to-feature-strategy.js diff --git a/src/lib/db/feature-strategy-store.ts b/src/lib/db/feature-strategy-store.ts index c7f93ba4416..ad33284e22a 100644 --- a/src/lib/db/feature-strategy-store.ts +++ b/src/lib/db/feature-strategy-store.ts @@ -35,6 +35,7 @@ const COLUMNS = [ 'parameters', 'constraints', 'created_at', + 'disabled', ]; /* const mapperToColumnNames = { @@ -62,6 +63,7 @@ interface IFeatureStrategiesTable { constraints: string; sort_order: number; created_at?: Date; + disabled?: boolean | null; } export interface ILoadFeatureToggleWithEnvsParams { @@ -83,6 +85,7 @@ function mapRow(row: IFeatureStrategiesTable): IFeatureStrategy { constraints: (row.constraints as unknown as IConstraint[]) || [], createdAt: row.created_at, sortOrder: row.sort_order, + disabled: row.disabled, }; } @@ -98,6 +101,7 @@ function mapInput(input: IFeatureStrategy): IFeatureStrategiesTable { constraints: JSON.stringify(input.constraints || []), created_at: input.createdAt, sort_order: input.sortOrder, + disabled: input.disabled, }; } @@ -106,6 +110,7 @@ interface StrategyUpdate { parameters: object; constraints: string; title?: string; + disabled?: boolean; } function mapStrategyUpdate( @@ -121,6 +126,9 @@ function mapStrategyUpdate( if (input.title !== null) { update.title = input.title; } + if (input.disabled !== null) { + update.disabled = input.disabled; + } update.constraints = JSON.stringify(input.constraints || []); return update; } @@ -590,6 +598,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { sortOrder: r.sort_order, id: r.strategy_id, title: r.strategy_title || '', + disabled: r.strategy_disabled || false, }; if (!includeId) { delete strategy.id; diff --git a/src/lib/db/feature-toggle-client-store.ts b/src/lib/db/feature-toggle-client-store.ts index 97c4c8e52ce..d7db7e4eb80 100644 --- a/src/lib/db/feature-toggle-client-store.ts +++ b/src/lib/db/feature-toggle-client-store.ts @@ -4,36 +4,24 @@ import { DB_TIME } from '../metric-events'; import { Logger, LogProvider } from '../logger'; import { IFeatureToggleClient, + IFeatureToggleClientStore, IFeatureToggleQuery, IStrategyConfig, ITag, -} from '../types/model'; -import { IFeatureToggleClientStore } from '../types/stores/feature-toggle-client-store'; -import { DEFAULT_ENV } from '../util/constants'; -import { PartialDeep } from '../types/partial'; + PartialDeep, +} from '../types'; +import { DEFAULT_ENV, ensureStringValue, mapValues } from '../util'; import EventEmitter from 'events'; import FeatureToggleStore from './feature-toggle-store'; -import { ensureStringValue } from '../util/ensureStringValue'; -import { mapValues } from '../util/map-values'; -import Raw = Knex.Raw; import { Db } from './db'; - -export interface FeaturesTable { - name: string; - description: string; - type: string; - stale: boolean; - variants: string; - project: string; - last_seen_at?: Date; - created_at?: Date; -} +import Raw = Knex.Raw; export interface IGetAllFeatures { featureQuery?: IFeatureToggleQuery; archived: boolean; isAdmin: boolean; includeStrategyIds?: boolean; + includeDisabledStrategies?: boolean; userId?: number; } @@ -67,6 +55,7 @@ export default class FeatureToggleClientStore archived, isAdmin, includeStrategyIds, + includeDisabledStrategies, userId, }: IGetAllFeatures): Promise { const environment = featureQuery?.environment || DEFAULT_ENV; @@ -86,6 +75,7 @@ export default class FeatureToggleClientStore 'fe.environment as environment', 'fs.id as strategy_id', 'fs.strategy_name as strategy_name', + 'fs.disabled as strategy_disabled', 'fs.parameters as parameters', 'fs.constraints as constraints', 'segments.id as segment_id', @@ -180,7 +170,7 @@ export default class FeatureToggleClientStore strategies: [], }; if (this.isUnseenStrategyRow(feature, r)) { - feature.strategies.push( + feature.strategies?.push( FeatureToggleClientStore.rowToStrategy(r), ); } @@ -221,6 +211,12 @@ export default class FeatureToggleClientStore FeatureToggleClientStore.removeIdsFromStrategies(features); } + if (!includeDisabledStrategies) { + // We should not send disabled strategies from the client API, + // as this breaks the way SDKs evaluate the status of the feature. + return this.removeDisabledStrategies(features); + } + return features; } @@ -228,6 +224,8 @@ export default class FeatureToggleClientStore return { id: row.strategy_id, name: row.strategy_name, + title: row.strategy_title, + disabled: row.strategy_disabled, constraints: row.constraints || [], parameters: mapValues(row.parameters || {}, ensureStringValue), }; @@ -248,13 +246,26 @@ export default class FeatureToggleClientStore }); } + private removeDisabledStrategies( + features: IFeatureToggleClient[], + ): IFeatureToggleClient[] { + const filtered: IFeatureToggleClient[] = []; + features.forEach((feature) => { + const filteredStrategies = feature.strategies.filter( + (strategy) => !strategy.disabled, + ); + filtered.push({ ...feature, strategies: filteredStrategies }); + }); + return filtered; + } + private isUnseenStrategyRow( feature: PartialDeep, row: Record, ): boolean { return ( row.strategy_id && - !feature.strategies.find((s) => s.id === row.strategy_id) + !feature.strategies?.find((s) => s?.id === row.strategy_id) ); } @@ -276,7 +287,7 @@ export default class FeatureToggleClientStore row.tag_value && !feature.tags?.some( (tag) => - tag.type === row.tag_type && tag.value === row.tag_value, + tag?.type === row.tag_type && tag?.value === row.tag_value, ) ); } @@ -286,16 +297,16 @@ export default class FeatureToggleClientStore row: Record, ) { feature.strategies - .find((s) => s.id === row.strategy_id) - ?.constraints.push(...row.segment_constraints); + ?.find((s) => s?.id === row.strategy_id) + ?.constraints?.push(...row.segment_constraints); } private addSegmentIdsToStrategy( feature: PartialDeep, row: Record, ) { - const strategy = feature.strategies.find( - (s) => s.id === row.strategy_id, + const strategy = feature.strategies?.find( + (s) => s?.id === row.strategy_id, ); if (!strategy) { return; @@ -309,12 +320,14 @@ export default class FeatureToggleClientStore async getClient( featureQuery?: IFeatureToggleQuery, includeStrategyIds?: boolean, + includeDisabledStrategies?: boolean, ): Promise { return this.getAll({ featureQuery, archived: false, isAdmin: false, includeStrategyIds, + includeDisabledStrategies, }); } @@ -323,7 +336,13 @@ export default class FeatureToggleClientStore userId, archived, }: IGetAdminFeatures): Promise { - return this.getAll({ featureQuery, archived, isAdmin: true, userId }); + return this.getAll({ + featureQuery, + archived: Boolean(archived), + isAdmin: true, + userId, + includeDisabledStrategies: true, + }); } } diff --git a/src/lib/openapi/spec/create-feature-strategy-schema.ts b/src/lib/openapi/spec/create-feature-strategy-schema.ts index 12e63595005..f8488ee13c7 100644 --- a/src/lib/openapi/spec/create-feature-strategy-schema.ts +++ b/src/lib/openapi/spec/create-feature-strategy-schema.ts @@ -18,6 +18,13 @@ export const createFeatureStrategySchema = { description: 'A descriptive title for the strategy', example: 'Gradual Rollout 25-Prod', }, + disabled: { + type: 'boolean', + description: + 'A toggle to disable the strategy. defaults to false. Disabled strategies are not evaluated or returned to the SDKs', + example: false, + nullable: true, + }, sortOrder: { type: 'number', description: 'The order of the strategy in the list', diff --git a/src/lib/openapi/spec/feature-strategy-schema.ts b/src/lib/openapi/spec/feature-strategy-schema.ts index d7979967fb8..3741f6ca31f 100644 --- a/src/lib/openapi/spec/feature-strategy-schema.ts +++ b/src/lib/openapi/spec/feature-strategy-schema.ts @@ -26,6 +26,13 @@ export const featureStrategySchema = { example: 'Gradual Rollout 25-Prod', nullable: true, }, + disabled: { + type: 'boolean', + description: + 'A toggle to disable the strategy. defaults to false. Disabled strategies are not evaluated or returned to the SDKs', + example: false, + nullable: true, + }, featureName: { type: 'string', description: 'The name or feature the strategy is attached to', diff --git a/src/lib/openapi/spec/playground-feature-schema.test.ts b/src/lib/openapi/spec/playground-feature-schema.test.ts index e4cf049c3b8..3da9ef6f988 100644 --- a/src/lib/openapi/spec/playground-feature-schema.test.ts +++ b/src/lib/openapi/spec/playground-feature-schema.test.ts @@ -64,6 +64,7 @@ const playgroundStrategy = ( parameters, constraints: playgroundStrategyConstraints(), segments: fc.array(playgroundSegment()), + disabled: fc.boolean(), }); const playgroundStrategies = (): Arbitrary => diff --git a/src/lib/openapi/spec/playground-strategy-schema.ts b/src/lib/openapi/spec/playground-strategy-schema.ts index 030ed44249a..13580f224b1 100644 --- a/src/lib/openapi/spec/playground-strategy-schema.ts +++ b/src/lib/openapi/spec/playground-strategy-schema.ts @@ -60,7 +60,15 @@ export const playgroundStrategySchema = { $id: '#/components/schemas/playgroundStrategySchema', type: 'object', additionalProperties: false, - required: ['id', 'name', 'result', 'segments', 'constraints', 'parameters'], + required: [ + 'id', + 'name', + 'result', + 'segments', + 'constraints', + 'parameters', + 'disabled', + ], properties: { name: { description: "The strategy's name.", @@ -79,6 +87,13 @@ export const playgroundStrategySchema = { description: `The strategy's evaluation result. If the strategy is a custom strategy that Unleash can't evaluate, \`evaluationStatus\` will be \`${playgroundStrategyEvaluation.unknownResult}\`. Otherwise, it will be \`true\` or \`false\``, ...strategyEvaluationResults, }, + disabled: { + type: 'boolean', + description: + "The strategy's status. Disabled strategies are not evaluated", + example: false, + nullable: true, + }, segments: { type: 'array', description: diff --git a/src/lib/openapi/spec/update-feature-strategy-schema.ts b/src/lib/openapi/spec/update-feature-strategy-schema.ts index ae243cb3051..0faf9d768f5 100644 --- a/src/lib/openapi/spec/update-feature-strategy-schema.ts +++ b/src/lib/openapi/spec/update-feature-strategy-schema.ts @@ -18,6 +18,19 @@ export const updateFeatureStrategySchema = { $ref: '#/components/schemas/constraintSchema', }, }, + title: { + type: 'string', + nullable: true, + description: 'A descriptive title for the strategy', + example: 'Gradual Rollout 25-Prod', + }, + disabled: { + type: 'boolean', + description: + 'A toggle to disable the strategy. defaults to true. Disabled strategies are not evaluated or returned to the SDKs', + example: false, + nullable: true, + }, parameters: { $ref: '#/components/schemas/parametersSchema', }, diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 7255666ce01..7a5d0415524 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -1,4 +1,5 @@ import { + CREATE_FEATURE_STRATEGY, EnvironmentVariantEvent, FEATURE_UPDATED, FeatureArchivedEvent, @@ -12,20 +13,38 @@ import { FeatureStrategyAddEvent, FeatureStrategyRemoveEvent, FeatureStrategyUpdateEvent, + FeatureToggle, + FeatureToggleDTO, + FeatureToggleLegacy, + FeatureToggleWithEnvironment, FeatureVariantEvent, + IConstraint, IEventStore, + IFeatureEnvironmentInfo, + IFeatureEnvironmentStore, + IFeatureOverview, + IFeatureStrategy, IFeatureTagStore, + IFeatureToggleClientStore, + IFeatureToggleQuery, IFeatureToggleStore, IFlagResolver, IProjectStore, + ISegment, + IStrategyConfig, IUnleashConfig, IUnleashStores, + IVariant, + Saved, + SKIP_CHANGE_REQUEST, + Unsaved, + WeightType, } from '../types'; import { Logger } from '../logger'; import BadDataError from '../error/bad-data-error'; import NameExistsError from '../error/name-exists-error'; import InvalidOperationError from '../error/invalid-operation-error'; -import { FOREIGN_KEY_VIOLATION } from '../error'; +import { FOREIGN_KEY_VIOLATION, OperationDeniedError } from '../error'; import { constraintSchema, featureMetadataSchema, @@ -37,32 +56,14 @@ import { FeatureConfigurationClient, IFeatureStrategiesStore, } from '../types/stores/feature-strategies-store'; -import { - FeatureToggle, - FeatureToggleDTO, - FeatureToggleLegacy, - FeatureToggleWithEnvironment, - IConstraint, - IFeatureEnvironmentInfo, - IFeatureOverview, - IFeatureStrategy, - IFeatureToggleQuery, - ISegment, - IStrategyConfig, - IVariant, - WeightType, -} from '../types/model'; -import { IFeatureEnvironmentStore } from '../types/stores/feature-environment-store'; -import { IFeatureToggleClientStore } from '../types/stores/feature-toggle-client-store'; import { DATE_OPERATORS, DEFAULT_ENV, NUM_OPERATORS, SEMVER_OPERATORS, STRING_OPERATORS, -} from '../util/constants'; +} from '../util'; import { applyPatch, deepClone, Operation } from 'fast-json-patch'; -import { OperationDeniedError } from '../error/operation-denied-error'; import { validateDate, validateLegalValues, @@ -71,15 +72,10 @@ import { validateString, } from '../util/validators/constraint-types'; import { IContextFieldStore } from 'lib/types/stores/context-field-store'; -import { Saved, Unsaved } from '../types/saved'; import { SetStrategySortOrderSchema } from 'lib/openapi/spec/set-strategy-sort-order-schema'; import { getDefaultStrategy } from '../util/feature-evaluator/helpers'; import { AccessService } from './access-service'; import { User } from '../server-impl'; -import { - CREATE_FEATURE_STRATEGY, - SKIP_CHANGE_REQUEST, -} from '../types/permissions'; import NoAccessError from '../error/no-access-error'; import { IFeatureProjectUserParams } from '../routes/admin-api/project/project-features'; import { unique } from '../util/unique'; @@ -356,6 +352,7 @@ class FeatureToggleService { id: featureStrategy.id, name: featureStrategy.strategyName, title: featureStrategy.title, + disabled: featureStrategy.disabled, constraints: featureStrategy.constraints || [], parameters: featureStrategy.parameters, segments: segments.map((segment) => segment.id) ?? [], @@ -418,6 +415,7 @@ class FeatureToggleService { await this.featureStrategiesStore.createStrategyFeatureEnv({ strategyName: strategyConfig.name, title: strategyConfig.title, + disabled: strategyConfig.disabled, constraints: strategyConfig.constraints || [], parameters: strategyConfig.parameters || {}, sortOrder: strategyConfig.sortOrder, @@ -474,6 +472,7 @@ class FeatureToggleService { * @param updates * @param context - Which context does this strategy live in (projectId, featureName, environment) * @param userName - Human readable id of the user performing the update + * @param user - Optional User object performing the action */ async updateStrategy( id: string, @@ -677,6 +676,8 @@ class FeatureToggleService { name: strat.strategyName, constraints: strat.constraints, parameters: strat.parameters, + title: strat.title, + disabled: strat.disabled, sortOrder: strat.sortOrder, segments, }); @@ -752,10 +753,12 @@ 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( @@ -1013,6 +1016,7 @@ class FeatureToggleService { parameters: strategy.parameters, segments: [], title: strategy.title, + disabled: strategy.disabled, }; if (segments && segments.length > 0) { diff --git a/src/lib/services/playground-service.ts b/src/lib/services/playground-service.ts index 4e675604998..42c4d54f3b1 100644 --- a/src/lib/services/playground-service.ts +++ b/src/lib/services/playground-service.ts @@ -41,6 +41,7 @@ export class PlaygroundService { environment, }, true, + true, ), this.segmentService.getActive(), ]); diff --git a/src/lib/services/strategy-schema.ts b/src/lib/services/strategy-schema.ts index 008d30fbdaa..4c293f32cd1 100644 --- a/src/lib/services/strategy-schema.ts +++ b/src/lib/services/strategy-schema.ts @@ -6,6 +6,7 @@ const strategySchema = joi .keys({ name: nameType, title: joi.string().allow(null).allow('').optional(), + disabled: joi.boolean().allow(null).optional(), editable: joi.boolean().default(true), deprecated: joi.boolean().default(false), description: joi.string().allow(null).allow('').optional(), diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index 5bdeb8647b4..82928dad7d9 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -28,6 +28,7 @@ export interface IStrategyConfig { parameters?: { [key: string]: string }; sortOrder?: number; title?: string | null; + disabled?: boolean | null; } export interface IFeatureStrategy { id: string; @@ -41,6 +42,7 @@ export interface IFeatureStrategy { createdAt?: Date; segments?: number[]; title?: string | null; + disabled?: boolean | null; } export interface FeatureToggleDTO { diff --git a/src/lib/types/stores/feature-toggle-client-store.ts b/src/lib/types/stores/feature-toggle-client-store.ts index 2902ca9d835..824429075f4 100644 --- a/src/lib/types/stores/feature-toggle-client-store.ts +++ b/src/lib/types/stores/feature-toggle-client-store.ts @@ -5,6 +5,7 @@ export interface IFeatureToggleClientStore { getClient( featureQuery: Partial, includeStrategyIds?: boolean, + includeDisabledStrategies?: boolean, ): Promise; // @Deprecated diff --git a/src/lib/util/feature-evaluator/client.ts b/src/lib/util/feature-evaluator/client.ts index 49b00f3c51e..9b58bff7f40 100644 --- a/src/lib/util/feature-evaluator/client.ts +++ b/src/lib/util/feature-evaluator/client.ts @@ -109,12 +109,14 @@ export default class UnleashClient { name: strategySelector.name, id: strategySelector.id, title: strategySelector.title, + disabled: strategySelector.disabled || false, parameters: strategySelector.parameters, ...strategy.isEnabledWithConstraints( strategySelector.parameters, context, strategySelector.constraints, segments, + strategySelector.disabled, ), }; }, diff --git a/src/lib/util/feature-evaluator/strategy/strategy.ts b/src/lib/util/feature-evaluator/strategy/strategy.ts index 76fe37caa0c..6f61cd58b9e 100644 --- a/src/lib/util/feature-evaluator/strategy/strategy.ts +++ b/src/lib/util/feature-evaluator/strategy/strategy.ts @@ -13,6 +13,7 @@ export type SegmentForEvaluation = { export interface StrategyTransportInterface { name: string; title?: string; + disabled?: boolean; parameters: any; constraints: Constraint[]; segments?: number[]; @@ -107,9 +108,7 @@ export class Strategy { }); return { - result: resolvedSegments.every( - (segment) => segment.result === true, - ), + result: resolvedSegments.every((segment) => segment.result), segments: resolvedSegments, }; } @@ -118,7 +117,8 @@ export class Strategy { parameters: unknown, context: Context, constraints: Iterable, - segments: SegmentForEvaluation[], + segments: Array, + disabled?: boolean, ): StrategyEvaluationResult { const constraintResults = this.checkConstraints(context, constraints); const enabledResult = this.isEnabled(parameters, context); @@ -128,7 +128,10 @@ export class Strategy { constraintResults.result && enabledResult && segmentResults.result; return { - result: { enabled: overallResult, evaluationStatus: 'complete' }, + result: { + enabled: disabled ? false : overallResult, + evaluationStatus: 'complete', + }, constraints: constraintResults.constraints, segments: segmentResults.segments, }; diff --git a/src/migrations/20230419104126-add-disabled-field-to-feature-strategy.js b/src/migrations/20230419104126-add-disabled-field-to-feature-strategy.js new file mode 100644 index 00000000000..342410d3995 --- /dev/null +++ b/src/migrations/20230419104126-add-disabled-field-to-feature-strategy.js @@ -0,0 +1,85 @@ +'use strict'; + +exports.up = function (db, callback) { + db.runSql( + ` + ALTER TABLE feature_strategies ADD COLUMN IF NOT EXISTS disabled BOOLEAN default false; + + CREATE OR REPLACE VIEW features_view AS + SELECT + features.name as name, + features.description as description, + features.type as type, + features.project as project, + features.stale as stale, + feature_environments.variants as variants, + features.impression_data as impression_data, + features.created_at as created_at, + features.last_seen_at as last_seen_at, + features.archived_at as archived_at, + feature_environments.enabled as enabled, + feature_environments.environment as environment, + environments.name as environment_name, + environments.type as environment_type, + environments.sort_order as environment_sort_order, + feature_strategies.id as strategy_id, + feature_strategies.strategy_name as strategy_name, + feature_strategies.parameters as parameters, + feature_strategies.constraints as constraints, + feature_strategies.sort_order as sort_order, + fss.segment_id as segments, + feature_strategies.title as strategy_title, + feature_strategies.disabled as strategy_disabled + FROM + features + LEFT JOIN feature_environments ON feature_environments.feature_name = features.name + LEFT JOIN feature_strategies ON feature_strategies.feature_name = feature_environments.feature_name + and feature_strategies.environment = feature_environments.environment + LEFT JOIN environments ON feature_environments.environment = environments.name + LEFT JOIN feature_strategy_segment as fss ON fss.feature_strategy_id = feature_strategies.id; + `, + callback, + ); +}; + +exports.down = function (db, callback) { + db.runSql( + ` + DROP VIEW features_view; + CREATE VIEW features_view AS + SELECT + features.name as name, + features.description as description, + features.type as type, + features.project as project, + features.stale as stale, + feature_environments.variants as variants, + features.impression_data as impression_data, + features.created_at as created_at, + features.last_seen_at as last_seen_at, + features.archived_at as archived_at, + feature_environments.enabled as enabled, + feature_environments.environment as environment, + environments.name as environment_name, + environments.type as environment_type, + environments.sort_order as environment_sort_order, + feature_strategies.id as strategy_id, + feature_strategies.strategy_name as strategy_name, + feature_strategies.parameters as parameters, + feature_strategies.constraints as constraints, + feature_strategies.sort_order as sort_order, + fss.segment_id as segments + feature_strategies.title as strategy_title + FROM + features + LEFT JOIN feature_environments ON feature_environments.feature_name = features.name + LEFT JOIN feature_strategies ON feature_strategies.feature_name = feature_environments.feature_name + and feature_strategies.environment = feature_environments.environment + LEFT JOIN environments ON feature_environments.environment = environments.name + LEFT JOIN feature_strategy_segment as fss ON fss.feature_strategy_id = feature_strategies.id; + + ALTER TABLE feature_strategies DROP COLUMN IF EXISTS disabled; + `, + callback, + ); +}; diff --git a/src/test/e2e/api/admin/playground.e2e.test.ts b/src/test/e2e/api/admin/playground.e2e.test.ts index d7d5489357f..296429eccb4 100644 --- a/src/test/e2e/api/admin/playground.e2e.test.ts +++ b/src/test/e2e/api/admin/playground.e2e.test.ts @@ -121,7 +121,7 @@ describe('Playground API E2E', () => { // assign strategies await Promise.all( - (feature.strategies || []).map((strategy) => + (feature.strategies || []).map((strategy, index) => database.stores.featureStrategiesStore.createStrategyFeatureEnv( { parameters: {}, @@ -130,6 +130,7 @@ describe('Playground API E2E', () => { featureName: feature.name, environment, strategyName: strategy.name, + disabled: !!(index % 2), projectId: feature.project, }, ), diff --git a/src/test/e2e/api/admin/project/features.e2e.test.ts b/src/test/e2e/api/admin/project/features.e2e.test.ts index 5580a5051d9..fa9b4718960 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/src/test/e2e/api/admin/project/features.e2e.test.ts @@ -2823,3 +2823,102 @@ test('Should batch stale features', async () => { .expect(200); expect(body.stale).toBeTruthy(); }); + +test('should return disabled strategies', async () => { + const toggle = { name: uuidv4(), impressionData: false }; + await app.request + .post('/api/admin/projects/default/features') + .send({ + name: toggle.name, + }) + .expect(201); + + const { body: strategyOne } = await app.request + .post( + `/api/admin/projects/default/features/${toggle.name}/environments/default/strategies`, + ) + .send({ + name: 'default', + parameters: { + userId: 'string', + }, + disabled: true, + }) + .expect(200); + + const { body: strategyTwo } = await app.request + .post( + `/api/admin/projects/default/features/${toggle.name}/environments/default/strategies`, + ) + .send({ + name: 'gradualrollout', + parameters: { + userId: 'string', + }, + }) + .expect(200); + + const { body: strategies } = await app.request.get( + `/api/admin/projects/default/features/${toggle.name}/environments/default/strategies`, + ); + + expect(strategies[0].id).toBe(strategyOne.id); + expect(strategies[0].disabled).toBe(strategyOne.disabled); + expect(strategies[1].id).toBe(strategyTwo.id); + expect(strategies[1].disabled).toBe(strategyTwo.disabled); +}); + +test('should disable strategies in place', async () => { + const toggle = { name: uuidv4(), impressionData: false }; + await app.request + .post('/api/admin/projects/default/features') + .send({ + name: toggle.name, + }) + .expect(201); + + const { body: strategyOne } = await app.request + .post( + `/api/admin/projects/default/features/${toggle.name}/environments/default/strategies`, + ) + .send({ + name: 'flexibleRollout', + constraints: [], + parameters: { + rollout: '100', + stickiness: 'default', + groupId: 'some-new', + }, + }) + .expect(200); + + const { body: strategies } = await app.request.get( + `/api/admin/projects/default/features/${toggle.name}/environments/default/strategies`, + ); + + expect(strategies[0].id).toBe(strategyOne.id); + expect(strategies[0].disabled).toBe(false); + + const { body: updatedStrategyOne } = await app.request + .put( + `/api/admin/projects/default/features/${toggle.name}/environments/default/strategies/${strategyOne.id}`, + ) + .send({ + name: 'flexibleRollout', + constraints: [], + disabled: true, + parameters: { + rollout: '100', + stickiness: 'default', + groupId: 'some-new', + }, + }) + .expect(200); + + const { body: updatedStrategies } = await app.request.get( + `/api/admin/projects/default/features/${toggle.name}/environments/default/strategies`, + ); + + expect(updatedStrategies[0].id).toBe(updatedStrategyOne.id); + expect(updatedStrategies[0].disabled).toBe(updatedStrategyOne.disabled); +}); diff --git a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap index 489a2d73950..b7fe378627e 100644 --- a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap +++ b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap @@ -1373,6 +1373,12 @@ The provider you choose for your addon dictates what properties the \`parameters }, "type": "array", }, + "disabled": { + "description": "A toggle to disable the strategy. defaults to false. Disabled strategies are not evaluated or returned to the SDKs", + "example": false, + "nullable": true, + "type": "boolean", + }, "name": { "description": "The name or type of strategy", "example": "flexibleRollout", @@ -2089,6 +2095,12 @@ The provider you choose for your addon dictates what properties the \`parameters }, "type": "array", }, + "disabled": { + "description": "A toggle to disable the strategy. defaults to false. Disabled strategies are not evaluated or returned to the SDKs", + "example": false, + "nullable": true, + "type": "boolean", + }, "featureName": { "description": "The name or feature the strategy is attached to", "example": "myAwesomeFeature", @@ -3220,6 +3232,12 @@ The provider you choose for your addon dictates what properties the \`parameters }, "type": "array", }, + "disabled": { + "description": "The strategy's status. Disabled strategies are not evaluated", + "example": false, + "nullable": true, + "type": "boolean", + }, "id": { "description": "The strategy's id.", "type": "string", @@ -3315,6 +3333,7 @@ The provider you choose for your addon dictates what properties the \`parameters "segments", "constraints", "parameters", + "disabled", ], "type": "object", }, @@ -4617,6 +4636,12 @@ Stats are divided into current and previous **windows**. }, "type": "array", }, + "disabled": { + "description": "A toggle to disable the strategy. defaults to true. Disabled strategies are not evaluated or returned to the SDKs", + "example": false, + "nullable": true, + "type": "boolean", + }, "name": { "type": "string", }, @@ -4626,6 +4651,12 @@ Stats are divided into current and previous **windows**. "sortOrder": { "type": "number", }, + "title": { + "description": "A descriptive title for the strategy", + "example": "Gradual Rollout 25-Prod", + "nullable": true, + "type": "string", + }, }, "type": "object", }, diff --git a/src/test/e2e/api/proxy/proxy.e2e.test.ts b/src/test/e2e/api/proxy/proxy.e2e.test.ts index 2d8f3ea182b..4244899774e 100644 --- a/src/test/e2e/api/proxy/proxy.e2e.test.ts +++ b/src/test/e2e/api/proxy/proxy.e2e.test.ts @@ -1105,3 +1105,104 @@ test('should not return all features', async () => { }); }); }); + +test('should NOT evaluate disabled strategies when returning toggles', async () => { + const frontendToken = await createApiToken(ApiTokenType.FRONTEND); + await createFeatureToggle({ + name: 'enabledFeature', + enabled: true, + strategies: [ + { + name: 'flexibleRollout', + constraints: [], + parameters: { + rollout: '100', + stickiness: 'default', + groupId: 'some-new', + }, + }, + ], + }); + await createFeatureToggle({ + name: 'disabledFeature', + enabled: false, + strategies: [ + { + name: 'flexibleRollout', + constraints: [], + disabled: true, + parameters: { + rollout: '100', + stickiness: 'default', + groupId: 'some-new', + }, + }, + ], + }); + await createFeatureToggle({ + name: 'disabledFeature3', + enabled: true, + strategies: [ + { + name: 'flexibleRollout', + constraints: [], + disabled: true, + parameters: { + rollout: '100', + stickiness: 'default', + groupId: 'some-new', + }, + }, + { + name: 'flexibleRollout', + constraints: [], + disabled: false, + parameters: { + rollout: '0', + stickiness: 'default', + groupId: 'some-new', + }, + }, + ], + }); + await createFeatureToggle({ + name: 'enabledFeature2', + enabled: true, + strategies: [ + { + name: 'flexibleRollout', + constraints: [], + disabled: true, + parameters: { + rollout: '100', + stickiness: 'default', + groupId: 'some-new', + }, + }, + ], + }); + + await app.request + .get('/api/frontend') + .set('Authorization', frontendToken.secret) + .expect('Content-Type', /json/) + .expect(200) + .expect((res) => { + expect(res.body).toEqual({ + toggles: [ + { + name: 'enabledFeature', + enabled: true, + impressionData: false, + variant: { enabled: false, name: 'disabled' }, + }, + { + name: 'enabledFeature2', + enabled: true, + impressionData: false, + variant: { enabled: false, name: 'disabled' }, + }, + ], + }); + }); +}); diff --git a/src/test/e2e/services/playground-service.test.ts b/src/test/e2e/services/playground-service.test.ts index 1f9c27c8e4a..efb962dede3 100644 --- a/src/test/e2e/services/playground-service.test.ts +++ b/src/test/e2e/services/playground-service.test.ts @@ -709,6 +709,8 @@ describe('the playground service (e2e)', () => { expect.arrayContaining([ { ...strategy, + title: undefined, + disabled: false, constraints: strategy.constraints ?? [], parameters: