diff --git a/src/lib/features/client-feature-toggles/tests/__snapshots__/client-feature-toggles.e2e.test.ts.snap b/src/lib/features/client-feature-toggles/tests/__snapshots__/client-feature-toggles.e2e.test.ts.snap index 99a7ae2ba4..4b824ee976 100644 --- a/src/lib/features/client-feature-toggles/tests/__snapshots__/client-feature-toggles.e2e.test.ts.snap +++ b/src/lib/features/client-feature-toggles/tests/__snapshots__/client-feature-toggles.e2e.test.ts.snap @@ -54,9 +54,7 @@ exports[`should match snapshot from /api/client/features 1`] = ` }, ], "name": "default", - "parameters": { - "stickiness": "default", - }, + "parameters": {}, "variants": [], }, ], diff --git a/src/lib/features/client-feature-toggles/tests/client-feature-toggles.e2e.test.ts b/src/lib/features/client-feature-toggles/tests/client-feature-toggles.e2e.test.ts index bf0a6d076b..5543330533 100644 --- a/src/lib/features/client-feature-toggles/tests/client-feature-toggles.e2e.test.ts +++ b/src/lib/features/client-feature-toggles/tests/client-feature-toggles.e2e.test.ts @@ -70,9 +70,7 @@ const getApiClientResponse = (project = 'default') => [ values: ['123'], }, ], - parameters: { - stickiness: 'default', - }, + parameters: {}, variants: [], }, ], diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index ac69c3b948..adfb6e8f7a 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/src/lib/features/feature-toggle/feature-toggle-service.ts @@ -117,7 +117,7 @@ import { sortStrategies } from '../../util/sortStrategies.js'; import type { ResourceLimitsSchema } from '../../openapi/index.js'; import type FeatureLinkService from '../feature-links/feature-link-service.js'; import type { IFeatureLink } from '../feature-links/feature-links-read-model-type.js'; - +import merge from 'deepmerge'; interface IFeatureContext { featureName: string; projectId: string; @@ -126,6 +126,9 @@ interface IFeatureContext { interface IFeatureStrategyContext extends IFeatureContext { environment: string; } +function mergeAll(objects: Partial[]): T { + return merge.all(objects.filter((i) => i)); +} export interface IGetFeatureParams { featureName: string; @@ -670,7 +673,48 @@ export class FeatureToggleService { ); } - async standardizeStrategyConfig( + private async defaultParameters( + projectId: string, + strategyName: string, + featureName: string, + params: IFeatureStrategy['parameters'] | undefined, + ) { + if (strategyName === 'flexibleRollout') { + if (params?.stickiness === '') { + // If stickiness is an empty string, we remove it to use the default stickiness. + delete params?.stickiness; + } + return { + rollout: '100', + stickiness: + params?.stickiness ?? + (await this.featureStrategiesStore.getDefaultStickiness( + projectId, + )), + groupId: featureName, + }; + } else { + /// We don't really have good defaults for the other kinds of known strategies, so return an empty map. + return {}; + } + } + private async parametersWithDefaults( + projectId: string, + strategyName: string, + featureName: string, + params: IFeatureStrategy['parameters'] | undefined, + ) { + return mergeAll([ + await this.defaultParameters( + projectId, + strategyName, + featureName, + params, + ), + params ?? {}, + ]); + } + private async standardizeStrategyConfig( projectId: string, strategyConfig: Unsaved, existing?: IFeatureStrategy, @@ -695,18 +739,12 @@ export class FeatureToggleService { constraints = await this.validateConstraints(constraints); } - if ( - parameters && - (!('stickiness' in parameters) || - ('stickiness' in parameters && parameters.stickiness === '')) - ) { - parameters.stickiness = - existing?.parameters?.stickiness || - (await this.featureStrategiesStore.getDefaultStickiness( - projectId, - )); - } - + parameters = await this.parametersWithDefaults( + projectId, + name, + strategyConfig.featureName!, + strategyConfig.parameters, + ); if (variants && variants.length > 0) { await variantsArraySchema.validateAsync(variants); const fixedVariants = this.fixVariantWeights(variants); diff --git a/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts b/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts index 91073690cf..1663d25c1d 100644 --- a/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts +++ b/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts @@ -28,7 +28,6 @@ import { import type { IFeatureProjectUserParams } from './feature-toggle-controller.js'; import type { Db } from '../../db/db.js'; import { isAfter } from 'date-fns'; -import merge from 'deepmerge'; import Raw = Knex.Raw; import type { ITag } from '../../tags/index.js'; @@ -157,32 +156,6 @@ function mapStrategyUpdate( return update; } -function mergeAll(objects: Partial[]): T { - return merge.all(objects.filter((i) => i)); -} - -const defaultParameters = ( - params: PartialSome, - stickiness: string, -) => { - if (params.strategyName === 'flexibleRollout') { - return { - rollout: '100', - stickiness, - groupId: params.featureName, - }; - } else { - /// We don't really have good defaults for the other kinds of known strategies, so return an empty map. - return {}; - } -}; - -const parametersWithDefaults = ( - params: PartialSome, - stickiness: string, -) => { - return mergeAll([defaultParameters(params, stickiness), params.parameters]); -}; class FeatureStrategiesStore implements IFeatureStrategiesStore { private db: Db; @@ -264,13 +237,6 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { strategyConfig.featureName, strategyConfig.environment, )); - const stickiness = - strategyConfig.parameters?.stickiness ?? - (await this.getDefaultStickiness(strategyConfig.projectId)); - strategyConfig.parameters = parametersWithDefaults( - strategyConfig, - stickiness, - ); const strategyRow = mapInput({ id: uuidv4(), ...strategyConfig,