Skip to content

Commit 7b5dc43

Browse files
junaed-optimizelyraju-opti
authored andcommitted
[FSSDK-11528] Holdout support in decision service (#1075)
1 parent fb5e012 commit 7b5dc43

File tree

6 files changed

+91
-15
lines changed

6 files changed

+91
-15
lines changed

lib/feature_toggle.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,6 @@
3131
* flag and all associated checks can be removed from the codebase.
3232
*/
3333

34-
export const holdout = () => true;
34+
export const holdout = () => false as const;
35+
36+
export type IfActive<T extends () => boolean, Y, N = unknown> = ReturnType<T> extends true ? Y : N;

lib/notification_center/type.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,16 @@ import {
2626
} from '../shared_types';
2727
import { DecisionSource } from '../utils/enums';
2828
import { Nullable } from '../utils/type';
29+
import { holdout, IfActive } from '../feature_toggle';
2930

3031
export type UserEventListenerPayload = {
3132
userId: string;
3233
attributes?: UserAttributes;
3334
}
3435

3536
export type ActivateListenerPayload = UserEventListenerPayload & {
36-
experiment: Experiment | Holdout | null;
37+
experiment: Experiment | null;
38+
holdout: IfActive<typeof holdout, Holdout | null>;
3739
variation: Variation | null;
3840
logEvent: LogEvent;
3941
}

lib/optimizely/index.spec.ts

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,8 @@ describe('Optimizely', () => {
249249
let projectConfig: any;
250250
let optimizely: any;
251251
let decisionService: any;
252-
let notificationSpy: any;
252+
let flagNotificationSpy: any;
253+
let activateNotificationSpy: any;
253254
let eventProcessor: any;
254255

255256
beforeEach(() => {
@@ -282,10 +283,16 @@ describe('Optimizely', () => {
282283
decisionService = optimizely.decisionService;
283284

284285
// Setup notification spy
285-
notificationSpy = vi.fn();
286+
flagNotificationSpy = vi.fn();
286287
optimizely.notificationCenter.addNotificationListener(
287288
NOTIFICATION_TYPES.DECISION,
288-
notificationSpy
289+
flagNotificationSpy
290+
);
291+
292+
activateNotificationSpy = vi.fn();
293+
optimizely.notificationCenter.addNotificationListener(
294+
NOTIFICATION_TYPES.ACTIVATE,
295+
activateNotificationSpy
289296
);
290297
});
291298

@@ -408,7 +415,7 @@ describe('Optimizely', () => {
408415
expect(decision.ruleKey).toBe('holdout_test_key');
409416

410417
// Verify decision notification was sent
411-
expect(notificationSpy).toHaveBeenCalledWith({
418+
expect(flagNotificationSpy).toHaveBeenCalledWith({
412419
type: DECISION_NOTIFICATION_TYPES.FLAG,
413420
userId: 'test_user',
414421
attributes: { country: 'US' },
@@ -422,6 +429,14 @@ describe('Optimizely', () => {
422429
decisionEventDispatched: true,
423430
}),
424431
});
432+
433+
expect(activateNotificationSpy).toHaveBeenCalledWith(expect.objectContaining({
434+
experiment: null,
435+
holdout: projectConfig.holdouts[0],
436+
userId: 'test_user',
437+
attributes: { country: 'US' },
438+
variation: projectConfig.holdouts[0].variations[0]
439+
}));
425440
});
426441

427442
it('should handle holdout with included flags', async () => {
@@ -455,7 +470,7 @@ describe('Optimizely', () => {
455470
expect(decision.variationKey).toBe('holdout_variation_key');
456471

457472
// Verify notification shows holdout details
458-
expect(notificationSpy).toHaveBeenCalledWith({
473+
expect(flagNotificationSpy).toHaveBeenCalledWith({
459474
type: DECISION_NOTIFICATION_TYPES.FLAG,
460475
userId: 'test_user',
461476
attributes: { country: 'US' },
@@ -465,6 +480,14 @@ describe('Optimizely', () => {
465480
ruleKey: 'holdout_test_key',
466481
}),
467482
});
483+
484+
expect(activateNotificationSpy).toHaveBeenCalledWith(expect.objectContaining({
485+
experiment: null,
486+
holdout: modifiedHoldout,
487+
userId: 'test_user',
488+
attributes: { country: 'US' },
489+
variation: modifiedHoldout.variations[0]
490+
}));
468491
});
469492

470493
it('should handle holdout with excluded flags', async () => {
@@ -499,7 +522,7 @@ describe('Optimizely', () => {
499522
expect(decision.variationKey).toBe('variation_3');
500523

501524
// Verify notification shows normal experiment details (not holdout)
502-
expect(notificationSpy).toHaveBeenCalledWith({
525+
expect(flagNotificationSpy).toHaveBeenCalledWith({
503526
type: DECISION_NOTIFICATION_TYPES.FLAG,
504527
userId: 'test_user',
505528
attributes: { country: 'BD', age: 80 },
@@ -509,6 +532,14 @@ describe('Optimizely', () => {
509532
ruleKey: 'exp_3',
510533
}),
511534
});
535+
536+
expect(activateNotificationSpy).toHaveBeenCalledWith(expect.objectContaining({
537+
experiment: projectConfig.experimentKeyMap['exp_3'],
538+
holdout: null,
539+
userId: 'test_user',
540+
attributes: { country: 'BD', age: 80 },
541+
variation: projectConfig.variationIdMap['5003']
542+
}));
512543
});
513544

514545
it('should handle multiple holdouts with correct priority', async () => {
@@ -568,7 +599,7 @@ describe('Optimizely', () => {
568599
expect(decision.variationKey).toBe('holdout_variation_key_2');
569600

570601
// Verify notification shows details of selected holdout
571-
expect(notificationSpy).toHaveBeenCalledWith({
602+
expect(flagNotificationSpy).toHaveBeenCalledWith({
572603
type: DECISION_NOTIFICATION_TYPES.FLAG,
573604
userId: 'test_user',
574605
attributes: { country: 'US' },
@@ -578,6 +609,14 @@ describe('Optimizely', () => {
578609
ruleKey: 'holdout_test_key_2',
579610
}),
580611
});
612+
613+
expect(activateNotificationSpy).toHaveBeenCalledWith(expect.objectContaining({
614+
experiment: null,
615+
holdout: holdout2,
616+
userId: 'test_user',
617+
attributes: { country: 'US' },
618+
variation: holdout2.variations[0]
619+
}));
581620
});
582621

583622
it('should respect sendFlagDecisions setting for holdout events - false', async () => {
@@ -744,7 +783,7 @@ describe('Optimizely', () => {
744783
expect(typeof decision.variables).toBe('object');
745784

746785
// Verify notification includes variable information
747-
expect(notificationSpy).toHaveBeenCalledWith({
786+
expect(flagNotificationSpy).toHaveBeenCalledWith({
748787
type: DECISION_NOTIFICATION_TYPES.FLAG,
749788
userId: 'test_user',
750789
attributes: { country: 'US' },
@@ -754,6 +793,14 @@ describe('Optimizely', () => {
754793
enabled: false,
755794
}),
756795
});
796+
797+
expect(activateNotificationSpy).toHaveBeenCalledWith(expect.objectContaining({
798+
experiment: null,
799+
holdout: projectConfig.holdouts[0],
800+
userId: 'test_user',
801+
attributes: { country: 'US' },
802+
variation: projectConfig.holdouts[0].variations[0]
803+
}));
757804
});
758805

759806
it('should handle disable decision event option for holdout', async () => {

lib/optimizely/index.tests.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ import {
6767

6868
import { USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP } from '../core/bucketer';
6969
import { resolvablePromise } from '../utils/promise/resolvablePromise';
70+
import { holdout } from '../feature_toggle';
7071

7172
var LOG_LEVEL = enums.LOG_LEVEL;
7273
var DECISION_SOURCES = enums.DECISION_SOURCES;
@@ -2281,6 +2282,7 @@ describe('lib/optimizely', function() {
22812282
var instanceExperiments = optlyInstance.projectConfigManager.getConfig().experiments;
22822283
var expectedArgument = {
22832284
experiment: instanceExperiments[0],
2285+
holdout: null,
22842286
userId: 'testUser',
22852287
attributes: undefined,
22862288
variation: instanceExperiments[0].variations[1],
@@ -2351,6 +2353,7 @@ describe('lib/optimizely', function() {
23512353
var instanceExperiments = optlyInstance.projectConfigManager.getConfig().experiments;
23522354
var expectedArgument = {
23532355
experiment: instanceExperiments[0],
2356+
holdout: null,
23542357
userId: 'testUser',
23552358
attributes: attributes,
23562359
variation: instanceExperiments[0].variations[1],

lib/optimizely/index.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
OptimizelyDecision,
3838
Client,
3939
UserProfileServiceAsync,
40+
isHoldout,
4041
} from '../shared_types';
4142
import { newErrorDecision } from '../optimizely_decision';
4243
import OptimizelyUserContext from '../optimizely_user_context';
@@ -62,7 +63,7 @@ import {
6263
import { Fn, Maybe, OpType } from '../utils/type';
6364
import { resolvablePromise } from '../utils/promise/resolvablePromise';
6465

65-
import { NOTIFICATION_TYPES, DecisionNotificationType, DECISION_NOTIFICATION_TYPES } from '../notification_center/type';
66+
import { NOTIFICATION_TYPES, DecisionNotificationType, DECISION_NOTIFICATION_TYPES, ActivateListenerPayload } from '../notification_center/type';
6667
import {
6768
FEATURE_NOT_IN_DATAFILE,
6869
INVALID_INPUT_FORMAT,
@@ -382,17 +383,29 @@ export default class Optimizely extends BaseService implements Client {
382383
this.eventProcessor.process(impressionEvent);
383384

384385
const logEvent = buildLogEvent([impressionEvent]);
385-
this.notificationCenter.sendNotifications(NOTIFICATION_TYPES.ACTIVATE, {
386-
experiment: decisionObj.experiment,
386+
387+
const activateNotificationPayload: ActivateListenerPayload = {
388+
experiment: null,
389+
holdout: null,
387390
userId: userId,
388391
attributes: attributes,
389392
variation: decisionObj.variation,
390393
logEvent,
391-
});
394+
};
395+
396+
if (decisionObj.experiment) {
397+
if (isHoldout(decisionObj.experiment)) {
398+
activateNotificationPayload.holdout = decisionObj.experiment;
399+
} else {
400+
activateNotificationPayload.experiment = decisionObj.experiment;
401+
}
402+
}
403+
404+
this.notificationCenter.sendNotifications(NOTIFICATION_TYPES.ACTIVATE, activateNotificationPayload);
392405
}
393406

394407
/**
395-
* Sends conversion event to Optimizely.
408+
* Sends conversion event to Optimizely.`
396409
* @param {string} eventKey
397410
* @param {string} userId
398411
* @param {UserAttributes} attributes

lib/shared_types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,15 @@ export interface Holdout extends ExperimentCore {
181181
excludedFlags: string[];
182182
}
183183

184+
export function isHoldout(obj: Experiment | Holdout): obj is Holdout {
185+
// Holdout has 'status', 'includedFlags', and 'excludedFlags' properties
186+
return (
187+
(obj as Holdout).status !== undefined &&
188+
Array.isArray((obj as Holdout).includeFlags) &&
189+
Array.isArray((obj as Holdout).excludeFlags)
190+
);
191+
}
192+
184193
export enum VariableType {
185194
BOOLEAN = 'boolean',
186195
DOUBLE = 'double',

0 commit comments

Comments
 (0)