From 6412e894f4f0cd4c6f9abb2217ee4ae8f9251b43 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Thu, 24 Jul 2025 23:08:28 +0600 Subject: [PATCH 01/18] decision service holdout implementation --- lib/core/decision_service/index.ts | 125 ++++++++++++++++++++++++++++- lib/notification_center/type.ts | 4 +- lib/shared_types.ts | 2 +- lib/utils/enums/index.ts | 1 + 4 files changed, 125 insertions(+), 7 deletions(-) diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 70673d68e..b8cb499f4 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -33,6 +33,7 @@ import { isActive, ProjectConfig, getTrafficAllocation, + getHoldoutsForFlag, } from '../../project_config/project_config'; import { AudienceEvaluator, createAudienceEvaluator } from '../audience_evaluator'; import * as stringValidator from '../../utils/string_value_validator'; @@ -41,7 +42,9 @@ import { DecisionResponse, Experiment, ExperimentBucketMap, + ExperimentCore, FeatureFlag, + Holdout, OptimizelyDecideOption, OptimizelyUserContext, TrafficAllocation, @@ -75,6 +78,7 @@ import { OptimizelyError } from '../../error/optimizly_error'; import { CmabService } from './cmab/cmab_service'; import { Maybe, OpType, OpValue } from '../../utils/type'; import { Value } from '../../utils/promise/operation_value'; +import { holdout } from '../../feature_toggle'; export const EXPERIMENT_NOT_RUNNING = 'Experiment %s is not running.'; export const RETURNING_STORED_VARIATION = @@ -112,9 +116,14 @@ export const USER_HAS_FORCED_DECISION_WITH_NO_RULE_SPECIFIED_BUT_INVALID = export const CMAB_NOT_SUPPORTED_IN_SYNC = 'CMAB is not supported in sync mode.'; export const CMAB_FETCH_FAILED = 'Failed to fetch CMAB data for experiment %s.'; export const CMAB_FETCHED_VARIATION_INVALID = 'Fetched variation %s for cmab experiment %s is invalid.'; +export const HOLDOUT_NOT_RUNNING = 'Holdout %s is not running.'; +export const USER_MEETS_CONDITIONS_FOR_HOLDOUT = 'User %s meets conditions for holdout %s.'; +export const USER_DOESNT_MEET_CONDITIONS_FOR_HOLDOUT = 'User %s does not meet conditions for holdout %s.'; +export const USER_BUCKETED_INTO_HOLDOUT_VARIATION = 'User %s is in variation %s of holdout %s.'; +export const USER_NOT_BUCKETED_INTO_HOLDOUT_VARIATION = 'User %s is in no holdout variation.'; export interface DecisionObj { - experiment: Experiment | null; + experiment: ExperimentCore | null; variation: Variation | null; decisionSource: DecisionSource; cmabUuid?: string; @@ -540,7 +549,7 @@ export class DecisionService { */ private checkIfUserIsInAudience( configObj: ProjectConfig, - experiment: Experiment, + experiment: ExperimentCore, evaluationAttribute: string, user: OptimizelyUserContext, loggingKey?: string | number, @@ -590,14 +599,14 @@ export class DecisionService { */ private buildBucketerParams( configObj: ProjectConfig, - experiment: Experiment, + experiment: Experiment | Holdout, bucketingId: string, userId: string ): BucketerParams { let validateEntity = true; let trafficAllocationConfig: TrafficAllocation[] = getTrafficAllocation(configObj, experiment.id); - if (experiment.cmab) { + if ('cmab' in experiment && experiment.cmab) { trafficAllocationConfig = [{ entityId: CMAB_DUMMY_ENTITY_ID, endOfRange: experiment.cmab.trafficAllocation @@ -621,6 +630,99 @@ export class DecisionService { } } + /** + * Determines if a user should be bucketed into a holdout variation. + * @param {ProjectConfig} configObj - The parsed project configuration object. + * @param {Holdout} holdout - The holdout to evaluate. + * @param {OptimizelyUserContext} user - The user context. + * @returns {DecisionResponse} - DecisionResponse containing holdout decision and reasons. + */ + private getVariationForHoldout( + configObj: ProjectConfig, + holdout: Holdout, + user: OptimizelyUserContext, + ): DecisionResponse { + const userId = user.getUserId(); + const decideReasons: DecisionReason[] = []; + + if (holdout.status !== 'Running') { + const reason: DecisionReason = [HOLDOUT_NOT_RUNNING, holdout.key]; + decideReasons.push(reason); + this.logger?.info(HOLDOUT_NOT_RUNNING, holdout.key); + return { + result: { + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.HOLDOUT + }, + reasons: decideReasons + }; + } + + const audienceResult = this.checkIfUserIsInAudience( + configObj, + holdout, + AUDIENCE_EVALUATION_TYPES.EXPERIMENT, + user + ); + decideReasons.push(...audienceResult.reasons); + + if (!audienceResult.result) { + const reason: DecisionReason = [USER_DOESNT_MEET_CONDITIONS_FOR_HOLDOUT, userId, holdout.key]; + decideReasons.push(reason); + this.logger?.info(USER_DOESNT_MEET_CONDITIONS_FOR_HOLDOUT, userId, holdout.key); + return { + result: { + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.HOLDOUT + }, + reasons: decideReasons + }; + } + + const reason: DecisionReason = [USER_MEETS_CONDITIONS_FOR_HOLDOUT, userId, holdout.key]; + decideReasons.push(reason); + this.logger?.info(USER_MEETS_CONDITIONS_FOR_HOLDOUT, userId, holdout.key); + + const attributes = user.getAttributes(); + const bucketingId = this.getBucketingId(userId, attributes); + const bucketerParams = this.buildBucketerParams(configObj, holdout, bucketingId, userId); + const bucketResult = bucket(bucketerParams); + + decideReasons.push(...bucketResult.reasons); + + if (bucketResult.result) { + const variation = configObj.variationIdMap[bucketResult.result]; + if (variation) { + const bucketReason: DecisionReason = [USER_BUCKETED_INTO_HOLDOUT_VARIATION, userId, holdout.key, variation.key]; + decideReasons.push(bucketReason); + this.logger?.info(USER_BUCKETED_INTO_HOLDOUT_VARIATION, userId, holdout.key, variation.key); + + return { + result: { + experiment: holdout, + variation: variation, + decisionSource: DECISION_SOURCES.HOLDOUT + }, + reasons: decideReasons + }; + } + } + + const noBucketReason: DecisionReason = [USER_NOT_BUCKETED_INTO_HOLDOUT_VARIATION, userId]; + decideReasons.push(noBucketReason); + this.logger?.info(USER_NOT_BUCKETED_INTO_HOLDOUT_VARIATION, userId); + return { + result: { + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.HOLDOUT + }, + reasons: decideReasons + }; + } + /** * Pull the stored variation out of the experimentBucketMap for an experiment/userId * @param {ProjectConfig} configObj The parsed project configuration object @@ -836,6 +938,21 @@ export class DecisionService { }); } + if (holdout()) { + const holdouts = getHoldoutsForFlag(configObj, feature.id); + for (const holdout of holdouts) { + const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); + decideReasons.push(...holdoutDecision.reasons); + + if (holdoutDecision.result.variation) { + return Value.of(op, { + result: holdoutDecision.result, + reasons: decideReasons, + }); + } + } + } + return this.getVariationForFeatureExperiment(op, configObj, feature, user, decideOptions, userProfileTracker).then((experimentDecision) => { if (experimentDecision.error || experimentDecision.result.variation !== null) { return Value.of(op, { diff --git a/lib/notification_center/type.ts b/lib/notification_center/type.ts index b433c0121..32652881e 100644 --- a/lib/notification_center/type.ts +++ b/lib/notification_center/type.ts @@ -15,7 +15,7 @@ */ import { LogEvent } from '../event_processor/event_dispatcher/event_dispatcher'; -import { EventTags, Experiment, FeatureVariableValue, UserAttributes, VariableType, Variation } from '../shared_types'; +import { EventTags, ExperimentCore, FeatureVariableValue, UserAttributes, VariableType, Variation } from '../shared_types'; import { DecisionSource } from '../utils/enums'; import { Nullable } from '../utils/type'; @@ -25,7 +25,7 @@ export type UserEventListenerPayload = { } export type ActivateListenerPayload = UserEventListenerPayload & { - experiment: Experiment | null; + experiment: ExperimentCore | null; variation: Variation | null; logEvent: LogEvent; } diff --git a/lib/shared_types.ts b/lib/shared_types.ts index 3d3492a2c..7c2046bf6 100644 --- a/lib/shared_types.ts +++ b/lib/shared_types.ts @@ -358,7 +358,7 @@ export interface Client { } export interface ActivateListenerPayload extends ListenerPayload { - experiment: import('./shared_types').Experiment; + experiment: import('./shared_types').ExperimentCore; variation: import('./shared_types').Variation; logEvent: Event; } diff --git a/lib/utils/enums/index.ts b/lib/utils/enums/index.ts index 892bff837..103cdac73 100644 --- a/lib/utils/enums/index.ts +++ b/lib/utils/enums/index.ts @@ -53,6 +53,7 @@ export const DECISION_SOURCES = { FEATURE_TEST: 'feature-test', ROLLOUT: 'rollout', EXPERIMENT: 'experiment', + HOLDOUT: 'holdout', } as const; export type DecisionSource = typeof DECISION_SOURCES[keyof typeof DECISION_SOURCES]; From 3a90a81c8792e76205e074e82451a85ec610662e Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Fri, 25 Jul 2025 18:41:04 +0600 Subject: [PATCH 02/18] [FSSDK-11528] audience condition improvement --- lib/core/decision_service/index.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index b8cb499f4..88038593d 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -555,8 +555,9 @@ export class DecisionService { loggingKey?: string | number, ): DecisionResponse { const decideReasons: DecisionReason[] = []; - const experimentAudienceConditions = getExperimentAudienceConditions(configObj, experiment.id); + const experimentAudienceConditions = experiment.audienceConditions || experiment.audienceIds; const audiencesById = getAudiencesById(configObj); + this.logger?.debug( EVALUATING_AUDIENCES_COMBINED, evaluationAttribute, @@ -569,7 +570,9 @@ export class DecisionService { loggingKey || experiment.key, JSON.stringify(experimentAudienceConditions), ]); + const result = this.audienceEvaluator.evaluate(experimentAudienceConditions, audiencesById, user); + this.logger?.info( AUDIENCE_EVALUATION_RESULT_COMBINED, evaluationAttribute, From f6c4cda44999f7c475a5ff45fc168605cd08fcab Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Fri, 25 Jul 2025 21:15:01 +0600 Subject: [PATCH 03/18] [FSSDK-11528] decision service tests added --- lib/core/decision_service/index.spec.ts | 428 +++++++++++++++++++++++- lib/core/decision_service/index.ts | 7 +- lib/project_config/project_config.ts | 6 + 3 files changed, 434 insertions(+), 7 deletions(-) diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index 975653611..d8db928f0 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { describe, it, expect, vi, MockInstance, beforeEach } from 'vitest'; +import { describe, it, expect, vi, MockInstance, beforeEach, afterEach } from 'vitest'; import { CMAB_DUMMY_ENTITY_ID, CMAB_FETCH_FAILED, DecisionService } from '.'; import { getMockLogger } from '../../tests/mock/mock_logger'; import OptimizelyUserContext from '../../optimizely_user_context'; @@ -24,14 +24,12 @@ import { BucketerParams, Experiment, OptimizelyDecideOption, UserAttributes, Use import { CONTROL_ATTRIBUTES, DECISION_SOURCES } from '../../utils/enums'; import { getDecisionTestDatafile } from '../../tests/decision_test_datafile'; import { Value } from '../../utils/promise/operation_value'; - import { USER_HAS_NO_FORCED_VARIATION, VALID_BUCKETING_ID, SAVED_USER_VARIATION, SAVED_VARIATION_NOT_FOUND, } from 'log_message'; - import { EXPERIMENT_NOT_RUNNING, RETURNING_STORED_VARIATION, @@ -48,7 +46,6 @@ import { NO_ROLLOUT_EXISTS, USER_MEETS_CONDITIONS_FOR_TARGETING_RULE, } from '../decision_service/index'; - import { BUCKETING_ID_NOT_STRING, USER_PROFILE_LOOKUP_ERROR, USER_PROFILE_SAVE_ERROR } from 'error_message'; type MockLogger = ReturnType; @@ -117,11 +114,118 @@ vi.mock('../bucketer', () => ({ bucket: mockBucket, })); +// Mock the feature toggle for holdout tests +const mockHoldoutToggle = vi.hoisted(() => vi.fn()); + +vi.mock('../../feature_toggle', () => ({ + holdout: mockHoldoutToggle, +})); + + const cloneDeep = (d: any) => JSON.parse(JSON.stringify(d)); const testData = getTestProjectConfig(); const testDataWithFeatures = getTestProjectConfigWithFeatures(); +// Utility function to create test datafile with holdout configurations +const getHoldoutTestDatafile = () => { + const datafile = getDecisionTestDatafile(); + + // Add holdouts to the datafile + datafile.holdouts = [ + { + id: 'holdout_running_id', + key: 'holdout_running', + status: 'Running', + includeFlags: [], + excludeFlags: [], + audienceIds: ['4001'], // age_22 audience + audienceConditions: ['or', '4001'], + variations: [ + { + id: 'holdout_variation_running_id', + key: 'holdout_variation_running', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_running_id', + endOfRange: 5000 + } + ] + }, + { + id: 'holdout_not_running_id', + key: 'holdout_not_running', + status: 'Draft', + includeFlags: [], + excludeFlags: [], + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'holdout_variation_not_running_id', + key: 'holdout_variation_not_running', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_not_running_id', + endOfRange: 5000 + } + ] + }, + { + id: 'holdout_excluded_id', + key: 'holdout_excluded', + status: 'Running', + includeFlags: [], + excludeFlags: ['flag_1'], + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'holdout_variation_excluded_id', + key: 'holdout_variation_excluded', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_excluded_id', + endOfRange: 5000 + } + ] + }, + { + id: 'holdout_no_traffic_id', + key: 'holdout_no_traffic', + status: 'Running', + includeFlags: [], + excludeFlags: [], + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'holdout_variation_no_traffic_id', + key: 'holdout_variation_no_traffic', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_no_traffic_id', + endOfRange: 0 // No traffic allocation + } + ] + } + ]; + + return datafile; +}; + const verifyBucketCall = ( call: number, projectConfig: ProjectConfig, @@ -1841,6 +1945,322 @@ describe('DecisionService', () => { expect(userProfileServiceAsync?.lookup).not.toHaveBeenCalled(); expect(userProfileServiceAsync?.save).not.toHaveBeenCalled(); }); + + describe('holdout', () => { + beforeEach(() => { + mockHoldoutToggle.mockReturnValue(true); + }); + + it('should return holdout variation when user is bucketed into running holdout', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(getHoldoutTestDatafile()); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 20, + }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey === 'holdout_running') { + return { + result: 'holdout_variation_running_id', + reasons: [], + }; + } + return { + result: null, + reasons: [], + }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + + it('should fallback to experiment when holdout status is not running', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(getHoldoutTestDatafile()); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 15, + }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey === 'exp_2') { + return { + result: '5002', + reasons: [], + }; + } + return { + result: null, + reasons: [], + }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['exp_2'], + variation: config.variationIdMap['5002'], + decisionSource: DECISION_SOURCES.FEATURE_TEST, + }); + }); + + it('should fallback to experiment when user does not meet holdout audience conditions', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(getHoldoutTestDatafile()); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 30, // does not satisfy age_22 audience condition for holdout_running + }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey === 'exp_2') { + return { + result: '5002', + reasons: [], + }; + } + return { + result: null, + reasons: [], + }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['exp_2'], + variation: config.variationIdMap['5002'], + decisionSource: DECISION_SOURCES.FEATURE_TEST, + }); + }); + + it('should fallback to experiment when user is not bucketed into holdout traffic', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(getHoldoutTestDatafile()); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 15, // should satisfy audience condition for experiments + }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey === 'holdout_no_traffic') { + return { + result: null, // Not bucketed due to 0% traffic allocation + reasons: [], + }; + } + if (ruleKey === 'exp_2') { + return { + result: '5002', + reasons: [], + }; + } + return { + result: null, + reasons: [], + }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['exp_2'], + variation: config.variationIdMap['5002'], + decisionSource: DECISION_SOURCES.FEATURE_TEST, + }); + }); + + it('should fallback to rollout when no holdout or experiment matches', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(getHoldoutTestDatafile()); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 55, // satisfies audience for targeted delivery + }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey === 'delivery_2') { + return { + result: '5005', + reasons: [], + }; + } + return { + result: null, + reasons: [], + }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['delivery_2'], + variation: config.variationIdMap['5005'], + decisionSource: DECISION_SOURCES.ROLLOUT, + }); + }); + + it('should skip holdouts excluded for specific flags', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(getHoldoutTestDatafile()); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 15, // should satisfy audience condition for experiments + }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey === 'exp_2') { + return { + result: '5002', + reasons: [], + }; + } + return { + result: null, + reasons: [], + }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['exp_2'], + variation: config.variationIdMap['5002'], + decisionSource: DECISION_SOURCES.FEATURE_TEST, + }); + }); + + it('should handle multiple holdouts and use first matching one', async () => { + const { decisionService } = getDecisionService(); + const datafile = getHoldoutTestDatafile(); + + datafile.holdouts.push({ + id: 'holdout_second_id', + key: 'holdout_second', + status: 'Running', + includeFlags: [], + excludeFlags: [], + audienceIds: [], // no audience requirements + audienceConditions: [], + variations: [ + { + id: 'holdout_variation_second_id', + key: 'holdout_variation_second', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_second_id', + endOfRange: 5000 + } + ] + }); + + const config = createProjectConfig(datafile); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 20, // satisfies audience for holdout_running + }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey === 'holdout_running') { + return { + result: 'holdout_variation_running_id', + reasons: [], + }; + } + if (ruleKey === 'holdout_second') { + return { + result: 'holdout_variation_second_id', + reasons: [], + }; + } + return { + result: null, + reasons: [], + }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + }); }); describe('resolveVariationForFeatureList - sync', () => { diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 88038593d..8c20381df 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -608,7 +608,8 @@ export class DecisionService { ): BucketerParams { let validateEntity = true; - let trafficAllocationConfig: TrafficAllocation[] = getTrafficAllocation(configObj, experiment.id); + let trafficAllocationConfig = experiment.trafficAllocation; + if ('cmab' in experiment && experiment.cmab) { trafficAllocationConfig = [{ entityId: CMAB_DUMMY_ENTITY_ID, @@ -940,13 +941,13 @@ export class DecisionService { reasons: decideReasons, }); } - if (holdout()) { const holdouts = getHoldoutsForFlag(configObj, feature.id); + for (const holdout of holdouts) { const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); decideReasons.push(...holdoutDecision.reasons); - + if (holdoutDecision.result.variation) { return Value.of(op, { result: holdoutDecision.result, diff --git a/lib/project_config/project_config.ts b/lib/project_config/project_config.ts index 7ae95e3e9..1b14e4408 100644 --- a/lib/project_config/project_config.ts +++ b/lib/project_config/project_config.ts @@ -370,6 +370,12 @@ const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => { } holdout.variationKeyMap = keyBy(holdout.variations, 'key'); + + projectConfig.variationIdMap = { + ...projectConfig.variationIdMap, + ...keyBy(holdout.variations, 'id'), + }; + if (holdout.includeFlags.length === 0) { projectConfig.globalHoldouts.push(holdout); From 9a772409d1f40c118f86e29b674de3dcf763b3d6 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Fri, 25 Jul 2025 21:37:24 +0600 Subject: [PATCH 04/18] [FSSDK-11528] decision service tests improvement --- lib/core/bucketer/index.ts | 2 +- lib/core/decision_service/index.spec.ts | 19 +++---------------- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/lib/core/bucketer/index.ts b/lib/core/bucketer/index.ts index b5a5e58c6..c6ed3813a 100644 --- a/lib/core/bucketer/index.ts +++ b/lib/core/bucketer/index.ts @@ -56,7 +56,7 @@ export const bucket = function(bucketerParams: BucketerParams): DecisionResponse const decideReasons: DecisionReason[] = []; // Check if user is in a random group; if so, check if user is bucketed into a specific experiment const experiment = bucketerParams.experimentIdMap[bucketerParams.experimentId]; - const groupId = experiment['groupId']; + const groupId = experiment?.['groupId']; if (groupId) { const group = bucketerParams.groupIdMap[groupId]; if (!group) { diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index d8db928f0..22c42c59e 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -1947,8 +1947,10 @@ describe('DecisionService', () => { }); describe('holdout', () => { - beforeEach(() => { + beforeEach(async() => { mockHoldoutToggle.mockReturnValue(true); + const actualBucketModule = (await vi.importActual('../bucketer')) as { bucket: typeof bucket }; + mockBucket.mockImplementation(actualBucketModule.bucket); }); it('should return holdout variation when user is bucketed into running holdout', async () => { @@ -1961,21 +1963,6 @@ describe('DecisionService', () => { age: 20, }, }); - - mockBucket.mockImplementation((param: BucketerParams) => { - const ruleKey = param.experimentKey; - if (ruleKey === 'holdout_running') { - return { - result: 'holdout_variation_running_id', - reasons: [], - }; - } - return { - result: null, - reasons: [], - }; - }); - const feature = config.featureKeyMap['flag_1']; const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); From b8eac89c504b89000f1959de492121bbba948190 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Mon, 28 Jul 2025 21:34:41 +0600 Subject: [PATCH 05/18] [FSSDK-11528] test improvement + decision service logic adjustment --- lib/core/decision_service/index.spec.ts | 330 ++++++++++++------------ lib/core/decision_service/index.ts | 2 +- 2 files changed, 170 insertions(+), 162 deletions(-) diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index 22c42c59e..3e6910f0f 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -156,71 +156,27 @@ const getHoldoutTestDatafile = () => { ] }, { - id: 'holdout_not_running_id', - key: 'holdout_not_running', - status: 'Draft', + id: "holdout_not_bucketed_id", + key: "holdout_not_bucketed", + status: "Running", includeFlags: [], excludeFlags: [], - audienceIds: [], - audienceConditions: [], + audienceIds: ['4002'], + audienceConditions: ['or', '4002'], variations: [ { - id: 'holdout_variation_not_running_id', - key: 'holdout_variation_not_running', + id: 'holdout_not_bucketed_variation_id', + key: 'holdout_not_bucketed_variation', variables: [] } ], trafficAllocation: [ { - entityId: 'holdout_variation_not_running_id', - endOfRange: 5000 - } - ] - }, - { - id: 'holdout_excluded_id', - key: 'holdout_excluded', - status: 'Running', - includeFlags: [], - excludeFlags: ['flag_1'], - audienceIds: [], - audienceConditions: [], - variations: [ - { - id: 'holdout_variation_excluded_id', - key: 'holdout_variation_excluded', - variables: [] - } - ], - trafficAllocation: [ - { - entityId: 'holdout_variation_excluded_id', - endOfRange: 5000 + entityId: 'holdout_not_bucketed_variation_id', + endOfRange: 0, } ] }, - { - id: 'holdout_no_traffic_id', - key: 'holdout_no_traffic', - status: 'Running', - includeFlags: [], - excludeFlags: [], - audienceIds: [], - audienceConditions: [], - variations: [ - { - id: 'holdout_variation_no_traffic_id', - key: 'holdout_variation_no_traffic', - variables: [] - } - ], - trafficAllocation: [ - { - entityId: 'holdout_variation_no_traffic_id', - endOfRange: 0 // No traffic allocation - } - ] - } ]; return datafile; @@ -1977,31 +1933,126 @@ describe('DecisionService', () => { }); }); - it('should fallback to experiment when holdout status is not running', async () => { + it("should consider global holdout even if local holdout is present", async () => { const { decisionService } = getDecisionService(); - const config = createProjectConfig(getHoldoutTestDatafile()); + const datafile = getHoldoutTestDatafile(); + + datafile.holdouts.push({ + id: 'holdout_included_id', + key: 'holdout_included', + status: 'Running', + includeFlags: ['flag_1'], + excludeFlags: [], + audienceIds: ['4002'], // age_40 audience + audienceConditions: ['or', '4002'], + variations: [ + { + id: 'holdout_variation_included_id', + key: 'holdout_variation_included', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_included_id', + endOfRange: 5000 + } + ] + }); + + const config = createProjectConfig(datafile); const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'tester', attributes: { - age: 15, + age: 20, // satisfies both global holdout (age_22) and included holdout (age_40) audiences }, }); - mockBucket.mockImplementation((param: BucketerParams) => { - const ruleKey = param.experimentKey; - if (ruleKey === 'exp_2') { + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + + it("should consider local holdout if misses global holdout", async () => { + const { decisionService } = getDecisionService(); + const datafile = getHoldoutTestDatafile(); + + datafile.holdouts.push({ + id: 'holdout_included_specific_id', + key: 'holdout_included_specific', + status: 'Running', + includeFlags: ['flag_1'], + excludeFlags: [], + audienceIds: ['4002'], // age_60 audience (age <= 60) + audienceConditions: ['or', '4002'], + variations: [ + { + id: 'holdout_variation_included_specific_id', + key: 'holdout_variation_included_specific', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_included_specific_id', + endOfRange: 5000 + } + ] + }); + const config = createProjectConfig(datafile); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'test_holdout_user', + attributes: { + age: 50, // Does not satisfy global holdout (age_22, age <= 22) but satisfies included holdout (age_60, age <= 60) + }, + }); + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_included_specific_id'], + variation: config.variationIdMap['holdout_variation_included_specific_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + + it('should fallback to experiment when holdout status is not running', async () => { + const { decisionService } = getDecisionService(); + const datafile = getHoldoutTestDatafile(); + + datafile.holdouts = datafile.holdouts.map(holdout => { + if(holdout.id === 'holdout_running_id') { return { - result: '5002', - reasons: [], - }; + ...holdout, + status: "Draft" + } } - return { - result: null, - reasons: [], - }; + return holdout; }); + const config = createProjectConfig(datafile); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 15, + }, + }); const feature = config.featureKeyMap['flag_1']; const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); @@ -2010,8 +2061,8 @@ describe('DecisionService', () => { const variation = (await value)[0]; expect(variation.result).toEqual({ - experiment: config.experimentKeyMap['exp_2'], - variation: config.variationIdMap['5002'], + experiment: config.experimentKeyMap['exp_1'], + variation: config.variationIdMap['5001'], decisionSource: DECISION_SOURCES.FEATURE_TEST, }); }); @@ -2026,21 +2077,6 @@ describe('DecisionService', () => { age: 30, // does not satisfy age_22 audience condition for holdout_running }, }); - - mockBucket.mockImplementation((param: BucketerParams) => { - const ruleKey = param.experimentKey; - if (ruleKey === 'exp_2') { - return { - result: '5002', - reasons: [], - }; - } - return { - result: null, - reasons: [], - }; - }); - const feature = config.featureKeyMap['flag_1']; const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); @@ -2062,30 +2098,9 @@ describe('DecisionService', () => { optimizely: {} as any, userId: 'tester', attributes: { - age: 15, // should satisfy audience condition for experiments + age: 50, }, }); - - mockBucket.mockImplementation((param: BucketerParams) => { - const ruleKey = param.experimentKey; - if (ruleKey === 'holdout_no_traffic') { - return { - result: null, // Not bucketed due to 0% traffic allocation - reasons: [], - }; - } - if (ruleKey === 'exp_2') { - return { - result: '5002', - reasons: [], - }; - } - return { - result: null, - reasons: [], - }; - }); - const feature = config.featureKeyMap['flag_1']; const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); @@ -2102,29 +2117,45 @@ describe('DecisionService', () => { it('should fallback to rollout when no holdout or experiment matches', async () => { const { decisionService } = getDecisionService(); - const config = createProjectConfig(getHoldoutTestDatafile()); - const user = new OptimizelyUserContext({ - optimizely: {} as any, - userId: 'tester', - attributes: { - age: 55, // satisfies audience for targeted delivery - }, + const datafile = getHoldoutTestDatafile(); + // Modify the datafile to create proper audience conditions for this test + // Make exp_1 and exp_2 use age conditions that won't match our test user + datafile.audiences = datafile.audiences.map(audience => { + if (audience.id === '4001') { // age_22 + return { + ...audience, + conditions: JSON.stringify(["or", {"match": "exact", "name": "age", "type": "custom_attribute", "value": 22}]) + }; + } + if (audience.id === '4002') { // age_60 + return { + ...audience, + conditions: JSON.stringify(["or", {"match": "exact", "name": "age", "type": "custom_attribute", "value": 60}]) + }; + } + return audience; }); - - mockBucket.mockImplementation((param: BucketerParams) => { - const ruleKey = param.experimentKey; - if (ruleKey === 'delivery_2') { + + // Make exp_2 use a different audience so it won't conflict with delivery_2 + datafile.experiments = datafile.experiments.map(experiment => { + if (experiment.key === 'exp_2') { return { - result: '5005', - reasons: [], + ...experiment, + audienceIds: ['4001'], // Change from 4002 to 4001 (age_22) + audienceConditions: ['or', '4001'] }; } - return { - result: null, - reasons: [], - }; + return experiment; }); + const config = createProjectConfig(datafile); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 60, // matches audience 4002 (age_60) used by delivery_2, but not experiments + }, + }); const feature = config.featureKeyMap['flag_1']; const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); @@ -2141,29 +2172,26 @@ describe('DecisionService', () => { it('should skip holdouts excluded for specific flags', async () => { const { decisionService } = getDecisionService(); - const config = createProjectConfig(getHoldoutTestDatafile()); + const datafile = getHoldoutTestDatafile(); + + datafile.holdouts = datafile.holdouts.map(holdout => { + if(holdout.id === 'holdout_running_id') { + return { + ...holdout, + excludeFlags: ['flag_1'] + } + } + return holdout; + }); + + const config = createProjectConfig(datafile); const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'tester', attributes: { - age: 15, // should satisfy audience condition for experiments + age: 15, // satisfies age_22 audience condition (age <= 22) for global holdout, but holdout excludes flag_1 }, }); - - mockBucket.mockImplementation((param: BucketerParams) => { - const ruleKey = param.experimentKey; - if (ruleKey === 'exp_2') { - return { - result: '5002', - reasons: [], - }; - } - return { - result: null, - reasons: [], - }; - }); - const feature = config.featureKeyMap['flag_1']; const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); @@ -2172,8 +2200,8 @@ describe('DecisionService', () => { const variation = (await value)[0]; expect(variation.result).toEqual({ - experiment: config.experimentKeyMap['exp_2'], - variation: config.variationIdMap['5002'], + experiment: config.experimentKeyMap['exp_1'], + variation: config.variationIdMap['5001'], decisionSource: DECISION_SOURCES.FEATURE_TEST, }); }); @@ -2214,26 +2242,6 @@ describe('DecisionService', () => { }, }); - mockBucket.mockImplementation((param: BucketerParams) => { - const ruleKey = param.experimentKey; - if (ruleKey === 'holdout_running') { - return { - result: 'holdout_variation_running_id', - reasons: [], - }; - } - if (ruleKey === 'holdout_second') { - return { - result: 'holdout_variation_second_id', - reasons: [], - }; - } - return { - result: null, - reasons: [], - }; - }); - const feature = config.featureKeyMap['flag_1']; const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 8c20381df..61bf54e89 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -942,7 +942,7 @@ export class DecisionService { }); } if (holdout()) { - const holdouts = getHoldoutsForFlag(configObj, feature.id); + const holdouts = getHoldoutsForFlag(configObj, feature.key); for (const holdout of holdouts) { const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); From 4c886484f890d06538c61dfd7249bbb4c89a70f4 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Mon, 28 Jul 2025 21:38:59 +0600 Subject: [PATCH 06/18] [FSSDK-11528] feature toggle readibility improvement --- lib/core/decision_service/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 61bf54e89..1636ca7f1 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -78,7 +78,7 @@ import { OptimizelyError } from '../../error/optimizly_error'; import { CmabService } from './cmab/cmab_service'; import { Maybe, OpType, OpValue } from '../../utils/type'; import { Value } from '../../utils/promise/operation_value'; -import { holdout } from '../../feature_toggle'; +import * as featureToggle from '../../feature_toggle'; export const EXPERIMENT_NOT_RUNNING = 'Experiment %s is not running.'; export const RETURNING_STORED_VARIATION = @@ -941,7 +941,7 @@ export class DecisionService { reasons: decideReasons, }); } - if (holdout()) { + if (featureToggle.holdout()) { const holdouts = getHoldoutsForFlag(configObj, feature.key); for (const holdout of holdouts) { From aa3381ec56dc435250883ca972201a54bdd1c206 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Mon, 28 Jul 2025 21:43:06 +0600 Subject: [PATCH 07/18] [FSSDK-11528] type def update --- lib/core/decision_service/index.spec.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index 3e6910f0f..2380407ed 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -20,7 +20,7 @@ import OptimizelyUserContext from '../../optimizely_user_context'; import { bucket } from '../bucketer'; import { getTestProjectConfig, getTestProjectConfigWithFeatures } from '../../tests/test_data'; import { createProjectConfig, ProjectConfig } from '../../project_config/project_config'; -import { BucketerParams, Experiment, OptimizelyDecideOption, UserAttributes, UserProfile } from '../../shared_types'; +import { BucketerParams, Experiment, Holdout, OptimizelyDecideOption, UserAttributes, UserProfile } from '../../shared_types'; import { CONTROL_ATTRIBUTES, DECISION_SOURCES } from '../../utils/enums'; import { getDecisionTestDatafile } from '../../tests/decision_test_datafile'; import { Value } from '../../utils/promise/operation_value'; @@ -2035,7 +2035,7 @@ describe('DecisionService', () => { const { decisionService } = getDecisionService(); const datafile = getHoldoutTestDatafile(); - datafile.holdouts = datafile.holdouts.map(holdout => { + datafile.holdouts = datafile.holdouts.map((holdout: Holdout) => { if(holdout.id === 'holdout_running_id') { return { ...holdout, @@ -2120,7 +2120,7 @@ describe('DecisionService', () => { const datafile = getHoldoutTestDatafile(); // Modify the datafile to create proper audience conditions for this test // Make exp_1 and exp_2 use age conditions that won't match our test user - datafile.audiences = datafile.audiences.map(audience => { + datafile.audiences = datafile.audiences.map((audience: any) => { if (audience.id === '4001') { // age_22 return { ...audience, @@ -2137,7 +2137,7 @@ describe('DecisionService', () => { }); // Make exp_2 use a different audience so it won't conflict with delivery_2 - datafile.experiments = datafile.experiments.map(experiment => { + datafile.experiments = datafile.experiments.map((experiment: any) => { if (experiment.key === 'exp_2') { return { ...experiment, @@ -2174,7 +2174,7 @@ describe('DecisionService', () => { const { decisionService } = getDecisionService(); const datafile = getHoldoutTestDatafile(); - datafile.holdouts = datafile.holdouts.map(holdout => { + datafile.holdouts = datafile.holdouts.map((holdout: any) => { if(holdout.id === 'holdout_running_id') { return { ...holdout, From 0d034b2c776cc69fde4b5b72bbcbd725054b9658 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Tue, 29 Jul 2025 00:17:22 +0600 Subject: [PATCH 08/18] [FSSDK-11528] test improvement --- lib/core/decision_service/index.spec.ts | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index 2380407ed..93537bba6 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -1933,11 +1933,10 @@ describe('DecisionService', () => { }); }); - it("should consider global holdout even if local holdout is present", async () => { + it.only("should consider global holdout even if local holdout is present", async () => { const { decisionService } = getDecisionService(); const datafile = getHoldoutTestDatafile(); - - datafile.holdouts.push({ + const newEntry = { id: 'holdout_included_id', key: 'holdout_included', status: 'Running', @@ -1949,17 +1948,17 @@ describe('DecisionService', () => { { id: 'holdout_variation_included_id', key: 'holdout_variation_included', - variables: [] - } + variables: [], + }, ], trafficAllocation: [ { entityId: 'holdout_variation_included_id', - endOfRange: 5000 - } - ] - }); - + endOfRange: 5000, + }, + ], + }; + datafile.holdouts = [newEntry, ...datafile.holdouts]; const config = createProjectConfig(datafile); const user = new OptimizelyUserContext({ optimizely: {} as any, @@ -1968,7 +1967,6 @@ describe('DecisionService', () => { age: 20, // satisfies both global holdout (age_22) and included holdout (age_40) audiences }, }); - const feature = config.featureKeyMap['flag_1']; const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); From 78eaa415f1b9ec98c9335e353d9c32338fdb077c Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Tue, 29 Jul 2025 00:20:58 +0600 Subject: [PATCH 09/18] [FSSDK-11528] remove only --- lib/core/decision_service/index.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index 93537bba6..e410e5d5f 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -1933,7 +1933,7 @@ describe('DecisionService', () => { }); }); - it.only("should consider global holdout even if local holdout is present", async () => { + it("should consider global holdout even if local holdout is present", async () => { const { decisionService } = getDecisionService(); const datafile = getHoldoutTestDatafile(); const newEntry = { From de3baaae94e66d71be360a113406e623f231bed5 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Tue, 29 Jul 2025 22:13:04 +0600 Subject: [PATCH 10/18] [FSSDK-11529] condition adjustment --- lib/optimizely/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/optimizely/index.ts b/lib/optimizely/index.ts index c84d6a4cb..2f3e3277f 100644 --- a/lib/optimizely/index.ts +++ b/lib/optimizely/index.ts @@ -652,7 +652,7 @@ export default class Optimizely extends BaseService implements Client { let featureEnabled = decision.getFeatureEnabledFromVariation(decisionObj); - if (decisionSource === DECISION_SOURCES.FEATURE_TEST) { + if (decisionSource === DECISION_SOURCES.FEATURE_TEST || decisionSource === DECISION_SOURCES.HOLDOUT) { sourceInfo = { experimentKey: experimentKey, variationKey: variationKey, @@ -661,6 +661,7 @@ export default class Optimizely extends BaseService implements Client { if ( decisionSource === DECISION_SOURCES.FEATURE_TEST || + decisionSource === DECISION_SOURCES.HOLDOUT || (decisionSource === DECISION_SOURCES.ROLLOUT && projectConfig.getSendFlagDecisionsValue(configObj)) ) { this.sendImpressionEvent(decisionObj, feature.key, userId, featureEnabled, attributes); @@ -1503,6 +1504,7 @@ export default class Optimizely extends BaseService implements Client { if ( !options[OptimizelyDecideOption.DISABLE_DECISION_EVENT] && (decisionSource === DECISION_SOURCES.FEATURE_TEST || + decisionSource === DECISION_SOURCES.HOLDOUT || (decisionSource === DECISION_SOURCES.ROLLOUT && projectConfig.getSendFlagDecisionsValue(configObj))) ) { this.sendImpressionEvent(decisionObj, key, userId, flagEnabled, attributes); From 8e33597f78eb752070ac0b050356b671bb730a88 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Tue, 29 Jul 2025 22:13:44 +0600 Subject: [PATCH 11/18] Revert "[FSSDK-11529] condition adjustment" This reverts commit de3baaae94e66d71be360a113406e623f231bed5. --- lib/optimizely/index.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/optimizely/index.ts b/lib/optimizely/index.ts index 2f3e3277f..c84d6a4cb 100644 --- a/lib/optimizely/index.ts +++ b/lib/optimizely/index.ts @@ -652,7 +652,7 @@ export default class Optimizely extends BaseService implements Client { let featureEnabled = decision.getFeatureEnabledFromVariation(decisionObj); - if (decisionSource === DECISION_SOURCES.FEATURE_TEST || decisionSource === DECISION_SOURCES.HOLDOUT) { + if (decisionSource === DECISION_SOURCES.FEATURE_TEST) { sourceInfo = { experimentKey: experimentKey, variationKey: variationKey, @@ -661,7 +661,6 @@ export default class Optimizely extends BaseService implements Client { if ( decisionSource === DECISION_SOURCES.FEATURE_TEST || - decisionSource === DECISION_SOURCES.HOLDOUT || (decisionSource === DECISION_SOURCES.ROLLOUT && projectConfig.getSendFlagDecisionsValue(configObj)) ) { this.sendImpressionEvent(decisionObj, feature.key, userId, featureEnabled, attributes); @@ -1504,7 +1503,6 @@ export default class Optimizely extends BaseService implements Client { if ( !options[OptimizelyDecideOption.DISABLE_DECISION_EVENT] && (decisionSource === DECISION_SOURCES.FEATURE_TEST || - decisionSource === DECISION_SOURCES.HOLDOUT || (decisionSource === DECISION_SOURCES.ROLLOUT && projectConfig.getSendFlagDecisionsValue(configObj))) ) { this.sendImpressionEvent(decisionObj, key, userId, flagEnabled, attributes); From 061880266c17cf383211a1d6491c0d17cb42d49b Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Tue, 29 Jul 2025 22:25:24 +0600 Subject: [PATCH 12/18] Reapply "[FSSDK-11529] condition adjustment" This reverts commit 8e33597f78eb752070ac0b050356b671bb730a88. --- lib/optimizely/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/optimizely/index.ts b/lib/optimizely/index.ts index c84d6a4cb..2f3e3277f 100644 --- a/lib/optimizely/index.ts +++ b/lib/optimizely/index.ts @@ -652,7 +652,7 @@ export default class Optimizely extends BaseService implements Client { let featureEnabled = decision.getFeatureEnabledFromVariation(decisionObj); - if (decisionSource === DECISION_SOURCES.FEATURE_TEST) { + if (decisionSource === DECISION_SOURCES.FEATURE_TEST || decisionSource === DECISION_SOURCES.HOLDOUT) { sourceInfo = { experimentKey: experimentKey, variationKey: variationKey, @@ -661,6 +661,7 @@ export default class Optimizely extends BaseService implements Client { if ( decisionSource === DECISION_SOURCES.FEATURE_TEST || + decisionSource === DECISION_SOURCES.HOLDOUT || (decisionSource === DECISION_SOURCES.ROLLOUT && projectConfig.getSendFlagDecisionsValue(configObj)) ) { this.sendImpressionEvent(decisionObj, feature.key, userId, featureEnabled, attributes); @@ -1503,6 +1504,7 @@ export default class Optimizely extends BaseService implements Client { if ( !options[OptimizelyDecideOption.DISABLE_DECISION_EVENT] && (decisionSource === DECISION_SOURCES.FEATURE_TEST || + decisionSource === DECISION_SOURCES.HOLDOUT || (decisionSource === DECISION_SOURCES.ROLLOUT && projectConfig.getSendFlagDecisionsValue(configObj))) ) { this.sendImpressionEvent(decisionObj, key, userId, flagEnabled, attributes); From 83b701792a0c147bf96ab9e62c016a2f4b8c6fe0 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Tue, 29 Jul 2025 23:46:27 +0600 Subject: [PATCH 13/18] [FSSDK-11529] test addition + layer logic adjustment --- .../event_builder/user_event.ts | 4 +- lib/feature_toggle.ts | 2 +- lib/optimizely/index.spec.ts | 248 ++++++++++++++++++ 3 files changed, 252 insertions(+), 2 deletions(-) diff --git a/lib/event_processor/event_builder/user_event.ts b/lib/event_processor/event_builder/user_event.ts index 5d098e87b..24419f4fc 100644 --- a/lib/event_processor/event_builder/user_event.ts +++ b/lib/event_processor/event_builder/user_event.ts @@ -28,6 +28,7 @@ import { import { EventTags, UserAttributes } from '../../shared_types'; import { LoggerFacade } from '../../logging/logger'; +import { DECISION_SOURCES } from '../../common_exports'; export type VisitorAttribute = { entityId: string @@ -185,7 +186,8 @@ export const buildImpressionEvent = function({ const variationId = decision.getVariationId(decisionObj); const cmabUuid = decisionObj.cmabUuid; - const layerId = experimentId !== null ? getLayerId(configObj, experimentId) : null; + const layerId = + experimentId !== null && ruleType !== DECISION_SOURCES.HOLDOUT ? getLayerId(configObj, experimentId) : null; return { ...buildBaseEvent({ diff --git a/lib/feature_toggle.ts b/lib/feature_toggle.ts index 22254e4f0..0a647c169 100644 --- a/lib/feature_toggle.ts +++ b/lib/feature_toggle.ts @@ -31,4 +31,4 @@ * flag and all associated checks can be removed from the codebase. */ -export const holdout = () => false; +export const holdout = () => true; diff --git a/lib/optimizely/index.spec.ts b/lib/optimizely/index.spec.ts index 165fae41b..39b52d289 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -29,6 +29,7 @@ import { DECISION_SOURCES } from '../utils/enums'; import OptimizelyUserContext from '../optimizely_user_context'; import { newErrorDecision } from '../optimizely_decision'; import { ImpressionEvent } from '../event_processor/event_builder/user_event'; +import { OptimizelyDecideOption } from '../shared_types'; describe('Optimizely', () => { const eventDispatcher = { @@ -212,5 +213,252 @@ describe('Optimizely', () => { const event = processSpy.mock.calls[0][0] as ImpressionEvent; expect(event.cmabUuid).toBe('uuid-cmab'); }); + + it('should dispatch impression event for holdout decision', async () => { + const datafile = getDecisionTestDatafile(); + + datafile.holdouts = [ + { + id: 'holdout_test_id', + key: 'holdout_test_key', + status: 'Running', + includeFlags: [], + excludeFlags: [], + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'holdout_variation_id', + key: 'holdout_variation_key', + variables: [], + featureEnabled: false + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_id', + endOfRange: 10000 + } + ] + } + ]; + + const projectConfig = createProjectConfig(datafile); + + const projectConfigManager = getMockProjectConfigManager({ + initConfig: projectConfig, + }); + + const mockEventDispatcher = { + dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), + }; + const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); + const processSpy = vi.spyOn(eventProcessor, 'process'); + + const optimizely = new Optimizely({ + clientEngine: 'node-sdk', + projectConfigManager, + eventProcessor, + jsonSchemaValidator, + logger, + odpManager, + disposable: true, + cmabService: {} as any + }); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const decisionService = optimizely.decisionService; + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: {}, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', []); + + expect(decision.ruleKey).toBe('holdout_test_key'); + expect(decision.flagKey).toBe('flag_1'); + expect(decision.variationKey).toBe('holdout_variation_key'); + expect(decision.enabled).toBe(false); + + expect(eventProcessor.process).toHaveBeenCalledOnce(); + + const event = processSpy.mock.calls[0][0] as ImpressionEvent; + + expect(event.type).toBe('impression'); + expect(event.ruleKey).toBe('holdout_test_key'); + expect(event.ruleType).toBe('holdout'); + expect(event.enabled).toBe(false); + }); + + it('should not dispatch impression event for holdout when DISABLE_DECISION_EVENT is used', async () => { + const datafile = getDecisionTestDatafile(); + + datafile.holdouts = [ + { + id: 'holdout_test_id', + key: 'holdout_test_key', + status: 'Running', + includeFlags: [], + excludeFlags: [], + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'holdout_variation_id', + key: 'holdout_variation_key', + variables: [], + featureEnabled: false + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_id', + endOfRange: 10000 + } + ] + } + ]; + + const projectConfig = createProjectConfig(datafile); + + const projectConfigManager = getMockProjectConfigManager({ + initConfig: projectConfig, + }); + + const mockEventDispatcher = { + dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), + }; + const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); + vi.spyOn(eventProcessor, 'process'); + + const optimizely = new Optimizely({ + clientEngine: 'node-sdk', + projectConfigManager, + eventProcessor, + jsonSchemaValidator, + logger, + odpManager, + disposable: true, + cmabService: {} as any + }); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const decisionService = optimizely.decisionService; + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts![0].variations[0], + experiment: projectConfig.holdouts![0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: {}, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', [OptimizelyDecideOption.DISABLE_DECISION_EVENT]); + + expect(decision.ruleKey).toBe('holdout_test_key'); + expect(decision.enabled).toBe(false); + expect(eventProcessor.process).not.toHaveBeenCalled(); + }); }); + describe('isFeatureEnabled', () => { + it('should dispatch impression event for holdout decision', async () => { + const datafile = getDecisionTestDatafile(); + datafile.holdouts = [ + { + id: 'holdout_test_id', + key: 'holdout_test_key', + status: 'Running', + includeFlags: [], + excludeFlags: [], + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'holdout_variation_id', + key: 'holdout_variation_key', + variables: [], + featureEnabled: false + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_id', + endOfRange: 10000 + } + ] + } + ]; + + const projectConfig = createProjectConfig(datafile); + const projectConfigManager = getMockProjectConfigManager({ + initConfig: projectConfig, + }); + const mockEventDispatcher = { + dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), + }; + const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); + vi.spyOn(eventProcessor, 'process'); + + const optimizely = new Optimizely({ + clientEngine: 'node-sdk', + projectConfigManager, + eventProcessor, + jsonSchemaValidator, + logger, + odpManager, + disposable: true, + cmabService: {} as any + }); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const decisionService = optimizely.decisionService; + vi.spyOn(decisionService, 'getVariationForFeature').mockReturnValue({ + error: false, + result: { + variation: projectConfig.holdouts![0].variations[0], + experiment: projectConfig.holdouts![0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }); + const result = optimizely.isFeatureEnabled('flag_1', 'test_user', {}); + + expect(result).toBe(false); + + expect(eventProcessor.process).toHaveBeenCalledOnce(); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const event = eventProcessor.process.mock.calls[0][0] as ImpressionEvent; + + expect(event.type).toBe('impression'); + expect(event.ruleKey).toBe('holdout_test_key'); + expect(event.ruleType).toBe('holdout'); + expect(event.enabled).toBe(false); + }); + }) }); From 894af9562073a214765080dde52317ba96bb20d6 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Wed, 30 Jul 2025 20:15:21 +0600 Subject: [PATCH 14/18] [FSSDK-11529] test data update --- lib/optimizely/index.spec.ts | 106 ++++++++++------------------------- 1 file changed, 30 insertions(+), 76 deletions(-) diff --git a/lib/optimizely/index.spec.ts b/lib/optimizely/index.spec.ts index 39b52d289..c333121c4 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -31,6 +31,33 @@ import { newErrorDecision } from '../optimizely_decision'; import { ImpressionEvent } from '../event_processor/event_builder/user_event'; import { OptimizelyDecideOption } from '../shared_types'; + +const holdoutData = [ + { + id: 'holdout_test_id', + key: 'holdout_test_key', + status: 'Running', + includeFlags: [], + excludeFlags: [], + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'holdout_variation_id', + key: 'holdout_variation_key', + variables: [], + featureEnabled: false, + }, + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_id', + endOfRange: 10000, + }, + ], + }, +]; + describe('Optimizely', () => { const eventDispatcher = { dispatchEvent: () => Promise.resolve({ statusCode: 200 }), @@ -217,32 +244,7 @@ describe('Optimizely', () => { it('should dispatch impression event for holdout decision', async () => { const datafile = getDecisionTestDatafile(); - datafile.holdouts = [ - { - id: 'holdout_test_id', - key: 'holdout_test_key', - status: 'Running', - includeFlags: [], - excludeFlags: [], - audienceIds: [], - audienceConditions: [], - variations: [ - { - id: 'holdout_variation_id', - key: 'holdout_variation_key', - variables: [], - featureEnabled: false - } - ], - trafficAllocation: [ - { - entityId: 'holdout_variation_id', - endOfRange: 10000 - } - ] - } - ]; - + datafile.holdouts = holdoutData; const projectConfig = createProjectConfig(datafile); const projectConfigManager = getMockProjectConfigManager({ @@ -307,31 +309,7 @@ describe('Optimizely', () => { it('should not dispatch impression event for holdout when DISABLE_DECISION_EVENT is used', async () => { const datafile = getDecisionTestDatafile(); - datafile.holdouts = [ - { - id: 'holdout_test_id', - key: 'holdout_test_key', - status: 'Running', - includeFlags: [], - excludeFlags: [], - audienceIds: [], - audienceConditions: [], - variations: [ - { - id: 'holdout_variation_id', - key: 'holdout_variation_key', - variables: [], - featureEnabled: false - } - ], - trafficAllocation: [ - { - entityId: 'holdout_variation_id', - endOfRange: 10000 - } - ] - } - ]; + datafile.holdouts = holdoutData; const projectConfig = createProjectConfig(datafile); @@ -387,31 +365,7 @@ describe('Optimizely', () => { describe('isFeatureEnabled', () => { it('should dispatch impression event for holdout decision', async () => { const datafile = getDecisionTestDatafile(); - datafile.holdouts = [ - { - id: 'holdout_test_id', - key: 'holdout_test_key', - status: 'Running', - includeFlags: [], - excludeFlags: [], - audienceIds: [], - audienceConditions: [], - variations: [ - { - id: 'holdout_variation_id', - key: 'holdout_variation_key', - variables: [], - featureEnabled: false - } - ], - trafficAllocation: [ - { - entityId: 'holdout_variation_id', - endOfRange: 10000 - } - ] - } - ]; + datafile.holdouts = holdoutData; const projectConfig = createProjectConfig(datafile); const projectConfigManager = getMockProjectConfigManager({ From 4fc5339af8a07be05684c8441edb9a710a1309e5 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Wed, 30 Jul 2025 20:16:15 +0600 Subject: [PATCH 15/18] [FSSDK-11529] test name update --- lib/optimizely/index.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/optimizely/index.spec.ts b/lib/optimizely/index.spec.ts index c333121c4..f72951786 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -363,7 +363,7 @@ describe('Optimizely', () => { }); }); describe('isFeatureEnabled', () => { - it('should dispatch impression event for holdout decision', async () => { + it('should dispatch impression event for holdout decision for isFeatureEnabled', async () => { const datafile = getDecisionTestDatafile(); datafile.holdouts = holdoutData; From eb3df0889289cde5c242c47968a27642a799f094 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Thu, 31 Jul 2025 21:01:28 +0600 Subject: [PATCH 16/18] [FSSDK-11529] review improvements --- .../event_builder/user_event.ts | 3 +- lib/optimizely/index.spec.ts | 514 ++++++++++++++++-- 2 files changed, 463 insertions(+), 54 deletions(-) diff --git a/lib/event_processor/event_builder/user_event.ts b/lib/event_processor/event_builder/user_event.ts index 24419f4fc..ae33d65da 100644 --- a/lib/event_processor/event_builder/user_event.ts +++ b/lib/event_processor/event_builder/user_event.ts @@ -185,9 +185,8 @@ export const buildImpressionEvent = function({ const variationKey = decision.getVariationKey(decisionObj); const variationId = decision.getVariationId(decisionObj); const cmabUuid = decisionObj.cmabUuid; - const layerId = - experimentId !== null && ruleType !== DECISION_SOURCES.HOLDOUT ? getLayerId(configObj, experimentId) : null; + experimentId !== null ? (ruleType === DECISION_SOURCES.HOLDOUT ? '' : getLayerId(configObj, experimentId)) : null; return { ...buildBaseEvent({ diff --git a/lib/optimizely/index.spec.ts b/lib/optimizely/index.spec.ts index f72951786..fcd786f42 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -30,6 +30,7 @@ import OptimizelyUserContext from '../optimizely_user_context'; import { newErrorDecision } from '../optimizely_decision'; import { ImpressionEvent } from '../event_processor/event_builder/user_event'; import { OptimizelyDecideOption } from '../shared_types'; +import { NOTIFICATION_TYPES, DECISION_NOTIFICATION_TYPES } from '../notification_center/type'; const holdoutData = [ @@ -241,11 +242,20 @@ describe('Optimizely', () => { expect(event.cmabUuid).toBe('uuid-cmab'); }); - it('should dispatch impression event for holdout decision', async () => { + + }); + + describe('holdout tests', () => { + let projectConfig: any; + let optimizely: any; + let decisionService: any; + let notificationSpy: any; + let eventProcessor: any; + + beforeEach(() => { const datafile = getDecisionTestDatafile(); - - datafile.holdouts = holdoutData; - const projectConfig = createProjectConfig(datafile); + datafile.holdouts = JSON.parse(JSON.stringify(holdoutData)); // Deep copy to avoid mutations + projectConfig = createProjectConfig(datafile); const projectConfigManager = getMockProjectConfigManager({ initConfig: projectConfig, @@ -254,10 +264,9 @@ describe('Optimizely', () => { const mockEventDispatcher = { dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), }; - const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); - const processSpy = vi.spyOn(eventProcessor, 'process'); + eventProcessor = getForwardingEventProcessor(mockEventDispatcher); - const optimizely = new Optimizely({ + optimizely = new Optimizely({ clientEngine: 'node-sdk', projectConfigManager, eventProcessor, @@ -270,7 +279,19 @@ describe('Optimizely', () => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore - const decisionService = optimizely.decisionService; + decisionService = optimizely.decisionService; + + // Setup notification spy + notificationSpy = vi.fn(); + optimizely.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.DECISION, + notificationSpy + ); + }); + + it('should dispatch impression event for holdout decision', async () => { + const processSpy = vi.spyOn(eventProcessor, 'process'); + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { return Value.of('async', [{ error: false, @@ -307,23 +328,342 @@ describe('Optimizely', () => { }); it('should not dispatch impression event for holdout when DISABLE_DECISION_EVENT is used', async () => { - const datafile = getDecisionTestDatafile(); + const processSpy = vi.spyOn(eventProcessor, 'process'); + + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: {}, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', [OptimizelyDecideOption.DISABLE_DECISION_EVENT]); + + expect(decision.ruleKey).toBe('holdout_test_key'); + expect(decision.enabled).toBe(false); + expect(processSpy).not.toHaveBeenCalled(); + }); + + it('should dispatch impression event for holdout decision for isFeatureEnabled', async () => { + const processSpy = vi.spyOn(eventProcessor, 'process'); - datafile.holdouts = holdoutData; + vi.spyOn(decisionService, 'getVariationForFeature').mockReturnValue({ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }); - const projectConfig = createProjectConfig(datafile); + const result = optimizely.isFeatureEnabled('flag_1', 'test_user', {}); + + expect(result).toBe(false); + + expect(eventProcessor.process).toHaveBeenCalledOnce(); + const event = processSpy.mock.calls[0][0] as ImpressionEvent; + + expect(event.type).toBe('impression'); + expect(event.ruleKey).toBe('holdout_test_key'); + expect(event.ruleType).toBe('holdout'); + expect(event.enabled).toBe(false); + }); + + it('should send correct decision notification for holdout decision', async () => { + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', []); + + expect(decision.flagKey).toBe('flag_1'); + expect(decision.enabled).toBe(false); + expect(decision.variationKey).toBe('holdout_variation_key'); + expect(decision.ruleKey).toBe('holdout_test_key'); + + // Verify decision notification was sent + expect(notificationSpy).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + flagKey: 'flag_1', + enabled: false, + variationKey: 'holdout_variation_key', + ruleKey: 'holdout_test_key', + variables: expect.any(Object), + reasons: expect.any(Array), + decisionEventDispatched: true, + }), + }); + }); + + it('should handle holdout with included flags', async () => { + // Modify holdout to include specific flag + const modifiedHoldout = { ...projectConfig.holdouts[0] }; + modifiedHoldout.includeFlags = ['1001']; // flag_1 ID from test datafile + projectConfig.holdouts = [modifiedHoldout]; + + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: modifiedHoldout.variations[0], + experiment: modifiedHoldout, + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', []); + + expect(decision.enabled).toBe(false); + expect(decision.ruleKey).toBe('holdout_test_key'); + expect(decision.variationKey).toBe('holdout_variation_key'); + + // Verify notification shows holdout details + expect(notificationSpy).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + flagKey: 'flag_1', + enabled: false, + ruleKey: 'holdout_test_key', + }), + }); + }); + + it('should handle holdout with excluded flags', async () => { + // Modify holdout to exclude specific flag + const modifiedHoldout = { ...projectConfig.holdouts[0] }; + modifiedHoldout.excludeFlags = ['1001']; // flag_1 ID from test datafile + projectConfig.holdouts = [modifiedHoldout]; + + // Mock normal feature test behavior for excluded flag + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.variationIdMap['5003'], + experiment: projectConfig.experimentKeyMap['exp_3'], + decisionSource: DECISION_SOURCES.FEATURE_TEST, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: { country: 'BD', age: 80 }, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', []); + + expect(decision.enabled).toBe(true); + expect(decision.ruleKey).toBe('exp_3'); + expect(decision.variationKey).toBe('variation_3'); + + // Verify notification shows normal experiment details (not holdout) + expect(notificationSpy).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'BD', age: 80 }, + decisionInfo: expect.objectContaining({ + flagKey: 'flag_1', + enabled: true, + ruleKey: 'exp_3', + }), + }); + }); + + it('should handle multiple holdouts with correct priority', async () => { + // Setup multiple holdouts + const holdout1 = { ...projectConfig.holdouts[0] }; + holdout1.excludeFlags = ['1001']; // exclude flag_1 + + const holdout2 = { + id: 'holdout_test_id_2', + key: 'holdout_test_key_2', + status: 'Running', + includeFlags: ['1001'], // include flag_1 + excludeFlags: [], + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'holdout_variation_id_2', + key: 'holdout_variation_key_2', + variables: [], + featureEnabled: false, + }, + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_id_2', + endOfRange: 10000, + }, + ], + }; + + projectConfig.holdouts = [holdout1, holdout2]; + + // Mock that holdout2 takes priority due to includeFlags + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: holdout2.variations[0], + experiment: holdout2, + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', []); + + expect(decision.enabled).toBe(false); + expect(decision.ruleKey).toBe('holdout_test_key_2'); + expect(decision.variationKey).toBe('holdout_variation_key_2'); + + // Verify notification shows details of selected holdout + expect(notificationSpy).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + flagKey: 'flag_1', + enabled: false, + ruleKey: 'holdout_test_key_2', + }), + }); + }); + + it('should respect sendFlagDecisions setting for holdout events - false', async () => { + // Set sendFlagDecisions to false + projectConfig.sendFlagDecisions = false; + + const mockEventDispatcher = { + dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), + }; + const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); + const processSpy = vi.spyOn(eventProcessor, 'process'); const projectConfigManager = getMockProjectConfigManager({ initConfig: projectConfig, }); + const optimizelyWithConfig = new Optimizely({ + clientEngine: 'node-sdk', + projectConfigManager, + eventProcessor, + jsonSchemaValidator, + logger, + odpManager, + disposable: true, + cmabService: {} as any + }); + + // Add notification listener + const notificationSpyLocal = vi.fn(); + optimizelyWithConfig.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.DECISION, + notificationSpyLocal + ); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const decisionServiceLocal = optimizelyWithConfig.decisionService; + vi.spyOn(decisionServiceLocal, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely: optimizelyWithConfig, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + await optimizelyWithConfig.decideAsync(user, 'flag_1', []); + + // Impression event should still be dispatched for holdouts even when sendFlagDecisions is false + expect(processSpy).toHaveBeenCalledOnce(); + + // Verify notification shows decisionEventDispatched: true + expect(notificationSpyLocal).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + decisionEventDispatched: true, + }), + }); + }); + + it('should respect sendFlagDecisions setting for holdout events - true', async () => { + // Set sendFlagDecisions to true + projectConfig.sendFlagDecisions = true; + const mockEventDispatcher = { dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), }; const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); - vi.spyOn(eventProcessor, 'process'); + const processSpy = vi.spyOn(eventProcessor, 'process'); - const optimizely = new Optimizely({ + const projectConfigManager = getMockProjectConfigManager({ + initConfig: projectConfig, + }); + + const optimizelyWithConfig = new Optimizely({ clientEngine: 'node-sdk', projectConfigManager, eventProcessor, @@ -334,15 +674,57 @@ describe('Optimizely', () => { cmabService: {} as any }); + // Add notification listener + const notificationSpyLocal = vi.fn(); + optimizelyWithConfig.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.DECISION, + notificationSpyLocal + ); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore - const decisionService = optimizely.decisionService; + const decisionServiceLocal = optimizelyWithConfig.decisionService; + vi.spyOn(decisionServiceLocal, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely: optimizelyWithConfig, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + await optimizelyWithConfig.decideAsync(user, 'flag_1', []); + + // Impression event should be dispatched for holdouts + expect(processSpy).toHaveBeenCalledOnce(); + + // Verify notification shows decisionEventDispatched: true + expect(notificationSpyLocal).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + decisionEventDispatched: true, + }), + }); + }); + + it('should return correct variable values for holdout decision', async () => { vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { return Value.of('async', [{ error: false, result: { - variation: projectConfig.holdouts![0].variations[0], - experiment: projectConfig.holdouts![0], + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], decisionSource: DECISION_SOURCES.HOLDOUT, }, reasons: [], @@ -352,32 +734,40 @@ describe('Optimizely', () => { const user = new OptimizelyUserContext({ optimizely, userId: 'test_user', - attributes: {}, + attributes: { country: 'US' }, }); - const decision = await optimizely.decideAsync(user, 'flag_1', [OptimizelyDecideOption.DISABLE_DECISION_EVENT]); + const decision = await optimizely.decideAsync(user, 'flag_1', []); - expect(decision.ruleKey).toBe('holdout_test_key'); expect(decision.enabled).toBe(false); - expect(eventProcessor.process).not.toHaveBeenCalled(); - }); - }); - describe('isFeatureEnabled', () => { - it('should dispatch impression event for holdout decision for isFeatureEnabled', async () => { - const datafile = getDecisionTestDatafile(); - datafile.holdouts = holdoutData; - - const projectConfig = createProjectConfig(datafile); - const projectConfigManager = getMockProjectConfigManager({ - initConfig: projectConfig, + expect(decision.variables).toBeDefined(); + expect(typeof decision.variables).toBe('object'); + + // Verify notification includes variable information + expect(notificationSpy).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + variables: expect.any(Object), + flagKey: 'flag_1', + enabled: false, + }), }); + }); + + it('should handle disable decision event option for holdout', async () => { const mockEventDispatcher = { dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), }; const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); - vi.spyOn(eventProcessor, 'process'); + const processSpy = vi.spyOn(eventProcessor, 'process'); - const optimizely = new Optimizely({ + const projectConfigManager = getMockProjectConfigManager({ + initConfig: projectConfig, + }); + + const optimizelyWithEventProcessor = new Optimizely({ clientEngine: 'node-sdk', projectConfigManager, eventProcessor, @@ -388,31 +778,51 @@ describe('Optimizely', () => { cmabService: {} as any }); + // Add notification listener + const notificationSpyLocal = vi.fn(); + optimizelyWithEventProcessor.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.DECISION, + notificationSpyLocal + ); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore - const decisionService = optimizely.decisionService; - vi.spyOn(decisionService, 'getVariationForFeature').mockReturnValue({ - error: false, - result: { - variation: projectConfig.holdouts![0].variations[0], - experiment: projectConfig.holdouts![0], - decisionSource: DECISION_SOURCES.HOLDOUT, - }, - reasons: [], + const decisionServiceLocal = optimizelyWithEventProcessor.decisionService; + vi.spyOn(decisionServiceLocal, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); }); - const result = optimizely.isFeatureEnabled('flag_1', 'test_user', {}); - expect(result).toBe(false); + const user = new OptimizelyUserContext({ + optimizely: optimizelyWithEventProcessor, + userId: 'test_user', + attributes: { country: 'US' }, + }); - expect(eventProcessor.process).toHaveBeenCalledOnce(); - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - const event = eventProcessor.process.mock.calls[0][0] as ImpressionEvent; + const decision = await optimizelyWithEventProcessor.decideAsync(user, 'flag_1', [OptimizelyDecideOption.DISABLE_DECISION_EVENT]); - expect(event.type).toBe('impression'); - expect(event.ruleKey).toBe('holdout_test_key'); - expect(event.ruleType).toBe('holdout'); - expect(event.enabled).toBe(false); + expect(decision.enabled).toBe(false); + expect(decision.ruleKey).toBe('holdout_test_key'); + + // No impression event should be dispatched + expect(processSpy).not.toHaveBeenCalled(); + + // Verify notification shows decisionEventDispatched: false + expect(notificationSpyLocal).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + decisionEventDispatched: false, + }), + }); }); - }) + }); }); From 504bb8c64c3ef749801e085b1e888c9ecd9a1fd3 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Fri, 1 Aug 2025 18:58:03 +0600 Subject: [PATCH 17/18] [FSSDK-11529] fsc fix --- lib/core/decision_service/index.spec.ts | 22 +++++++++---------- lib/optimizely/index.spec.ts | 16 +++++++------- lib/project_config/project_config.spec.ts | 26 +++++++++++------------ lib/project_config/project_config.ts | 14 ++++++------ lib/shared_types.ts | 4 ++-- 5 files changed, 41 insertions(+), 41 deletions(-) diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index e410e5d5f..e8c29861e 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -137,8 +137,8 @@ const getHoldoutTestDatafile = () => { id: 'holdout_running_id', key: 'holdout_running', status: 'Running', - includeFlags: [], - excludeFlags: [], + includedFlags: [], + excludedFlags: [], audienceIds: ['4001'], // age_22 audience audienceConditions: ['or', '4001'], variations: [ @@ -159,8 +159,8 @@ const getHoldoutTestDatafile = () => { id: "holdout_not_bucketed_id", key: "holdout_not_bucketed", status: "Running", - includeFlags: [], - excludeFlags: [], + includedFlags: [], + excludedFlags: [], audienceIds: ['4002'], audienceConditions: ['or', '4002'], variations: [ @@ -1940,8 +1940,8 @@ describe('DecisionService', () => { id: 'holdout_included_id', key: 'holdout_included', status: 'Running', - includeFlags: ['flag_1'], - excludeFlags: [], + includedFlags: ['flag_1'], + excludedFlags: [], audienceIds: ['4002'], // age_40 audience audienceConditions: ['or', '4002'], variations: [ @@ -1989,8 +1989,8 @@ describe('DecisionService', () => { id: 'holdout_included_specific_id', key: 'holdout_included_specific', status: 'Running', - includeFlags: ['flag_1'], - excludeFlags: [], + includedFlags: ['flag_1'], + excludedFlags: [], audienceIds: ['4002'], // age_60 audience (age <= 60) audienceConditions: ['or', '4002'], variations: [ @@ -2176,7 +2176,7 @@ describe('DecisionService', () => { if(holdout.id === 'holdout_running_id') { return { ...holdout, - excludeFlags: ['flag_1'] + excludedFlags: ['flag_1'] } } return holdout; @@ -2212,8 +2212,8 @@ describe('DecisionService', () => { id: 'holdout_second_id', key: 'holdout_second', status: 'Running', - includeFlags: [], - excludeFlags: [], + includedFlags: [], + excludedFlags: [], audienceIds: [], // no audience requirements audienceConditions: [], variations: [ diff --git a/lib/optimizely/index.spec.ts b/lib/optimizely/index.spec.ts index fcd786f42..0c7e2fc79 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -38,8 +38,8 @@ const holdoutData = [ id: 'holdout_test_id', key: 'holdout_test_key', status: 'Running', - includeFlags: [], - excludeFlags: [], + includedFlags: [], + excludedFlags: [], audienceIds: [], audienceConditions: [], variations: [ @@ -427,7 +427,7 @@ describe('Optimizely', () => { it('should handle holdout with included flags', async () => { // Modify holdout to include specific flag const modifiedHoldout = { ...projectConfig.holdouts[0] }; - modifiedHoldout.includeFlags = ['1001']; // flag_1 ID from test datafile + modifiedHoldout.includedFlags = ['1001']; // flag_1 ID from test datafile projectConfig.holdouts = [modifiedHoldout]; vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { @@ -470,7 +470,7 @@ describe('Optimizely', () => { it('should handle holdout with excluded flags', async () => { // Modify holdout to exclude specific flag const modifiedHoldout = { ...projectConfig.holdouts[0] }; - modifiedHoldout.excludeFlags = ['1001']; // flag_1 ID from test datafile + modifiedHoldout.excludedFlags = ['1001']; // flag_1 ID from test datafile projectConfig.holdouts = [modifiedHoldout]; // Mock normal feature test behavior for excluded flag @@ -514,14 +514,14 @@ describe('Optimizely', () => { it('should handle multiple holdouts with correct priority', async () => { // Setup multiple holdouts const holdout1 = { ...projectConfig.holdouts[0] }; - holdout1.excludeFlags = ['1001']; // exclude flag_1 + holdout1.excludedFlags = ['1001']; // exclude flag_1 const holdout2 = { id: 'holdout_test_id_2', key: 'holdout_test_key_2', status: 'Running', - includeFlags: ['1001'], // include flag_1 - excludeFlags: [], + includedFlags: ['1001'], // include flag_1 + excludedFlags: [], audienceIds: [], audienceConditions: [], variations: [ @@ -542,7 +542,7 @@ describe('Optimizely', () => { projectConfig.holdouts = [holdout1, holdout2]; - // Mock that holdout2 takes priority due to includeFlags + // Mock that holdout2 takes priority due to includedFlags vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { return Value.of('async', [{ error: false, diff --git a/lib/project_config/project_config.spec.ts b/lib/project_config/project_config.spec.ts index 662488914..3df0305a5 100644 --- a/lib/project_config/project_config.spec.ts +++ b/lib/project_config/project_config.spec.ts @@ -316,8 +316,8 @@ const getHoldoutDatafile = () => { id: 'holdout_id_1', key: 'holdout_1', status: 'Running', - includeFlags: [], - excludeFlags: [], + includedFlags: [], + excludedFlags: [], audienceIds: ['13389130056'], audienceConditions: ['or', '13389130056'], variations: [ @@ -338,8 +338,8 @@ const getHoldoutDatafile = () => { id: 'holdout_id_2', key: 'holdout_2', status: 'Running', - includeFlags: [], - excludeFlags: ['feature_3'], + includedFlags: [], + excludedFlags: ['feature_3'], audienceIds: [], audienceConditions: [], variations: [ @@ -360,8 +360,8 @@ const getHoldoutDatafile = () => { id: 'holdout_id_3', key: 'holdout_3', status: 'Draft', - includeFlags: ['feature_1'], - excludeFlags: [], + includedFlags: ['feature_1'], + excludedFlags: [], audienceIds: [], audienceConditions: [], variations: [ @@ -415,8 +415,8 @@ describe('createProjectConfig - holdouts, feature toggle is on', () => { expect(configObj.globalHoldouts).toHaveLength(2); expect(configObj.globalHoldouts).toEqual([ - configObj.holdouts[0], // holdout_1 has empty includeFlags - configObj.holdouts[1] // holdout_2 has empty includeFlags + configObj.holdouts[0], // holdout_1 has empty includedFlags + configObj.holdouts[1] // holdout_2 has empty includedFlags ]); expect(configObj.includedHoldouts).toEqual({ @@ -443,16 +443,16 @@ describe('createProjectConfig - holdouts, feature toggle is on', () => { expect(configObj.flagHoldoutsMap).toEqual({}); }); - it('should handle undefined includeFlags and excludeFlags in holdout', function() { + it('should handle undefined includedFlags and excludedFlags in holdout', function() { const datafile = getHoldoutDatafile(); - datafile.holdouts[0].includeFlags = undefined; - datafile.holdouts[0].excludeFlags = undefined; + datafile.holdouts[0].includedFlags = undefined; + datafile.holdouts[0].excludedFlags = undefined; const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile))); expect(configObj.holdouts).toHaveLength(3); - expect(configObj.holdouts[0].includeFlags).toEqual([]); - expect(configObj.holdouts[0].excludeFlags).toEqual([]); + expect(configObj.holdouts[0].includedFlags).toEqual([]); + expect(configObj.holdouts[0].excludedFlags).toEqual([]); }); }); diff --git a/lib/project_config/project_config.ts b/lib/project_config/project_config.ts index 1b14e4408..aa224d2c9 100644 --- a/lib/project_config/project_config.ts +++ b/lib/project_config/project_config.ts @@ -361,12 +361,12 @@ const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => { projectConfig.flagHoldoutsMap = {}; projectConfig.holdouts.forEach((holdout) => { - if (!holdout.includeFlags) { - holdout.includeFlags = []; + if (!holdout.includedFlags) { + holdout.includedFlags = []; } - if (!holdout.excludeFlags) { - holdout.excludeFlags = []; + if (!holdout.excludedFlags) { + holdout.excludedFlags = []; } holdout.variationKeyMap = keyBy(holdout.variations, 'key'); @@ -376,17 +376,17 @@ const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => { ...keyBy(holdout.variations, 'id'), }; - if (holdout.includeFlags.length === 0) { + if (holdout.includedFlags.length === 0) { projectConfig.globalHoldouts.push(holdout); - holdout.excludeFlags.forEach((flagKey) => { + holdout.excludedFlags.forEach((flagKey: string) => { if (!projectConfig.excludedHoldouts[flagKey]) { projectConfig.excludedHoldouts[flagKey] = []; } projectConfig.excludedHoldouts[flagKey].push(holdout); }); } else { - holdout.includeFlags.forEach((flagKey) => { + holdout.includedFlags.forEach((flagKey: string) => { if (!projectConfig.includedHoldouts[flagKey]) { projectConfig.includedHoldouts[flagKey] = []; } diff --git a/lib/shared_types.ts b/lib/shared_types.ts index 7c2046bf6..65b9594b2 100644 --- a/lib/shared_types.ts +++ b/lib/shared_types.ts @@ -177,8 +177,8 @@ export type HoldoutStatus = 'Draft' | 'Running' | 'Concluded' | 'Archived'; export interface Holdout extends ExperimentCore { status: HoldoutStatus; - includeFlags: string[]; - excludeFlags: string[]; + includedFlags: string[]; + excludedFlags: string[]; } export enum VariableType { From 6bf661a8d9e606a09e39444af0150c826e4a7e6f Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Fri, 1 Aug 2025 22:59:35 +0600 Subject: [PATCH 18/18] [FSSDK-11529] project config issue fix --- lib/core/decision_service/index.spec.ts | 6 ++--- lib/project_config/project_config.spec.ts | 8 +++---- lib/project_config/project_config.ts | 28 +++++++++++++++-------- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index e8c29861e..9fc9d89a1 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -1940,7 +1940,7 @@ describe('DecisionService', () => { id: 'holdout_included_id', key: 'holdout_included', status: 'Running', - includedFlags: ['flag_1'], + includedFlags: ['1001'], excludedFlags: [], audienceIds: ['4002'], // age_40 audience audienceConditions: ['or', '4002'], @@ -1989,7 +1989,7 @@ describe('DecisionService', () => { id: 'holdout_included_specific_id', key: 'holdout_included_specific', status: 'Running', - includedFlags: ['flag_1'], + includedFlags: ['1001'], excludedFlags: [], audienceIds: ['4002'], // age_60 audience (age <= 60) audienceConditions: ['or', '4002'], @@ -2176,7 +2176,7 @@ describe('DecisionService', () => { if(holdout.id === 'holdout_running_id') { return { ...holdout, - excludedFlags: ['flag_1'] + excludedFlags: ['1001'] } } return holdout; diff --git a/lib/project_config/project_config.spec.ts b/lib/project_config/project_config.spec.ts index 3df0305a5..21d18940c 100644 --- a/lib/project_config/project_config.spec.ts +++ b/lib/project_config/project_config.spec.ts @@ -339,7 +339,7 @@ const getHoldoutDatafile = () => { key: 'holdout_2', status: 'Running', includedFlags: [], - excludedFlags: ['feature_3'], + excludedFlags: ['44829230000'], audienceIds: [], audienceConditions: [], variations: [ @@ -360,7 +360,7 @@ const getHoldoutDatafile = () => { id: 'holdout_id_3', key: 'holdout_3', status: 'Draft', - includedFlags: ['feature_1'], + includedFlags: ['4482920077'], excludedFlags: [], audienceIds: [], audienceConditions: [], @@ -420,11 +420,11 @@ describe('createProjectConfig - holdouts, feature toggle is on', () => { ]); expect(configObj.includedHoldouts).toEqual({ - feature_1: [configObj.holdouts[2]], // holdout_3 includes feature_1 + feature_1: [configObj.holdouts[2]], // holdout_3 includes feature_1 (ID: 4482920077) }); expect(configObj.excludedHoldouts).toEqual({ - feature_3: [configObj.holdouts[1]] // holdout_2 excludes feature_3 + feature_3: [configObj.holdouts[1]] // holdout_2 excludes feature_3 (ID: 44829230000) }); expect(configObj.flagHoldoutsMap).toEqual({}); diff --git a/lib/project_config/project_config.ts b/lib/project_config/project_config.ts index aa224d2c9..efb50301e 100644 --- a/lib/project_config/project_config.ts +++ b/lib/project_config/project_config.ts @@ -360,6 +360,8 @@ const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => { projectConfig.excludedHoldouts = {}; projectConfig.flagHoldoutsMap = {}; + const featureFlagIdMap = keyBy(projectConfig.featureFlags, 'id'); + projectConfig.holdouts.forEach((holdout) => { if (!holdout.includedFlags) { holdout.includedFlags = []; @@ -379,19 +381,27 @@ const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => { if (holdout.includedFlags.length === 0) { projectConfig.globalHoldouts.push(holdout); - holdout.excludedFlags.forEach((flagKey: string) => { - if (!projectConfig.excludedHoldouts[flagKey]) { - projectConfig.excludedHoldouts[flagKey] = []; + holdout.excludedFlags.forEach((flagId: string) => { + const flag = featureFlagIdMap[flagId]; + if (flag) { + const flagKey = flag.key; + if (!projectConfig.excludedHoldouts[flagKey]) { + projectConfig.excludedHoldouts[flagKey] = []; + } + projectConfig.excludedHoldouts[flagKey].push(holdout); } - projectConfig.excludedHoldouts[flagKey].push(holdout); }); } else { - holdout.includedFlags.forEach((flagKey: string) => { - if (!projectConfig.includedHoldouts[flagKey]) { - projectConfig.includedHoldouts[flagKey] = []; + holdout.includedFlags.forEach((flagId: string) => { + const flag = featureFlagIdMap[flagId]; + if (flag) { + const flagKey = flag.key; + if (!projectConfig.includedHoldouts[flagKey]) { + projectConfig.includedHoldouts[flagKey] = []; + } + projectConfig.includedHoldouts[flagKey].push(holdout); } - projectConfig.includedHoldouts[flagKey].push(holdout); - }); + }) } }); }