Skip to content

Commit 354d2bc

Browse files
committed
[AI-FSSDK] [FSSDK-12735] Add holdout event dispatch, local holdout guard, and null TD fallback
1 parent 475d932 commit 354d2bc

4 files changed

Lines changed: 123 additions & 12 deletions

File tree

lib/core/decision_service/index.spec.ts

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,7 +2252,7 @@ describe('DecisionService', () => {
22522252
});
22532253
});
22542254

2255-
it('should return delivery variation when excludeTargetedDeliveries is true and delivery matches', async () => {
2255+
it('should return delivery variation with holdout info when excludeTargetedDeliveries is true and delivery matches', async () => {
22562256
const { decisionService } = getDecisionService();
22572257
const datafile = getExcludeTDDatafile(true);
22582258

@@ -2281,10 +2281,14 @@ describe('DecisionService', () => {
22812281
experiment: config.experimentKeyMap['delivery_1'],
22822282
variation: config.variationIdMap['5004'],
22832283
decisionSource: DECISION_SOURCES.ROLLOUT,
2284+
holdout: {
2285+
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2286+
variation: config.variationIdMap['holdout_variation_running_id'],
2287+
},
22842288
});
22852289
});
22862290

2287-
it('should return holdout variation when excludeTargetedDeliveries is true but no delivery matches', async () => {
2291+
it('should return null decision with holdout info when excludeTargetedDeliveries is true but no delivery matches', async () => {
22882292
const { decisionService } = getDecisionService();
22892293
const datafile = getExcludeTDDatafile(true);
22902294

@@ -2307,9 +2311,13 @@ describe('DecisionService', () => {
23072311
const variation = (await value)[0];
23082312

23092313
expect(variation.result).toEqual({
2310-
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2311-
variation: config.variationIdMap['holdout_variation_running_id'],
2312-
decisionSource: DECISION_SOURCES.HOLDOUT,
2314+
experiment: null,
2315+
variation: null,
2316+
decisionSource: DECISION_SOURCES.ROLLOUT,
2317+
holdout: {
2318+
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2319+
variation: config.variationIdMap['holdout_variation_running_id'],
2320+
},
23132321
});
23142322
});
23152323

@@ -2338,12 +2346,14 @@ describe('DecisionService', () => {
23382346
const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
23392347
const variation = (await value)[0];
23402348

2341-
// Should NOT get exp_1 — holdout blocks experiments even with excludeTargetedDeliveries
2342-
// Should get holdout since no delivery matched
23432349
expect(variation.result).toEqual({
2344-
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2345-
variation: config.variationIdMap['holdout_variation_running_id'],
2346-
decisionSource: DECISION_SOURCES.HOLDOUT,
2350+
experiment: null,
2351+
variation: null,
2352+
decisionSource: DECISION_SOURCES.ROLLOUT,
2353+
holdout: {
2354+
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2355+
variation: config.variationIdMap['holdout_variation_running_id'],
2356+
},
23472357
});
23482358
});
23492359
});
@@ -3252,6 +3262,34 @@ describe('DecisionService', () => {
32523262
expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.FEATURE_TEST);
32533263
expect(value[0].result.variation?.key).toBe('variation_1');
32543264
});
3265+
3266+
it('local holdout ignores excludeTargetedDeliveries and applies normally', async () => {
3267+
const datafile = makeLocalHoldoutDatafile('2001');
3268+
(datafile as any).localHoldouts[0].excludeTargetedDeliveries = true;
3269+
const config = createProjectConfig(JSON.stringify(datafile));
3270+
const { decisionService } = getDecisionService();
3271+
3272+
mockBucket.mockImplementation((params: BucketerParams) => {
3273+
if (params.experimentId === 'local_holdout_id') {
3274+
return { result: 'local_holdout_variation_id', reasons: [] };
3275+
}
3276+
return { result: null, reasons: [] };
3277+
});
3278+
3279+
const user = new OptimizelyUserContext({
3280+
optimizely: {} as any,
3281+
userId: 'user1',
3282+
attributes: { age: 15 },
3283+
});
3284+
3285+
const feature = config.featureKeyMap['flag_1'];
3286+
const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
3287+
3288+
// Local holdout applies normally even with excludeTargetedDeliveries set
3289+
expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.HOLDOUT);
3290+
expect(value[0].result.experiment?.id).toBe('local_holdout_id');
3291+
expect(value[0].result.variation?.id).toBe('local_holdout_variation_id');
3292+
});
32553293
});
32563294
});
32573295

