cleanup

Author: raju-opti

lib/core/decision_service/index.spec.ts

@@ -1960,63 +1960,15 @@ describe('DecisionService', () => {
         });
       });
 
-      it("should consider global holdout even if local holdout is present", async () => {
-        const { decisionService } = getDecisionService();
-        const datafile = getHoldoutTestDatafile();
-        const newEntry = {
-          id: 'holdout_included_id',
-          key: 'holdout_included',
-          status: 'Running',
-          includedFlags: ['1001'],
-          excludedFlags: [],
-          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,
-            },
-          ],
-        };
-        datafile.holdouts = [newEntry, ...datafile.holdouts];
-        const config = createProjectConfig(datafile);
-        const user = new OptimizelyUserContext({
-          optimizely: {} as any,
-          userId: 'tester',
-          attributes: {
-            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();
-
-        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 () => {
+      it("should consider next global holdout if misses previous holdouts", async () => {
         const { decisionService } = getDecisionService();
         const datafile = getHoldoutTestDatafile();
 
         datafile.holdouts.push({
           id: 'holdout_included_specific_id',
           key: 'holdout_included_specific',
           status: 'Running',
-          includedFlags: ['1001'],
+          includedFlags: [],
           excludedFlags: [], 
           audienceIds: ['4002'], // age_60 audience (age <= 60)
           audienceConditions: ['or', '4002'],
@@ -2039,7 +1991,7 @@ describe('DecisionService', () => {
           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)
+            age: 50, // Does not satisfy first global holdout (age_22, age <= 22) but satisfies newly added holdout (age_60, age <= 60)
           },
         });
         const feature = config.featureKeyMap['flag_1'];
@@ -2195,42 +2147,6 @@ describe('DecisionService', () => {
         });
       });
 
-      it('should skip holdouts excluded for specific flags', async () => {
-        const { decisionService } = getDecisionService();
-        const datafile = getHoldoutTestDatafile();
-        
-        datafile.holdouts = datafile.holdouts.map((holdout: any) => {
-          if(holdout.id === 'holdout_running_id') {
-            return {
-              ...holdout,
-              excludedFlags: ['1001']
-            }
-          }
-          return holdout;
-        });
-
-        const config = createProjectConfig(datafile);
-        const user = new OptimizelyUserContext({
-          optimizely: {} as any,
-          userId: 'tester',
-          attributes: {
-            age: 15, // satisfies age_22 audience condition (age <= 22) for global holdout, but holdout excludes flag_1
-          },
-        });
-        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_1'],
-          variation: config.variationIdMap['5001'],
-          decisionSource: DECISION_SOURCES.FEATURE_TEST,
-        });
-      });
-
       it('should handle multiple holdouts and use first matching one', async () => {
         const { decisionService } = getDecisionService();
         const datafile = getHoldoutTestDatafile();

lib/core/decision_service/index.ts

@@ -31,7 +31,6 @@ import {
   getVariationKeyFromId,
   isActive,
   ProjectConfig,
-  getHoldoutsForFlag,
 } from '../../project_config/project_config';
 import { AudienceEvaluator, createAudienceEvaluator } from '../audience_evaluator';
 import * as stringValidator from '../../utils/string_value_validator';
@@ -943,7 +942,10 @@ export class DecisionService {
         reasons: decideReasons,
       });
     }
-    const holdouts = getHoldoutsForFlag(configObj, feature.key);
+
+    // all global holouts should be evaluated for all flags
+    // global holdouts are available in configObj.holdouts
+    const { holdouts } = configObj;
 
     for (const holdout of holdouts) {
       const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user);

lib/project_config/project_config.spec.ts

@@ -30,7 +30,7 @@ import testDatafile from '../tests/test_data';
 import configValidator from '../utils/config_validator';
 import { FEATURE_VARIABLE_TYPES } from '../utils/enums';
 import { keyBy, sprintf } from '../utils/fns';
-import projectConfig, { ProjectConfig, getHoldoutsForFlag } from './project_config';
+import projectConfig, { ProjectConfig } from './project_config';
 
 const cloneDeep = (obj: any) => JSON.parse(JSON.stringify(obj));
 const logger = getMockLogger();
@@ -392,8 +392,6 @@ describe('createProjectConfig - holdouts', () => {
       holdout_id_2: configObj.holdouts[1],
       holdout_id_3: configObj.holdouts[2],
     });
-
-    expect(configObj.flagHoldoutsMap).toEqual({});
   });
 
   it('should handle empty holdouts array', function() {
@@ -403,27 +401,6 @@ describe('createProjectConfig - holdouts', () => {
 
     expect(configObj.holdouts).toEqual([]);
     expect(configObj.holdoutIdMap).toEqual({});
-    expect(configObj.flagHoldoutsMap).toEqual({});
-  });
-});
-
-describe('getHoldoutsForFlag', () => {
-  it('should return all holdouts for any flag', () => {
-    const datafile = getHoldoutDatafile();
-    const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile)));
-
-    // All holdouts now apply globally to all flags
-    const feature1Holdouts = getHoldoutsForFlag(configObj, 'feature_1');
-    expect(feature1Holdouts).toHaveLength(3);
-    expect(feature1Holdouts).toEqual(configObj.holdouts);
-
-    const feature2Holdouts = getHoldoutsForFlag(configObj, 'feature_2');
-    expect(feature2Holdouts).toHaveLength(3);
-    expect(feature2Holdouts).toEqual(configObj.holdouts);
-
-    const feature3Holdouts = getHoldoutsForFlag(configObj, 'feature_3');
-    expect(feature3Holdouts).toHaveLength(3);
-    expect(feature3Holdouts).toEqual(configObj.holdouts);
   });
 });
 

lib/project_config/project_config.ts

@@ -113,7 +113,6 @@ export interface ProjectConfig {
   odpIntegrationConfig: OdpIntegrationConfig;
   holdouts: Holdout[];
   holdoutIdMap?: { [id: string]: Holdout };
-  flagHoldoutsMap: { [key: string]: Holdout[]; }
 }
 
 const EXPERIMENT_RUNNING_STATUS = 'Running';
@@ -387,24 +386,13 @@ const getEveryoneElseVariation = function(
 const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => {
   projectConfig.holdouts = projectConfig.holdouts || [];
   projectConfig.holdoutIdMap = keyBy(projectConfig.holdouts, 'id');
-  projectConfig.flagHoldoutsMap = {};
 
   projectConfig.holdouts.forEach((holdout) => {
     holdout.variationKeyMap = keyBy(holdout.variations, 'key');
     assignBy(holdout.variations, 'id', projectConfig.variationIdMap);
   });
 }
 
-export const getHoldoutsForFlag = (projectConfig: ProjectConfig, flagKey: string): Holdout[] => {
-  if (projectConfig.flagHoldoutsMap[flagKey]) {
-    return projectConfig.flagHoldoutsMap[flagKey];
-  }
-
-  // All holdouts now apply to all flags (global holdouts)
-  projectConfig.flagHoldoutsMap[flagKey] = projectConfig.holdouts;
-  return projectConfig.holdouts;
-}
-
 /**
  * Extract all audience segments used in this audience's conditions
  * @param  {Audience}     audience  Object representing the audience being parsed