diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index e410e5d5f..9fc9d89a1 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: ['1001'], + 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: ['1001'], + 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: ['1001'] } } 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/event_processor/event_builder/user_event.ts b/lib/event_processor/event_builder/user_event.ts index 5d098e87b..ae33d65da 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 @@ -184,8 +185,8 @@ export const buildImpressionEvent = function({ const variationKey = decision.getVariationKey(decisionObj); 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..0c7e2fc79 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -29,6 +29,35 @@ 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'; +import { NOTIFICATION_TYPES, DECISION_NOTIFICATION_TYPES } from '../notification_center/type'; + + +const holdoutData = [ + { + id: 'holdout_test_id', + key: 'holdout_test_key', + status: 'Running', + includedFlags: [], + excludedFlags: [], + 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 = { @@ -212,5 +241,588 @@ describe('Optimizely', () => { const event = processSpy.mock.calls[0][0] as ImpressionEvent; expect(event.cmabUuid).toBe('uuid-cmab'); }); + + + }); + + describe('holdout tests', () => { + let projectConfig: any; + let optimizely: any; + let decisionService: any; + let notificationSpy: any; + let eventProcessor: any; + + beforeEach(() => { + const datafile = getDecisionTestDatafile(); + datafile.holdouts = JSON.parse(JSON.stringify(holdoutData)); // Deep copy to avoid mutations + projectConfig = createProjectConfig(datafile); + + const projectConfigManager = getMockProjectConfigManager({ + initConfig: projectConfig, + }); + + const mockEventDispatcher = { + dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), + }; + eventProcessor = getForwardingEventProcessor(mockEventDispatcher); + + 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 + 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, + 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 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'); + + 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(); + 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.includedFlags = ['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.excludedFlags = ['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.excludedFlags = ['1001']; // exclude flag_1 + + const holdout2 = { + id: 'holdout_test_id_2', + key: 'holdout_test_key_2', + status: 'Running', + includedFlags: ['1001'], // include flag_1 + excludedFlags: [], + 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 includedFlags + 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); + 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 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], + 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.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); + const processSpy = vi.spyOn(eventProcessor, 'process'); + + const projectConfigManager = getMockProjectConfigManager({ + initConfig: projectConfig, + }); + + const optimizelyWithEventProcessor = new Optimizely({ + clientEngine: 'node-sdk', + projectConfigManager, + eventProcessor, + jsonSchemaValidator, + logger, + odpManager, + disposable: true, + 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 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 user = new OptimizelyUserContext({ + optimizely: optimizelyWithEventProcessor, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + const decision = await optimizelyWithEventProcessor.decideAsync(user, 'flag_1', [OptimizelyDecideOption.DISABLE_DECISION_EVENT]); + + 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, + }), + }); + }); }); }); 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); diff --git a/lib/project_config/project_config.spec.ts b/lib/project_config/project_config.spec.ts index 662488914..21d18940c 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: ['44829230000'], audienceIds: [], audienceConditions: [], variations: [ @@ -360,8 +360,8 @@ const getHoldoutDatafile = () => { id: 'holdout_id_3', key: 'holdout_3', status: 'Draft', - includeFlags: ['feature_1'], - excludeFlags: [], + includedFlags: ['4482920077'], + excludedFlags: [], audienceIds: [], audienceConditions: [], variations: [ @@ -415,16 +415,16 @@ 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({ - 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({}); @@ -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..efb50301e 100644 --- a/lib/project_config/project_config.ts +++ b/lib/project_config/project_config.ts @@ -360,13 +360,15 @@ const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => { projectConfig.excludedHoldouts = {}; projectConfig.flagHoldoutsMap = {}; + const featureFlagIdMap = keyBy(projectConfig.featureFlags, 'id'); + 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,22 +378,30 @@ 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) => { - 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.includeFlags.forEach((flagKey) => { - 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); - }); + }) } }); } 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 {