[FSSDK-12080] remove holdouts feature toggle (#1110)

Author: raju-opti

lib/core/decision_service/index.spec.ts

@@ -13,40 +13,30 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { describe, it, expect, vi, MockInstance, beforeEach, afterEach } from 'vitest';
+import { USER_PROFILE_LOOKUP_ERROR, USER_PROFILE_SAVE_ERROR } from 'error_message';
+import {
+  SAVED_VARIATION_NOT_FOUND,
+  USER_HAS_NO_FORCED_VARIATION
+} from 'log_message';
+import { beforeEach, describe, expect, it, MockInstance, vi } from 'vitest';
 import { CMAB_DUMMY_ENTITY_ID, CMAB_FETCH_FAILED, DecisionService } from '.';
-import { getMockLogger } from '../../tests/mock/mock_logger';
 import OptimizelyUserContext from '../../optimizely_user_context';
-import { bucket } from '../bucketer';
-import { getTestProjectConfig, getTestProjectConfigWithFeatures } from '../../tests/test_data';
 import { createProjectConfig, ProjectConfig } from '../../project_config/project_config';
 import { BucketerParams, Experiment, ExperimentBucketMap, Holdout, OptimizelyDecideOption, UserAttributes, UserProfile } from '../../shared_types';
-import { CONTROL_ATTRIBUTES, DECISION_SOURCES } from '../../utils/enums';
 import { getDecisionTestDatafile } from '../../tests/decision_test_datafile';
+import { getMockLogger } from '../../tests/mock/mock_logger';
+import { getTestProjectConfig, getTestProjectConfigWithFeatures } from '../../tests/test_data';
+import { CONTROL_ATTRIBUTES, DECISION_SOURCES } from '../../utils/enums';
 import { Value } from '../../utils/promise/operation_value';
-import { 
-  USER_HAS_NO_FORCED_VARIATION,
-  VALID_BUCKETING_ID,
-  SAVED_USER_VARIATION,
-  SAVED_VARIATION_NOT_FOUND,
-} from 'log_message';
+import { bucket } from '../bucketer';
 import {
+  AUDIENCE_EVALUATION_RESULT_COMBINED,
+  EVALUATING_AUDIENCES_COMBINED,
   EXPERIMENT_NOT_RUNNING,
   RETURNING_STORED_VARIATION,
-  USER_NOT_IN_EXPERIMENT,
   USER_FORCED_IN_VARIATION,
-  EVALUATING_AUDIENCES_COMBINED,
-  AUDIENCE_EVALUATION_RESULT_COMBINED,
-  USER_IN_ROLLOUT,
-  USER_NOT_IN_ROLLOUT,
-  FEATURE_HAS_NO_EXPERIMENTS,
-  USER_DOESNT_MEET_CONDITIONS_FOR_TARGETING_RULE,
-  USER_NOT_BUCKETED_INTO_TARGETING_RULE,
-  USER_BUCKETED_INTO_TARGETING_RULE,
-  NO_ROLLOUT_EXISTS,
-  USER_MEETS_CONDITIONS_FOR_TARGETING_RULE,
+  USER_NOT_IN_EXPERIMENT
 } from '../decision_service/index';
-import { BUCKETING_ID_NOT_STRING, USER_PROFILE_LOOKUP_ERROR, USER_PROFILE_SAVE_ERROR } from 'error_message';
 
 type MockLogger = ReturnType<typeof getMockLogger>;
 
@@ -114,14 +104,6 @@ vi.mock('../bucketer', () => ({
   bucket: mockBucket,
 }));
 
-// Mock the feature toggle for holdout tests
-const mockHoldoutToggle = vi.hoisted(() => vi.fn());
-
-vi.mock('../../feature_toggle', () => ({
-  holdout: mockHoldoutToggle,
-}));
-
-
 const cloneDeep = (d: any) => JSON.parse(JSON.stringify(d));
 
 const testData = getTestProjectConfig();
@@ -1956,7 +1938,6 @@ describe('DecisionService', () => {
 
     describe('holdout', () => {
       beforeEach(async() => {
-        mockHoldoutToggle.mockReturnValue(true);
         const actualBucketModule = (await vi.importActual('../bucketer')) as { bucket: typeof bucket };
         mockBucket.mockImplementation(actualBucketModule.bucket);
       });

lib/core/decision_service/index.ts

@@ -75,7 +75,6 @@ import { OptimizelyError } from '../../error/optimizly_error';
 import { CmabService } from './cmab/cmab_service';
 import { Maybe, OpType, OpValue } from '../../utils/type';
 import { Value } from '../../utils/promise/operation_value';
-import * as featureToggle from '../../feature_toggle';
 
 export const EXPERIMENT_NOT_RUNNING = 'Experiment %s is not running.';
 export const RETURNING_STORED_VARIATION =
@@ -940,19 +939,17 @@ export class DecisionService {
         reasons: decideReasons,
       });
     }
-    if (featureToggle.holdout()) {
-      const holdouts = getHoldoutsForFlag(configObj, feature.key);
-
-      for (const holdout of holdouts) {
-        const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user);
-        decideReasons.push(...holdoutDecision.reasons);
-
-        if (holdoutDecision.result.variation) {
-          return Value.of(op, {
-            result: holdoutDecision.result,
-            reasons: decideReasons,
-          });
-        }
+    const holdouts = getHoldoutsForFlag(configObj, feature.key);
+
+    for (const holdout of holdouts) {
+      const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user);
+      decideReasons.push(...holdoutDecision.reasons);
+
+      if (holdoutDecision.result.variation) {
+        return Value.of(op, {
+          result: holdoutDecision.result,
+          reasons: decideReasons,
+        });
       }
     }
 

lib/feature_toggle.ts

@@ -31,6 +31,7 @@
  * flag and all associated checks can be removed from the codebase.
  */
 
-export const holdout = () => false as const;
+// example feature flag definition
+// export const wipFeat = () => false as const;
 
 export type IfActive<T extends () => boolean, Y, N = unknown> = ReturnType<T> extends true ? Y : N;

lib/notification_center/type.ts

@@ -26,7 +26,6 @@ import {
 } from '../shared_types';
 import { DecisionSource } from '../utils/enums';
 import { Nullable } from '../utils/type';
-import { holdout, IfActive } from '../feature_toggle';
 
 export type UserEventListenerPayload = {
   userId: string;
@@ -35,7 +34,7 @@ export type UserEventListenerPayload = {
 
 export type ActivateListenerPayload = UserEventListenerPayload & {
   experiment: Experiment | null;
-  holdout: IfActive<typeof holdout, Holdout | null>;
+  holdout: Holdout | null;
   variation: Variation | null;
   logEvent: LogEvent;
 }

lib/optimizely/index.tests.js

@@ -67,7 +67,6 @@ import {
 
 import { USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP } from '../core/bucketer';
 import { resolvablePromise } from '../utils/promise/resolvablePromise';
-import { holdout } from '../feature_toggle';
 
 var LOG_LEVEL = enums.LOG_LEVEL;
 var DECISION_SOURCES = enums.DECISION_SOURCES;

lib/project_config/project_config.spec.ts

@@ -13,38 +13,28 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { describe, it, expect, beforeEach, afterEach, vi, assert, Mock, beforeAll, afterAll } from 'vitest';
-import { sprintf } from '../utils/fns';
-import { keyBy } from '../utils/fns';
-import projectConfig, { ProjectConfig, getHoldoutsForFlag } from './project_config';
-import { FEATURE_VARIABLE_TYPES, LOG_LEVEL } from '../utils/enums';
-import testDatafile from '../tests/test_data';
-import configValidator from '../utils/config_validator';
 import {
+  FEATURE_NOT_IN_DATAFILE,
   INVALID_EXPERIMENT_ID,
   INVALID_EXPERIMENT_KEY,
+  UNABLE_TO_CAST_VALUE,
   UNEXPECTED_RESERVED_ATTRIBUTE_PREFIX,
   UNRECOGNIZED_ATTRIBUTE,
   VARIABLE_KEY_NOT_IN_DATAFILE,
-  FEATURE_NOT_IN_DATAFILE,
-  UNABLE_TO_CAST_VALUE,
 } from 'error_message';
-import { getMockLogger } from '../tests/mock/mock_logger';
-import { VariableType } from '../shared_types';
+import { Mock, afterAll, afterEach, assert, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
 import { OptimizelyError } from '../error/optimizly_error';
-import { mock } from 'node:test';
+import { VariableType } from '../shared_types';
+import { getMockLogger } from '../tests/mock/mock_logger';
+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';
 
-const buildLogMessageFromArgs = (args: any[]) => sprintf(args[1], ...args.splice(2));
 const cloneDeep = (obj: any) => JSON.parse(JSON.stringify(obj));
 const logger = getMockLogger();
 
-const mockHoldoutToggle = vi.hoisted(() => vi.fn());
-
-vi.mock('../feature_toggle', () => {
-  return {
-    holdout: mockHoldoutToggle,
-  };
-});
 
 describe('createProjectConfig', () => {
   let configObj: ProjectConfig;
@@ -383,20 +373,10 @@ const getHoldoutDatafile = () => {
   return datafile;
 }
 
-describe('createProjectConfig - holdouts, feature toggle is on', () => {
-  beforeAll(() => {
-    mockHoldoutToggle.mockReturnValue(true);
-  });
-
-  afterAll(() => {
-    mockHoldoutToggle.mockReset();
-  });
-
+describe('createProjectConfig - holdouts', () => {
   it('should populate holdouts fields correctly', function() {
     const datafile = getHoldoutDatafile();
 
-    mockHoldoutToggle.mockReturnValue(true);
-
     const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile)));
 
     expect(configObj.holdouts).toHaveLength(3);
@@ -456,15 +436,7 @@ describe('createProjectConfig - holdouts, feature toggle is on', () => {
   });
 });
 
-describe('getHoldoutsForFlag: feature toggle is on', () => {
-    beforeAll(() => {
-    mockHoldoutToggle.mockReturnValue(true);
-  });
-
-  afterAll(() => {
-    mockHoldoutToggle.mockReset();
-  });
-
+describe('getHoldoutsForFlag', () => {
   it('should return all applicable holdouts for a flag', () => {
     const datafile = getHoldoutDatafile();
     const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile)));

lib/project_config/project_config.ts

@@ -52,7 +52,6 @@ import {
 } from 'error_message';
 import { SKIPPING_JSON_VALIDATION, VALID_DATAFILE } from 'log_message';
 import { OptimizelyError } from '../error/optimizly_error';
-import * as featureToggle from '../feature_toggle';
 
 interface TryCreatingProjectConfigConfig {
   // TODO[OASIS-6649]: Don't use object type
@@ -345,10 +344,6 @@ export const createProjectConfig = function(datafileObj?: JSON, datafileStr: str
 };
 
 const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => {
-  if (!featureToggle.holdout()) {
-    return;
-  }
-
   projectConfig.holdouts = projectConfig.holdouts || [];
   projectConfig.holdoutIdMap = keyBy(projectConfig.holdouts, 'id');
   projectConfig.globalHoldouts = [];

package.json

@@ -66,8 +66,8 @@
     "test-umdbrowser": "npm run build-browser-umd && karma start karma.umd.conf.js --single-run",
     "test-karma-local": "karma start karma.local_chrome.bs.conf.js && npm run build-browser-umd && karma start karma.local_chrome.umd.conf.js",
     "prebuild": "npm run clean",
-    "build": "npm run genmsg && rollup -c && cp dist/index.browser.d.ts dist/index.d.ts",
-    "build:win": "npm run genmsg && rollup -c && type nul > dist/optimizely.lite.es.d.ts && type nul > dist/optimizely.lite.es.min.d.ts && type nul > dist/optimizely.lite.min.d.ts",
+    "build": "tsc --noEmit && npm run genmsg && rollup -c && cp dist/index.browser.d.ts dist/index.d.ts",
+    "build:win": "tsc --noEmit && npm run genmsg && rollup -c && type nul > dist/optimizely.lite.es.d.ts && type nul > dist/optimizely.lite.es.min.d.ts && type nul > dist/optimizely.lite.min.d.ts",
     "build-browser-umd": "rollup -c --config-umd",
     "coveralls": "nyc --reporter=lcov npm test",
     "prepare": "npm run build",