lib/core/decision_service/index.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,10 @@ export interface DecisionObj {
126126
variation: Variation | null;
127127
decisionSource: DecisionSource;
128128
cmabUuid?: string;
129+
holdout?: {
130+
experiment: Holdout;
131+
variation: Variation;
132+
};
129133
}
130134

131135
interface DecisionServiceOptions {
@@ -1035,7 +1039,13 @@ export class DecisionService {
10351039
this.logger?.debug(USER_IN_ROLLOUT, userId, feature.key);
10361040
decideReasons.push([USER_IN_ROLLOUT, userId, feature.key]);
10371041
return Value.of(op, {
1038-
result: rolloutDecisionResult,
1042+
result: {
1043+
...rolloutDecisionResult,
1044+
holdout: {
1045+
experiment: activeHoldout!,
1046+
variation: activeHoldoutDecision!.variation!,
1047+
},
1048+
},
10391049
reasons: decideReasons,
10401050
});
10411051
}
@@ -1044,7 +1054,15 @@ export class DecisionService {
10441054
decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]);
10451055

10461056
return Value.of(op, {
1047-
result: activeHoldoutDecision,
1057+
result: {
1058+
experiment: null,
1059+
variation: null,
1060+
decisionSource: DECISION_SOURCES.ROLLOUT,
1061+
holdout: {
1062+
experiment: activeHoldout!,
1063+
variation: activeHoldoutDecision!.variation!,
1064+
},
1065+
},
10481066
reasons: decideReasons,
10491067
});
10501068
}

lib/optimizely/index.spec.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,52 @@ describe('Optimizely', () => {
872872
}),
873873
});
874874
});
875+
876+
it('should dispatch separate holdout impression when decisionObj.holdout is populated', async () => {
877+
const processSpy = vi.spyOn(eventProcessor, 'process');
878+
879+
const holdoutExperiment = projectConfig.holdouts[0];
880+
const holdoutVariation = projectConfig.holdouts[0].variations[0];
881+
const rolloutExperiment = projectConfig.experimentKeyMap['delivery_1'] || Object.values(projectConfig.experimentIdMap)[0];
882+
const rolloutVariation = rolloutExperiment?.variations?.[0] || { id: 'test_var_id', key: 'test_var_key', variables: [] };
883+
884+
vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => {
885+
return Value.of('async', [{
886+
error: false,
887+
result: {
888+
variation: rolloutVariation,
889+
experiment: rolloutExperiment,
890+
decisionSource: DECISION_SOURCES.ROLLOUT,
891+
holdout: {
892+
experiment: holdoutExperiment,
893+
variation: holdoutVariation,
894+
},
895+
},
896+
reasons: [],
897+
}]);
898+
});
899+
900+
const user = new OptimizelyUserContext({
901+
optimizely,
902+
userId: 'test_user',
903+
attributes: {},
904+
});
905+
906+
await optimizely.decideAsync(user, 'flag_1', []);
907+
908+
// The holdout impression is dispatched even when the main rollout impression is not
909+
// (sendFlagDecisions not set). Verify at least one impression is a holdout event.
910+
expect(processSpy).toHaveBeenCalled();
911+
912+
const holdoutEvent = processSpy.mock.calls.find(
913+
(call: any) => (call[0] as ImpressionEvent).ruleType === 'holdout'
914+
);
915+
expect(holdoutEvent).toBeDefined();
916+
const holdoutImpression = holdoutEvent![0] as ImpressionEvent;
917+
expect(holdoutImpression.ruleKey).toBe('holdout_test_key');
918+
expect(holdoutImpression.ruleType).toBe('holdout');
919+
expect(holdoutImpression.enabled).toBe(false);
920+
});
875921
});
876922

877923
it('should flush eventProcessor and odpManager on flushImmediately()', async () => {

lib/optimizely/index.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,6 +1542,15 @@ export default class Optimizely extends BaseService implements Client {
15421542
decisionEventDispatched = true;
15431543
}
15441544

1545+
if (decisionObj.holdout && !options[OptimizelyDecideOption.DISABLE_DECISION_EVENT]) {
1546+
const holdoutDecisionObj: DecisionObj = {
1547+
experiment: decisionObj.holdout.experiment,
1548+
variation: decisionObj.holdout.variation,
1549+
decisionSource: DECISION_SOURCES.HOLDOUT,
1550+
};
1551+
this.sendImpressionEvent(holdoutDecisionObj, key, userId, false, attributes);
1552+
}
1553+
15451554
const shouldIncludeReasons = options[OptimizelyDecideOption.INCLUDE_REASONS];
15461555

15471556
let reportedReasons: string[] = [];

0 commit comments

Comments
 (0)