diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index 2334a386b..944fe27c8 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -2196,6 +2196,157 @@ describe('DecisionService', () => { decisionSource: DECISION_SOURCES.HOLDOUT, }); }); + + describe('excludeTargetedDeliveries', () => { + const getExcludeTDDatafile = (excludeTargetedDeliveries?: boolean) => { + const datafile = getHoldoutTestDatafile(); + datafile.holdouts = datafile.holdouts.map((holdout: any) => { + if (holdout.id === 'holdout_running_id') { + return { + ...holdout, + ...(excludeTargetedDeliveries !== undefined ? { excludeTargetedDeliveries } : {}), + }; + } + return holdout; + }); + return datafile; + }; + + it('should return holdout variation immediately when excludeTargetedDeliveries is false', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(JSON.stringify(getExcludeTDDatafile(false))); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + 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 return holdout variation immediately when excludeTargetedDeliveries is undefined', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(JSON.stringify(getExcludeTDDatafile(undefined))); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + 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 return delivery variation when excludeTargetedDeliveries is true and delivery matches', async () => { + const { decisionService } = getDecisionService(); + const datafile = getExcludeTDDatafile(true); + + const config = createProjectConfig(JSON.stringify(datafile)); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + if (param.experimentKey === 'delivery_1') { + return { result: '5004', reasons: [] }; + } + if (param.experimentKey === '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(); + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['delivery_1'], + variation: config.variationIdMap['5004'], + decisionSource: DECISION_SOURCES.ROLLOUT, + }); + }); + + it('should return holdout variation when excludeTargetedDeliveries is true but no delivery matches', async () => { + const { decisionService } = getDecisionService(); + const datafile = getExcludeTDDatafile(true); + + const config = createProjectConfig(JSON.stringify(datafile)); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + if (param.experimentKey === '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(); + 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 still block experiment rules when excludeTargetedDeliveries is true', async () => { + const { decisionService } = getDecisionService(); + const datafile = getExcludeTDDatafile(true); + + const config = createProjectConfig(JSON.stringify(datafile)); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + if (param.experimentKey === 'holdout_running') { + return { result: 'holdout_variation_running_id', reasons: [] }; + } + if (param.experimentKey === 'exp_1') { + return { result: '5001', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + const variation = (await value)[0]; + + // Should NOT get exp_1 — holdout blocks experiments even with excludeTargetedDeliveries + // Should get holdout since no delivery matched + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + }); }); }); diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 2064ec2a2..14213e5b3 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -119,6 +119,7 @@ export const USER_MEETS_CONDITIONS_FOR_HOLDOUT = 'User %s meets conditions for h 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 const TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT = 'User %s is in holdout %s but targeted deliveries are excluded from holdout.'; export interface DecisionObj { experiment: Experiment | Holdout | null; @@ -969,11 +970,23 @@ export class DecisionService { // getGlobalHoldouts() returns holdouts with includedRules == null/undefined. const globalHoldouts = getGlobalHoldouts(configObj); + let activeHoldoutDecision: DecisionObj | null = null; + let activeHoldout: Holdout | null = null; + for (const holdout of globalHoldouts) { const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); decideReasons.push(...holdoutDecision.reasons); if (holdoutDecision.result.variation) { + if (holdout.excludeTargetedDeliveries) { + const userId = user.getUserId(); + this.logger?.info(TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT, userId, holdout.key); + decideReasons.push([TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT, userId, holdout.key]); + activeHoldoutDecision = holdoutDecision.result; + activeHoldout = holdout; + break; + } + return Value.of(op, { result: holdoutDecision.result, reasons: decideReasons, @@ -981,33 +994,58 @@ export class DecisionService { } } - return this.getVariationForFeatureExperiment(op, configObj, feature, user, decideOptions, userProfileTracker).then((experimentDecision) => { - if (experimentDecision.error || experimentDecision.result.variation !== null) { + if (!activeHoldoutDecision) { + return this.getVariationForFeatureExperiment(op, configObj, feature, user, decideOptions, userProfileTracker).then((experimentDecision) => { + if (experimentDecision.error || experimentDecision.result.variation !== null) { + return Value.of(op, { + ...experimentDecision, + reasons: [...decideReasons, ...experimentDecision.reasons], + }); + } + + decideReasons.push(...experimentDecision.reasons); + + const rolloutDecision = this.getVariationForRollout(configObj, feature, user); + decideReasons.push(...rolloutDecision.reasons); + const rolloutDecisionResult = rolloutDecision.result; + const userId = user.getUserId(); + + if (rolloutDecisionResult.variation) { + this.logger?.debug(USER_IN_ROLLOUT, userId, feature.key); + decideReasons.push([USER_IN_ROLLOUT, userId, feature.key]); + } else { + this.logger?.debug(USER_NOT_IN_ROLLOUT, userId, feature.key); + decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]); + } + return Value.of(op, { - ...experimentDecision, - reasons: [...decideReasons, ...experimentDecision.reasons], + result: rolloutDecisionResult, + reasons: decideReasons, }); - } + }); + } - decideReasons.push(...experimentDecision.reasons); - - const rolloutDecision = this.getVariationForRollout(configObj, feature, user); - decideReasons.push(...rolloutDecision.reasons); - const rolloutDecisionResult = rolloutDecision.result; - const userId = user.getUserId(); - - if (rolloutDecisionResult.variation) { - this.logger?.debug(USER_IN_ROLLOUT, userId, feature.key); - decideReasons.push([USER_IN_ROLLOUT, userId, feature.key]); - } else { - this.logger?.debug(USER_NOT_IN_ROLLOUT, userId, feature.key); - decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]); - } - + // User is in holdout with excludeTargetedDeliveries — skip experiments, evaluate delivery rules + const rolloutDecision = this.getVariationForRollout(configObj, feature, user); + decideReasons.push(...rolloutDecision.reasons); + const rolloutDecisionResult = rolloutDecision.result; + const userId = user.getUserId(); + + if (rolloutDecisionResult.variation) { + this.logger?.debug(USER_IN_ROLLOUT, userId, feature.key); + decideReasons.push([USER_IN_ROLLOUT, userId, feature.key]); return Value.of(op, { result: rolloutDecisionResult, reasons: decideReasons, }); + } + + this.logger?.debug(USER_NOT_IN_ROLLOUT, userId, feature.key); + decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]); + + return Value.of(op, { + result: activeHoldoutDecision, + reasons: decideReasons, }); } diff --git a/lib/shared_types.ts b/lib/shared_types.ts index 6640622ab..4cc772019 100644 --- a/lib/shared_types.ts +++ b/lib/shared_types.ts @@ -190,6 +190,7 @@ export interface Holdout extends ExperimentCore { * membership. */ isGlobal: boolean; + excludeTargetedDeliveries?: boolean; } export function isHoldout(obj: Experiment | Holdout): obj is Holdout {