From 32b97194222f409e55134f883dbd72fa30dfcca0 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Fri, 8 Mar 2019 19:23:14 +0500 Subject: [PATCH 01/10] feat(api): OnDecision Notification Listener for isFeatureEnabled and getEnabledFeatures APIs. --- .../optimizely-sdk/lib/optimizely/index.js | 40 +++- .../lib/optimizely/index.tests.js | 209 +++++++++++++++++- .../optimizely-sdk/lib/utils/enums/index.js | 10 +- 3 files changed, 248 insertions(+), 11 deletions(-) diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index a2090225d..c238bd42d 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -34,6 +34,7 @@ var LOG_MESSAGES = enums.LOG_MESSAGES; var MODULE_NAME = 'OPTIMIZELY'; var DECISION_SOURCES = enums.DECISION_SOURCES; var FEATURE_VARIABLE_TYPES = enums.FEATURE_VARIABLE_TYPES; +var DECISION_INFO_TYPES = enums.DECISION_INFO_TYPES; /** * The Optimizely class @@ -463,20 +464,47 @@ Optimizely.prototype.isFeatureEnabled = function (featureKey, userId, attributes return false; } + var featureEnabled = false; + var experimentKey = null; + var variationKey = null; var decision = this.decisionService.getVariationForFeature(feature, userId, attributes); var variation = decision.variation; + if (!!variation) { + featureEnabled = variation.featureEnabled; if (decision.decisionSource === DECISION_SOURCES.EXPERIMENT) { + experimentKey = decision.experiment.key; + variationKey = decision.variation.key; // got a variation from the exp, so we track the impression this._sendImpressionEvent(decision.experiment.key, decision.variation.key, userId, attributes); } - if (variation.featureEnabled === true) { - this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.FEATURE_ENABLED_FOR_USER, MODULE_NAME, featureKey, userId)); - return true; - } } - this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.FEATURE_NOT_ENABLED_FOR_USER, MODULE_NAME, featureKey, userId)); - return false; + + if (!featureEnabled) { + this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.FEATURE_NOT_ENABLED_FOR_USER, MODULE_NAME, featureKey, userId)); + featureEnabled = false; + } else { + this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.FEATURE_ENABLED_FOR_USER, MODULE_NAME, featureKey, userId)); + } + + var decisionSource = decision.decisionSource || DECISION_SOURCES.ROLLOUT; + this.notificationCenter.sendNotifications( + enums.NOTIFICATION_TYPES.ON_DECISION, + { + type: DECISION_INFO_TYPES.FEATURE, + userId: userId, + attributes: attributes, + decisionInfo: { + featureKey: featureKey, + featureEnabled: featureEnabled, + source: decisionSource, + sourceExperimentKey: experimentKey, + sourceVariationKey: variationKey, + } + } + ); + + return featureEnabled; } catch (e) { this.logger.log(LOG_LEVEL.ERROR, e.message); this.errorHandler.handleError(e); diff --git a/packages/optimizely-sdk/lib/optimizely/index.tests.js b/packages/optimizely-sdk/lib/optimizely/index.tests.js index 1585653db..6914b9846 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.tests.js +++ b/packages/optimizely-sdk/lib/optimizely/index.tests.js @@ -40,6 +40,8 @@ var ERROR_MESSAGES = enums.ERROR_MESSAGES; var LOG_LEVEL = enums.LOG_LEVEL; var LOG_MESSAGES = enums.LOG_MESSAGES; var DECISION_SOURCES = enums.DECISION_SOURCES; +var NOTIFICATION_TYPES = enums.NOTIFICATION_TYPES; +var DECISION_INFO_TYPES = enums.DECISION_INFO_TYPES; describe('lib/optimizely', function() { describe('constructor', function() { @@ -2590,6 +2592,8 @@ describe('lib/optimizely', function() { }); var optlyInstance; var clock; + var onDecisionListener; + beforeEach(function() { optlyInstance = new Optimizely({ clientEngine: 'node-sdk', @@ -2608,6 +2612,7 @@ describe('lib/optimizely', function() { sandbox.stub(uuid, 'v4').returns('a68cf1ad-0393-4e18-af87-efe8f01a7c9c'); sandbox.stub(fns, 'currentTimestamp').returns(1509489766569); clock = sinon.useFakeTimers(new Date().getTime()); + onDecisionListener = sinon.spy(); }); afterEach(function() { @@ -2654,7 +2659,11 @@ describe('lib/optimizely', function() { }); }); - it('returns true and dispatches an impression event', function() { + it('returns true, sends notification and dispatches an impression event', function() { + optlyInstance.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.ON_DECISION, + onDecisionListener + ); var result = optlyInstance.isFeatureEnabled('test_feature_for_experiment', 'user1', attributes); assert.strictEqual(result, true); sinon.assert.calledOnce(optlyInstance.decisionService.getVariationForFeature); @@ -2665,6 +2674,21 @@ describe('lib/optimizely', function() { 'user1', attributes ); + + var expectedArguments = { + type: DECISION_INFO_TYPES.FEATURE, + userId: 'user1', + attributes: attributes, + decisionInfo: { + featureKey: 'test_feature_for_experiment', + featureEnabled: true, + source: DECISION_SOURCES.EXPERIMENT, + sourceExperimentKey: 'testing_my_feature', + sourceVariationKey: 'variation' + } + }; + sinon.assert.calledWith(onDecisionListener, expectedArguments); + sinon.assert.calledOnce(eventDispatcher.dispatchEvent); var expectedImpressionEvent = { 'httpVerb': 'POST', @@ -2820,6 +2844,10 @@ describe('lib/optimizely', function() { variation: variation, decisionSource: DECISION_SOURCES.EXPERIMENT, }); + optlyInstance.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.ON_DECISION, + onDecisionListener + ); result = optlyInstance.isFeatureEnabled('shared_feature', 'user1', attributes); }); @@ -2835,6 +2863,23 @@ describe('lib/optimizely', function() { ); }); + it('should send notification', function() { + assert.strictEqual(result, false); + var expectedArguments = { + type: DECISION_INFO_TYPES.FEATURE, + userId: 'user1', + attributes: attributes, + decisionInfo: { + featureKey: 'shared_feature', + featureEnabled: false, + source: DECISION_SOURCES.EXPERIMENT, + sourceExperimentKey: 'test_shared_feature', + sourceVariationKey: 'control' + } + }; + sinon.assert.calledWith(onDecisionListener, expectedArguments); + }); + it('should dispatch an impression event', function() { sinon.assert.calledOnce(eventDispatcher.dispatchEvent); var expectedImpressionEvent = { @@ -2933,13 +2978,30 @@ describe('lib/optimizely', function() { }); }); - it('returns true and does not dispatch an event', function() { + it('returns true sends notification and does not dispatch an event', function() { + optlyInstance.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.ON_DECISION, + onDecisionListener + ); var result = optlyInstance.isFeatureEnabled('test_feature', 'user1', { test_attribute: 'test_value', }); assert.strictEqual(result, true); sinon.assert.notCalled(eventDispatcher.dispatchEvent); sinon.assert.calledWith(createdLogger.log, LOG_LEVEL.INFO, 'OPTIMIZELY: Feature test_feature is enabled for user user1.'); + var expectedArguments = { + type: DECISION_INFO_TYPES.FEATURE, + userId: 'user1', + attributes: { test_attribute: 'test_value' }, + decisionInfo: { + featureKey: 'test_feature', + featureEnabled: true, + source: DECISION_SOURCES.ROLLOUT, + sourceExperimentKey: null, + sourceVariationKey: null + } + }; + sinon.assert.calledWith(onDecisionListener, expectedArguments); }); }); @@ -2955,12 +3017,30 @@ describe('lib/optimizely', function() { }); }); - it('returns false ', function() { + it('returns false and send notification', function() { + optlyInstance.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.ON_DECISION, + onDecisionListener + ); var result = optlyInstance.isFeatureEnabled('test_feature', 'user1', { test_attribute: 'test_value', }); assert.strictEqual(result, false); sinon.assert.calledWith(createdLogger.log, LOG_LEVEL.INFO, 'OPTIMIZELY: Feature test_feature is not enabled for user user1.'); + + var expectedArguments = { + type: DECISION_INFO_TYPES.FEATURE, + userId: 'user1', + attributes: { test_attribute: 'test_value' }, + decisionInfo: { + featureKey: 'test_feature', + featureEnabled: false, + source: DECISION_SOURCES.ROLLOUT, + sourceExperimentKey: null, + sourceVariationKey: null + } + }; + sinon.assert.calledWith(onDecisionListener, expectedArguments); }); }); }); @@ -2975,10 +3055,27 @@ describe('lib/optimizely', function() { }); it('returns false and does not dispatch an event', function() { + optlyInstance.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.ON_DECISION, + onDecisionListener + ); var result = optlyInstance.isFeatureEnabled('test_feature', 'user1'); assert.strictEqual(result, false); sinon.assert.notCalled(eventDispatcher.dispatchEvent); sinon.assert.calledWith(createdLogger.log, LOG_LEVEL.INFO, 'OPTIMIZELY: Feature test_feature is not enabled for user user1.'); + var expectedArguments = { + type: DECISION_INFO_TYPES.FEATURE, + userId: 'user1', + attributes: undefined, + decisionInfo: { + featureKey: 'test_feature', + featureEnabled: false, + source: DECISION_SOURCES.ROLLOUT, + sourceExperimentKey: null, + sourceVariationKey: null + } + }; + sinon.assert.calledWith(onDecisionListener, expectedArguments); }); }); }); @@ -3058,6 +3155,112 @@ describe('lib/optimizely', function() { attributes ); }); + + it('return features that are enabled for the user and send notification for every feature', function() { + optlyInstance = new Optimizely({ + clientEngine: 'node-sdk', + datafile: testData.getTestProjectConfigWithFeatures(), + eventBuilder: eventBuilder, + errorHandler: errorHandler, + eventDispatcher: eventDispatcher, + jsonSchemaValidator: jsonSchemaValidator, + logger: createdLogger, + isValidInstance: true, + }); + + var attributes = { test_attribute: 'test_value' }; + optlyInstance.notificationCenter.addNotificationListener(NOTIFICATION_TYPES.ON_DECISION, onDecisionListener); + var result = optlyInstance.getEnabledFeatures('test_user', attributes); + assert.strictEqual(result.length, 3); + assert.isAbove(result.indexOf('test_feature_2'), -1); + assert.isAbove(result.indexOf('test_feature_for_experiment'), -1); + assert.isAbove(result.indexOf('shared_feature'), -1); + + assert.isTrue(onDecisionListener.getCall(0).calledWith({ + type: DECISION_INFO_TYPES.FEATURE, + userId: 'test_user', + attributes: attributes, + decisionInfo: { + featureKey: 'test_feature', + featureEnabled: false, + source: DECISION_SOURCES.ROLLOUT, + sourceExperimentKey: null, + sourceVariationKey: null + } + })); + assert.isTrue(onDecisionListener.getCall(1).calledWith({ + type: DECISION_INFO_TYPES.FEATURE, + userId: 'test_user', + attributes: attributes, + decisionInfo: { + featureKey: 'test_feature_2', + featureEnabled: true, + source: DECISION_SOURCES.ROLLOUT, + sourceExperimentKey: null, + sourceVariationKey: null + } + })); + assert.isTrue(onDecisionListener.getCall(2).calledWith({ + type: DECISION_INFO_TYPES.FEATURE, + userId: 'test_user', + attributes: attributes, + decisionInfo: { + featureKey: 'test_feature_for_experiment', + featureEnabled: true, + source: DECISION_SOURCES.EXPERIMENT, + sourceExperimentKey: 'testing_my_feature', + sourceVariationKey: 'variation' + } + })); + assert.isTrue(onDecisionListener.getCall(3).calledWith({ + type: DECISION_INFO_TYPES.FEATURE, + userId: 'test_user', + attributes: attributes, + decisionInfo: { + featureKey: 'feature_with_group', + featureEnabled: false, + source: DECISION_SOURCES.ROLLOUT, + sourceExperimentKey: null, + sourceVariationKey: null + } + })); + assert.isTrue(onDecisionListener.getCall(4).calledWith({ + type: DECISION_INFO_TYPES.FEATURE, + userId: 'test_user', + attributes: attributes, + decisionInfo: { + featureKey: 'shared_feature', + featureEnabled: true, + source: DECISION_SOURCES.EXPERIMENT, + sourceExperimentKey: 'test_shared_feature', + sourceVariationKey: 'treatment' + } + })); + assert.isTrue(onDecisionListener.getCall(5).calledWith({ + type: DECISION_INFO_TYPES.FEATURE, + userId: 'test_user', + attributes: attributes, + decisionInfo: { + featureKey: 'unused_flag', + featureEnabled: false, + source: DECISION_SOURCES.ROLLOUT, + sourceExperimentKey: null, + sourceVariationKey: null + } + })); + assert.isTrue(onDecisionListener.getCall(6).calledWith({ + type: DECISION_INFO_TYPES.FEATURE, + userId: 'test_user', + attributes: attributes, + decisionInfo: { + featureKey: 'feature_exp_no_traffic', + featureEnabled: false, + source: DECISION_SOURCES.ROLLOUT, + sourceExperimentKey: null, + sourceVariationKey: null + } + })); + }); }); describe('feature variable APIs', function() { diff --git a/packages/optimizely-sdk/lib/utils/enums/index.js b/packages/optimizely-sdk/lib/utils/enums/index.js index 91bc01871..ab6b380f9 100644 --- a/packages/optimizely-sdk/lib/utils/enums/index.js +++ b/packages/optimizely-sdk/lib/utils/enums/index.js @@ -175,8 +175,14 @@ exports.NODE_CLIENT_VERSION = '3.1.0-beta1'; * - logEvent {Object} */ exports.NOTIFICATION_TYPES = { - ACTIVATE: 'ACTIVATE:experiment, user_id,attributes, variation, event', - TRACK: 'TRACK:event_key, user_id, attributes, event_tags, event', + ACTIVATE: 'ACTIVATE:experiment, userId, attributes, variation, logEvent', + TRACK: 'TRACK:eventKey, userId, attributes, eventTags, logEvent', + ON_DECISION: 'ON_DECISION:type, userId, attributes, decisionInfo', +}; + +exports.DECISION_INFO_TYPES = { + EXPERIMENT: 'experiment', + FEATURE: 'feature', }; /* From 1b6e3beb33b6ec4aee114db542a3a0f33f6850c0 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Mon, 11 Mar 2019 22:18:00 +0500 Subject: [PATCH 02/10] Move decision listener tests inside notification listeners tests. --- .../lib/optimizely/index.tests.js | 295 ++++++++++++------ .../optimizely-sdk/lib/utils/enums/index.js | 13 +- 2 files changed, 209 insertions(+), 99 deletions(-) diff --git a/packages/optimizely-sdk/lib/optimizely/index.tests.js b/packages/optimizely-sdk/lib/optimizely/index.tests.js index 6914b9846..04ada6044 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.tests.js +++ b/packages/optimizely-sdk/lib/optimizely/index.tests.js @@ -2521,6 +2521,201 @@ describe('lib/optimizely', function() { }; sinon.assert.calledWith(trackListener, expectedArgument); }); + + describe('OnDecision Listener', function() { + var onDecisionListener; + beforeEach(function() { + onDecisionListener = sinon.spy(); + }); + + describe('feature management', function() { + var sandbox = sinon.sandbox.create(); + var experiment; + var variation; + + beforeEach(function() { + optlyInstance = new Optimizely({ + clientEngine: 'node-sdk', + datafile: testData.getTestProjectConfigWithFeatures(), + eventBuilder: eventBuilder, + errorHandler: errorHandler, + eventDispatcher: eventDispatcher, + jsonSchemaValidator: jsonSchemaValidator, + logger: createdLogger, + isValidInstance: true, + }); + + optlyInstance.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.ON_DECISION, + onDecisionListener + ); + }); + + afterEach(function() { + sandbox.restore(); + }); + + describe('isFeatureEnabled', function() { + describe('when the user bucketed into a variation of an experiment of the feature', function() { + var attributes = { test_attribute: 'test_value' }; + + describe('when the variation is toggled ON', function() { + beforeEach(function() { + experiment = optlyInstance.configObj.experimentKeyMap.testing_my_feature; + variation = experiment.variations[0]; + sandbox.stub(optlyInstance.decisionService, 'getVariationForFeature').returns({ + experiment: experiment, + variation: variation, + decisionSource: DECISION_SOURCES.EXPERIMENT, + }); + }); + + it('should return true and send notification', function() { + var result = optlyInstance.isFeatureEnabled('test_feature_for_experiment', 'user1', attributes); + assert.strictEqual(result, true); + sinon.assert.calledWith(onDecisionListener, { + type: DECISION_INFO_TYPES.FEATURE, + userId: 'user1', + attributes: attributes, + decisionInfo: { + featureKey: 'test_feature_for_experiment', + featureEnabled: true, + source: DECISION_SOURCES.EXPERIMENT, + sourceExperimentKey: 'testing_my_feature', + sourceVariationKey: 'variation' + } + }); + }); + }); + + describe('when the variation is toggled OFF', function() { + beforeEach(function() { + var experiment = optlyInstance.configObj.experimentKeyMap.test_shared_feature; + var variation = experiment.variations[1]; + sandbox.stub(optlyInstance.decisionService, 'getVariationForFeature').returns({ + experiment: experiment, + variation: variation, + decisionSource: DECISION_SOURCES.EXPERIMENT, + }); + }); + + it('should return false and send notification', function() { + var result = optlyInstance.isFeatureEnabled('shared_feature', 'user1', attributes); + assert.strictEqual(result, false); + sinon.assert.calledWith(onDecisionListener, { + type: DECISION_INFO_TYPES.FEATURE, + userId: 'user1', + attributes: attributes, + decisionInfo: { + featureKey: 'shared_feature', + featureEnabled: false, + source: DECISION_SOURCES.EXPERIMENT, + sourceExperimentKey: 'test_shared_feature', + sourceVariationKey: 'control' + } + }); + }); + }); + }); + + describe('user bucketed into a variation of a rollout of the feature', function() { + describe('when the variation is toggled ON', function() { + beforeEach(function() { + // This experiment is the first audience targeting rule in the rollout of feature 'test_feature' + var experiment = optlyInstance.configObj.experimentKeyMap['594031']; + var variation = experiment.variations[0]; + sandbox.stub(optlyInstance.decisionService, 'getVariationForFeature').returns({ + experiment: experiment, + variation: variation, + decisionSource: DECISION_SOURCES.ROLLOUT, + }); + }); + + it('should return true and send notification', function() { + var result = optlyInstance.isFeatureEnabled('test_feature', 'user1', { + test_attribute: 'test_value', + }); + assert.strictEqual(result, true); + sinon.assert.calledWith(onDecisionListener, { + type: DECISION_INFO_TYPES.FEATURE, + userId: 'user1', + attributes: { test_attribute: 'test_value' }, + decisionInfo: { + featureKey: 'test_feature', + featureEnabled: true, + source: DECISION_SOURCES.ROLLOUT, + sourceExperimentKey: null, + sourceVariationKey: null + } + }); + }); + }); + + describe('when the variation is toggled OFF', function() { + beforeEach(function() { + // This experiment is the second audience targeting rule in the rollout of feature 'test_feature' + var experiment = optlyInstance.configObj.experimentKeyMap['594037']; + var variation = experiment.variations[0]; + sandbox.stub(optlyInstance.decisionService, 'getVariationForFeature').returns({ + experiment: experiment, + variation: variation, + decisionSource: DECISION_SOURCES.ROLLOUT, + }); + }); + + it('returns false and send notification', function() { + var result = optlyInstance.isFeatureEnabled('test_feature', 'user1', { + test_attribute: 'test_value', + }); + assert.strictEqual(result, false); + sinon.assert.calledWith(createdLogger.log, LOG_LEVEL.INFO, 'OPTIMIZELY: Feature test_feature is not enabled for user user1.'); + + var expectedArguments = { + type: DECISION_INFO_TYPES.FEATURE, + userId: 'user1', + attributes: { test_attribute: 'test_value' }, + decisionInfo: { + featureKey: 'test_feature', + featureEnabled: false, + source: DECISION_SOURCES.ROLLOUT, + sourceExperimentKey: null, + sourceVariationKey: null + } + }; + sinon.assert.calledWith(onDecisionListener, expectedArguments); + }); + }); + }); + + describe('user not bucketed into an experiment or a rollout', function() { + beforeEach(function() { + sandbox.stub(optlyInstance.decisionService, 'getVariationForFeature').returns({ + experiment: null, + variation: null, + decisionSource: null, + }); + }); + + it('returns false and send notification', function() { + var result = optlyInstance.isFeatureEnabled('test_feature', 'user1'); + assert.strictEqual(result, false); + sinon.assert.calledWith(onDecisionListener, { + type: DECISION_INFO_TYPES.FEATURE, + userId: 'user1', + attributes: undefined, + decisionInfo: { + featureKey: 'test_feature', + featureEnabled: false, + source: DECISION_SOURCES.ROLLOUT, + sourceExperimentKey: null, + sourceVariationKey: null + } + }); + }); + }); + }); + }); + }); }); }); @@ -2592,7 +2787,6 @@ describe('lib/optimizely', function() { }); var optlyInstance; var clock; - var onDecisionListener; beforeEach(function() { optlyInstance = new Optimizely({ @@ -2612,7 +2806,6 @@ describe('lib/optimizely', function() { sandbox.stub(uuid, 'v4').returns('a68cf1ad-0393-4e18-af87-efe8f01a7c9c'); sandbox.stub(fns, 'currentTimestamp').returns(1509489766569); clock = sinon.useFakeTimers(new Date().getTime()); - onDecisionListener = sinon.spy(); }); afterEach(function() { @@ -2659,11 +2852,7 @@ describe('lib/optimizely', function() { }); }); - it('returns true, sends notification and dispatches an impression event', function() { - optlyInstance.notificationCenter.addNotificationListener( - NOTIFICATION_TYPES.ON_DECISION, - onDecisionListener - ); + it('returns true and dispatches an impression event', function() { var result = optlyInstance.isFeatureEnabled('test_feature_for_experiment', 'user1', attributes); assert.strictEqual(result, true); sinon.assert.calledOnce(optlyInstance.decisionService.getVariationForFeature); @@ -2675,20 +2864,6 @@ describe('lib/optimizely', function() { attributes ); - var expectedArguments = { - type: DECISION_INFO_TYPES.FEATURE, - userId: 'user1', - attributes: attributes, - decisionInfo: { - featureKey: 'test_feature_for_experiment', - featureEnabled: true, - source: DECISION_SOURCES.EXPERIMENT, - sourceExperimentKey: 'testing_my_feature', - sourceVariationKey: 'variation' - } - }; - sinon.assert.calledWith(onDecisionListener, expectedArguments); - sinon.assert.calledOnce(eventDispatcher.dispatchEvent); var expectedImpressionEvent = { 'httpVerb': 'POST', @@ -2844,10 +3019,6 @@ describe('lib/optimizely', function() { variation: variation, decisionSource: DECISION_SOURCES.EXPERIMENT, }); - optlyInstance.notificationCenter.addNotificationListener( - NOTIFICATION_TYPES.ON_DECISION, - onDecisionListener - ); result = optlyInstance.isFeatureEnabled('shared_feature', 'user1', attributes); }); @@ -2863,23 +3034,6 @@ describe('lib/optimizely', function() { ); }); - it('should send notification', function() { - assert.strictEqual(result, false); - var expectedArguments = { - type: DECISION_INFO_TYPES.FEATURE, - userId: 'user1', - attributes: attributes, - decisionInfo: { - featureKey: 'shared_feature', - featureEnabled: false, - source: DECISION_SOURCES.EXPERIMENT, - sourceExperimentKey: 'test_shared_feature', - sourceVariationKey: 'control' - } - }; - sinon.assert.calledWith(onDecisionListener, expectedArguments); - }); - it('should dispatch an impression event', function() { sinon.assert.calledOnce(eventDispatcher.dispatchEvent); var expectedImpressionEvent = { @@ -2978,30 +3132,13 @@ describe('lib/optimizely', function() { }); }); - it('returns true sends notification and does not dispatch an event', function() { - optlyInstance.notificationCenter.addNotificationListener( - NOTIFICATION_TYPES.ON_DECISION, - onDecisionListener - ); + it('returns true and does not dispatch an event', function() { var result = optlyInstance.isFeatureEnabled('test_feature', 'user1', { test_attribute: 'test_value', }); assert.strictEqual(result, true); sinon.assert.notCalled(eventDispatcher.dispatchEvent); sinon.assert.calledWith(createdLogger.log, LOG_LEVEL.INFO, 'OPTIMIZELY: Feature test_feature is enabled for user user1.'); - var expectedArguments = { - type: DECISION_INFO_TYPES.FEATURE, - userId: 'user1', - attributes: { test_attribute: 'test_value' }, - decisionInfo: { - featureKey: 'test_feature', - featureEnabled: true, - source: DECISION_SOURCES.ROLLOUT, - sourceExperimentKey: null, - sourceVariationKey: null - } - }; - sinon.assert.calledWith(onDecisionListener, expectedArguments); }); }); @@ -3017,30 +3154,12 @@ describe('lib/optimizely', function() { }); }); - it('returns false and send notification', function() { - optlyInstance.notificationCenter.addNotificationListener( - NOTIFICATION_TYPES.ON_DECISION, - onDecisionListener - ); + it('returns false', function() { var result = optlyInstance.isFeatureEnabled('test_feature', 'user1', { test_attribute: 'test_value', }); assert.strictEqual(result, false); sinon.assert.calledWith(createdLogger.log, LOG_LEVEL.INFO, 'OPTIMIZELY: Feature test_feature is not enabled for user user1.'); - - var expectedArguments = { - type: DECISION_INFO_TYPES.FEATURE, - userId: 'user1', - attributes: { test_attribute: 'test_value' }, - decisionInfo: { - featureKey: 'test_feature', - featureEnabled: false, - source: DECISION_SOURCES.ROLLOUT, - sourceExperimentKey: null, - sourceVariationKey: null - } - }; - sinon.assert.calledWith(onDecisionListener, expectedArguments); }); }); }); @@ -3055,27 +3174,10 @@ describe('lib/optimizely', function() { }); it('returns false and does not dispatch an event', function() { - optlyInstance.notificationCenter.addNotificationListener( - NOTIFICATION_TYPES.ON_DECISION, - onDecisionListener - ); var result = optlyInstance.isFeatureEnabled('test_feature', 'user1'); assert.strictEqual(result, false); sinon.assert.notCalled(eventDispatcher.dispatchEvent); sinon.assert.calledWith(createdLogger.log, LOG_LEVEL.INFO, 'OPTIMIZELY: Feature test_feature is not enabled for user user1.'); - var expectedArguments = { - type: DECISION_INFO_TYPES.FEATURE, - userId: 'user1', - attributes: undefined, - decisionInfo: { - featureKey: 'test_feature', - featureEnabled: false, - source: DECISION_SOURCES.ROLLOUT, - sourceExperimentKey: null, - sourceVariationKey: null - } - }; - sinon.assert.calledWith(onDecisionListener, expectedArguments); }); }); }); @@ -3168,6 +3270,7 @@ describe('lib/optimizely', function() { isValidInstance: true, }); + var onDecisionListener = sinon.spy(); var attributes = { test_attribute: 'test_value' }; optlyInstance.notificationCenter.addNotificationListener(NOTIFICATION_TYPES.ON_DECISION, onDecisionListener); var result = optlyInstance.getEnabledFeatures('test_user', attributes); diff --git a/packages/optimizely-sdk/lib/utils/enums/index.js b/packages/optimizely-sdk/lib/utils/enums/index.js index ab6b380f9..fb89437b5 100644 --- a/packages/optimizely-sdk/lib/utils/enums/index.js +++ b/packages/optimizely-sdk/lib/utils/enums/index.js @@ -173,15 +173,22 @@ exports.NODE_CLIENT_VERSION = '3.1.0-beta1'; * - attributes {Object|undefined} * - eventTags {Object|undefined} * - logEvent {Object} + * + * ON_DECISION: A decision is made in the system. i.e. user activation, + * feature access or feature-variable value retrieval + * Callbacks will receive an object argument with the following properties: + * - type {string} + * - userId {string} + * - attributes {Object|undefined} + * - decisionInfo {Object|undefined} */ exports.NOTIFICATION_TYPES = { - ACTIVATE: 'ACTIVATE:experiment, userId, attributes, variation, logEvent', - TRACK: 'TRACK:eventKey, userId, attributes, eventTags, logEvent', + ACTIVATE: 'ACTIVATE:experiment, user_id,attributes, variation, event', + TRACK: 'TRACK:event_key, user_id, attributes, event_tags, event', ON_DECISION: 'ON_DECISION:type, userId, attributes, decisionInfo', }; exports.DECISION_INFO_TYPES = { - EXPERIMENT: 'experiment', FEATURE: 'feature', }; From 3b2b61bbfedddd049656a40029fac32454b7628b Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Mon, 11 Mar 2019 22:23:24 +0500 Subject: [PATCH 03/10] Remove unused variables. --- packages/optimizely-sdk/lib/optimizely/index.tests.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/optimizely-sdk/lib/optimizely/index.tests.js b/packages/optimizely-sdk/lib/optimizely/index.tests.js index 04ada6044..27837d89c 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.tests.js +++ b/packages/optimizely-sdk/lib/optimizely/index.tests.js @@ -2530,9 +2530,7 @@ describe('lib/optimizely', function() { describe('feature management', function() { var sandbox = sinon.sandbox.create(); - var experiment; - var variation; - + beforeEach(function() { optlyInstance = new Optimizely({ clientEngine: 'node-sdk', @@ -2561,8 +2559,8 @@ describe('lib/optimizely', function() { describe('when the variation is toggled ON', function() { beforeEach(function() { - experiment = optlyInstance.configObj.experimentKeyMap.testing_my_feature; - variation = experiment.variations[0]; + var experiment = optlyInstance.configObj.experimentKeyMap.testing_my_feature; + var variation = experiment.variations[0]; sandbox.stub(optlyInstance.decisionService, 'getVariationForFeature').returns({ experiment: experiment, variation: variation, From 26aef1ae8ae9a3747051df934d976705a3ee7875 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Tue, 12 Mar 2019 17:32:42 +0500 Subject: [PATCH 04/10] Address code review comments. --- .../optimizely-sdk/lib/optimizely/index.js | 4 +- .../lib/optimizely/index.tests.js | 96 +++++++++---------- .../optimizely-sdk/lib/utils/enums/index.js | 4 +- 3 files changed, 52 insertions(+), 52 deletions(-) diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index c238bd42d..c62e1996d 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -489,11 +489,11 @@ Optimizely.prototype.isFeatureEnabled = function (featureKey, userId, attributes var decisionSource = decision.decisionSource || DECISION_SOURCES.ROLLOUT; this.notificationCenter.sendNotifications( - enums.NOTIFICATION_TYPES.ON_DECISION, + enums.NOTIFICATION_TYPES.DECISION, { type: DECISION_INFO_TYPES.FEATURE, userId: userId, - attributes: attributes, + attributes: attributes || {}, decisionInfo: { featureKey: featureKey, featureEnabled: featureEnabled, diff --git a/packages/optimizely-sdk/lib/optimizely/index.tests.js b/packages/optimizely-sdk/lib/optimizely/index.tests.js index 27837d89c..ff1be454a 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.tests.js +++ b/packages/optimizely-sdk/lib/optimizely/index.tests.js @@ -2066,15 +2066,15 @@ describe('lib/optimizely', function() { }); describe('notification listeners', function() { - var decisionListener; + var activateListener; var trackListener; - var decisionListener2; + var activateListener2; var trackListener2; beforeEach(function() { - decisionListener = sinon.spy(); + activateListener = sinon.spy(); trackListener = sinon.spy(); - decisionListener2 = sinon.spy(); + activateListener2 = sinon.spy(); trackListener2 = sinon.spy(); bucketStub.returns('111129'); sinon.stub(fns, 'currentTimestamp').returns(1509489766569); @@ -2087,11 +2087,11 @@ describe('lib/optimizely', function() { it('should call a listener added for activate when activate is called', function() { optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.ACTIVATE, - decisionListener + activateListener ); var variationKey = optlyInstance.activate('testExperiment', 'testUser'); assert.strictEqual(variationKey, 'variation'); - sinon.assert.calledOnce(decisionListener); + sinon.assert.calledOnce(activateListener); }); it('should call a listener added for track when track is called', function() { @@ -2107,12 +2107,12 @@ describe('lib/optimizely', function() { it('should not call a removed activate listener when activate is called', function() { var listenerId = optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.ACTIVATE, - decisionListener + activateListener ); optlyInstance.notificationCenter.removeNotificationListener(listenerId); var variationKey = optlyInstance.activate('testExperiment', 'testUser'); assert.strictEqual(variationKey, 'variation'); - sinon.assert.notCalled(decisionListener); + sinon.assert.notCalled(activateListener); }); it('should not call a removed track listener when track is called', function() { @@ -2129,7 +2129,7 @@ describe('lib/optimizely', function() { it('removeNotificationListener should only remove the listener with the argument ID', function() { optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.ACTIVATE, - decisionListener + activateListener ); var trackListenerId = optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.TRACK, @@ -2138,13 +2138,13 @@ describe('lib/optimizely', function() { optlyInstance.notificationCenter.removeNotificationListener(trackListenerId); optlyInstance.activate('testExperiment', 'testUser'); optlyInstance.track('testEvent', 'testUser'); - sinon.assert.calledOnce(decisionListener); + sinon.assert.calledOnce(activateListener); }); it('should clear all notification listeners when clearAllNotificationListeners is called', function() { optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.ACTIVATE, - decisionListener + activateListener ); optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.TRACK, @@ -2154,14 +2154,14 @@ describe('lib/optimizely', function() { optlyInstance.activate('testExperiment', 'testUser'); optlyInstance.track('testEvent', 'testUser'); - sinon.assert.notCalled(decisionListener); + sinon.assert.notCalled(activateListener); sinon.assert.notCalled(trackListener); }); it('should clear listeners of certain notification type when clearNotificationListeners is called', function() { optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.ACTIVATE, - decisionListener + activateListener ); optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.TRACK, @@ -2171,47 +2171,47 @@ describe('lib/optimizely', function() { optlyInstance.activate('testExperiment', 'testUser'); optlyInstance.track('testEvent', 'testUser'); - sinon.assert.notCalled(decisionListener); + sinon.assert.notCalled(activateListener); sinon.assert.calledOnce(trackListener); }); it('should only call the listener once after the same listener was added twice', function() { optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.ACTIVATE, - decisionListener + activateListener ); optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.ACTIVATE, - decisionListener + activateListener ); optlyInstance.activate('testExperiment', 'testUser'); - sinon.assert.calledOnce(decisionListener); + sinon.assert.calledOnce(activateListener); }); it('should not add a listener with an invalid type argument', function() { var listenerId = optlyInstance.notificationCenter.addNotificationListener( 'not a notification type', - decisionListener + activateListener ); assert.strictEqual(listenerId, -1); optlyInstance.activate('testExperiment', 'testUser'); - sinon.assert.notCalled(decisionListener); + sinon.assert.notCalled(activateListener); optlyInstance.track('testEvent', 'testUser'); - sinon.assert.notCalled(decisionListener); + sinon.assert.notCalled(activateListener); }); it('should call multiple notification listeners for activate when activate is called', function() { optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.ACTIVATE, - decisionListener + activateListener ); optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.ACTIVATE, - decisionListener2 + activateListener2 ); optlyInstance.activate('testExperiment', 'testUser'); - sinon.assert.calledOnce(decisionListener); - sinon.assert.calledOnce(decisionListener2); + sinon.assert.calledOnce(activateListener); + sinon.assert.calledOnce(activateListener2); }); it('should call multiple notification listeners for track when track is called', function() { @@ -2232,7 +2232,7 @@ describe('lib/optimizely', function() { it('should pass the correct arguments to an activate listener when activate is called', function() { optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.ACTIVATE, - decisionListener + activateListener ); optlyInstance.activate('testExperiment', 'testUser'); var expectedImpressionEvent = { @@ -2281,7 +2281,7 @@ describe('lib/optimizely', function() { variation: instanceExperiments[0].variations[1], logEvent: expectedImpressionEvent, }; - sinon.assert.calledWith(decisionListener, expectedArgument); + sinon.assert.calledWith(activateListener, expectedArgument); }); it('should pass the correct arguments to an activate listener when activate is called with attributes', function() { @@ -2290,7 +2290,7 @@ describe('lib/optimizely', function() { }; optlyInstance.notificationCenter.addNotificationListener( enums.NOTIFICATION_TYPES.ACTIVATE, - decisionListener + activateListener ); optlyInstance.activate('testExperiment', 'testUser', attributes); var expectedImpressionEvent = { @@ -2346,7 +2346,7 @@ describe('lib/optimizely', function() { variation: instanceExperiments[0].variations[1], logEvent: expectedImpressionEvent, }; - sinon.assert.calledWith(decisionListener, expectedArgument); + sinon.assert.calledWith(activateListener, expectedArgument); }); it('should pass the correct arguments to a track listener when track is called', function() { @@ -2522,10 +2522,10 @@ describe('lib/optimizely', function() { sinon.assert.calledWith(trackListener, expectedArgument); }); - describe('OnDecision Listener', function() { - var onDecisionListener; + describe('Decision Listener', function() { + var decisionListener; beforeEach(function() { - onDecisionListener = sinon.spy(); + decisionListener = sinon.spy(); }); describe('feature management', function() { @@ -2544,8 +2544,8 @@ describe('lib/optimizely', function() { }); optlyInstance.notificationCenter.addNotificationListener( - NOTIFICATION_TYPES.ON_DECISION, - onDecisionListener + NOTIFICATION_TYPES.DECISION, + decisionListener ); }); @@ -2571,7 +2571,7 @@ describe('lib/optimizely', function() { it('should return true and send notification', function() { var result = optlyInstance.isFeatureEnabled('test_feature_for_experiment', 'user1', attributes); assert.strictEqual(result, true); - sinon.assert.calledWith(onDecisionListener, { + sinon.assert.calledWith(decisionListener, { type: DECISION_INFO_TYPES.FEATURE, userId: 'user1', attributes: attributes, @@ -2600,7 +2600,7 @@ describe('lib/optimizely', function() { it('should return false and send notification', function() { var result = optlyInstance.isFeatureEnabled('shared_feature', 'user1', attributes); assert.strictEqual(result, false); - sinon.assert.calledWith(onDecisionListener, { + sinon.assert.calledWith(decisionListener, { type: DECISION_INFO_TYPES.FEATURE, userId: 'user1', attributes: attributes, @@ -2634,7 +2634,7 @@ describe('lib/optimizely', function() { test_attribute: 'test_value', }); assert.strictEqual(result, true); - sinon.assert.calledWith(onDecisionListener, { + sinon.assert.calledWith(decisionListener, { type: DECISION_INFO_TYPES.FEATURE, userId: 'user1', attributes: { test_attribute: 'test_value' }, @@ -2680,7 +2680,7 @@ describe('lib/optimizely', function() { sourceVariationKey: null } }; - sinon.assert.calledWith(onDecisionListener, expectedArguments); + sinon.assert.calledWith(decisionListener, expectedArguments); }); }); }); @@ -2697,10 +2697,10 @@ describe('lib/optimizely', function() { it('returns false and send notification', function() { var result = optlyInstance.isFeatureEnabled('test_feature', 'user1'); assert.strictEqual(result, false); - sinon.assert.calledWith(onDecisionListener, { + sinon.assert.calledWith(decisionListener, { type: DECISION_INFO_TYPES.FEATURE, userId: 'user1', - attributes: undefined, + attributes: {}, decisionInfo: { featureKey: 'test_feature', featureEnabled: false, @@ -3268,16 +3268,16 @@ describe('lib/optimizely', function() { isValidInstance: true, }); - var onDecisionListener = sinon.spy(); + var decisionListener = sinon.spy(); var attributes = { test_attribute: 'test_value' }; - optlyInstance.notificationCenter.addNotificationListener(NOTIFICATION_TYPES.ON_DECISION, onDecisionListener); + optlyInstance.notificationCenter.addNotificationListener(NOTIFICATION_TYPES.DECISION, decisionListener); var result = optlyInstance.getEnabledFeatures('test_user', attributes); assert.strictEqual(result.length, 3); assert.isAbove(result.indexOf('test_feature_2'), -1); assert.isAbove(result.indexOf('test_feature_for_experiment'), -1); assert.isAbove(result.indexOf('shared_feature'), -1); - assert.isTrue(onDecisionListener.getCall(0).calledWith({ + assert.isTrue(decisionListener.getCall(0).calledWith({ type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3289,7 +3289,7 @@ describe('lib/optimizely', function() { sourceVariationKey: null } })); - assert.isTrue(onDecisionListener.getCall(1).calledWith({ + assert.isTrue(decisionListener.getCall(1).calledWith({ type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3301,7 +3301,7 @@ describe('lib/optimizely', function() { sourceVariationKey: null } })); - assert.isTrue(onDecisionListener.getCall(2).calledWith({ + assert.isTrue(decisionListener.getCall(2).calledWith({ type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3313,7 +3313,7 @@ describe('lib/optimizely', function() { sourceVariationKey: 'variation' } })); - assert.isTrue(onDecisionListener.getCall(3).calledWith({ + assert.isTrue(decisionListener.getCall(3).calledWith({ type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3325,7 +3325,7 @@ describe('lib/optimizely', function() { sourceVariationKey: null } })); - assert.isTrue(onDecisionListener.getCall(4).calledWith({ + assert.isTrue(decisionListener.getCall(4).calledWith({ type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3337,7 +3337,7 @@ describe('lib/optimizely', function() { sourceVariationKey: 'treatment' } })); - assert.isTrue(onDecisionListener.getCall(5).calledWith({ + assert.isTrue(decisionListener.getCall(5).calledWith({ type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3349,7 +3349,7 @@ describe('lib/optimizely', function() { sourceVariationKey: null } })); - assert.isTrue(onDecisionListener.getCall(6).calledWith({ + assert.isTrue(decisionListener.getCall(6).calledWith({ type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, diff --git a/packages/optimizely-sdk/lib/utils/enums/index.js b/packages/optimizely-sdk/lib/utils/enums/index.js index fb89437b5..d96335ee7 100644 --- a/packages/optimizely-sdk/lib/utils/enums/index.js +++ b/packages/optimizely-sdk/lib/utils/enums/index.js @@ -174,7 +174,7 @@ exports.NODE_CLIENT_VERSION = '3.1.0-beta1'; * - eventTags {Object|undefined} * - logEvent {Object} * - * ON_DECISION: A decision is made in the system. i.e. user activation, + * DECISION: A decision is made in the system. i.e. user activation, * feature access or feature-variable value retrieval * Callbacks will receive an object argument with the following properties: * - type {string} @@ -185,7 +185,7 @@ exports.NODE_CLIENT_VERSION = '3.1.0-beta1'; exports.NOTIFICATION_TYPES = { ACTIVATE: 'ACTIVATE:experiment, user_id,attributes, variation, event', TRACK: 'TRACK:event_key, user_id, attributes, event_tags, event', - ON_DECISION: 'ON_DECISION:type, userId, attributes, decisionInfo', + DECISION: 'DECISION:type, userId, attributes, decisionInfo', }; exports.DECISION_INFO_TYPES = { From c30da191119611bb967d5f4825d4c04b29a4bc89 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Tue, 12 Mar 2019 18:26:29 +0500 Subject: [PATCH 05/10] Explicitly check for featureEnabled is true. --- packages/optimizely-sdk/lib/optimizely/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index c62e1996d..0ea7198b8 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -480,11 +480,11 @@ Optimizely.prototype.isFeatureEnabled = function (featureKey, userId, attributes } } - if (!featureEnabled) { + if (featureEnabled === true) { + this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.FEATURE_ENABLED_FOR_USER, MODULE_NAME, featureKey, userId)); + } else { this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.FEATURE_NOT_ENABLED_FOR_USER, MODULE_NAME, featureKey, userId)); featureEnabled = false; - } else { - this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.FEATURE_ENABLED_FOR_USER, MODULE_NAME, featureKey, userId)); } var decisionSource = decision.decisionSource || DECISION_SOURCES.ROLLOUT; From b139eec030d8ad260ba68f130a68b9f1617fde9b Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Tue, 12 Mar 2019 19:00:36 +0500 Subject: [PATCH 06/10] Remove NOTIFICAITON_TYPES variable. --- packages/optimizely-sdk/lib/optimizely/index.tests.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/optimizely-sdk/lib/optimizely/index.tests.js b/packages/optimizely-sdk/lib/optimizely/index.tests.js index ff1be454a..c99cf0bef 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.tests.js +++ b/packages/optimizely-sdk/lib/optimizely/index.tests.js @@ -40,7 +40,6 @@ var ERROR_MESSAGES = enums.ERROR_MESSAGES; var LOG_LEVEL = enums.LOG_LEVEL; var LOG_MESSAGES = enums.LOG_MESSAGES; var DECISION_SOURCES = enums.DECISION_SOURCES; -var NOTIFICATION_TYPES = enums.NOTIFICATION_TYPES; var DECISION_INFO_TYPES = enums.DECISION_INFO_TYPES; describe('lib/optimizely', function() { @@ -2544,7 +2543,7 @@ describe('lib/optimizely', function() { }); optlyInstance.notificationCenter.addNotificationListener( - NOTIFICATION_TYPES.DECISION, + enums.NOTIFICATION_TYPES.DECISION, decisionListener ); }); @@ -3270,7 +3269,7 @@ describe('lib/optimizely', function() { var decisionListener = sinon.spy(); var attributes = { test_attribute: 'test_value' }; - optlyInstance.notificationCenter.addNotificationListener(NOTIFICATION_TYPES.DECISION, decisionListener); + optlyInstance.notificationCenter.addNotificationListener(enums.NOTIFICATION_TYPES.DECISION, decisionListener); var result = optlyInstance.getEnabledFeatures('test_user', attributes); assert.strictEqual(result.length, 3); assert.isAbove(result.indexOf('test_feature_2'), -1); From 126ce3b7b693a07e78ee13917dc8b1d6a03d9d94 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Wed, 13 Mar 2019 15:40:50 +0500 Subject: [PATCH 07/10] Capitalize decision source values. --- packages/optimizely-sdk/lib/utils/enums/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/optimizely-sdk/lib/utils/enums/index.js b/packages/optimizely-sdk/lib/utils/enums/index.js index d96335ee7..d217d6848 100644 --- a/packages/optimizely-sdk/lib/utils/enums/index.js +++ b/packages/optimizely-sdk/lib/utils/enums/index.js @@ -199,8 +199,8 @@ exports.DECISION_INFO_TYPES = { * Optimizely. */ exports.DECISION_SOURCES = { - EXPERIMENT: 'experiment', - ROLLOUT: 'rollout', + EXPERIMENT: 'EXPERIMENT', + ROLLOUT: 'ROLLOUT', }; /* From 04376c8d7689aa548bed584f0eb7dfce0fb7c78f Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Thu, 14 Mar 2019 17:30:47 +0500 Subject: [PATCH 08/10] getVariationForFeature method now returns decision source as rollout if the user is not evaluated into the experiment. --- .../lib/core/decision_service/index.js | 7 +--- .../lib/core/decision_service/index.tests.js | 22 ++++++------ .../optimizely-sdk/lib/optimizely/index.js | 3 +- .../lib/optimizely/index.tests.js | 36 +++++++++---------- 4 files changed, 30 insertions(+), 38 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index d0daa6f9d..1ef78b311 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.js @@ -303,12 +303,7 @@ DecisionService.prototype.getVariationForFeature = function(feature, userId, att } this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.USER_NOT_IN_ROLLOUT, MODULE_NAME, userId, feature.key)); - - return { - experiment: null, - variation: null, - decisionSource: null, - }; + return rolloutDecision; }; DecisionService.prototype._getVariationForFeatureExperiment = function(feature, userId, attributes) { diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.tests.js b/packages/optimizely-sdk/lib/core/decision_service/index.tests.js index 20693e4a5..a65c47133 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.tests.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.tests.js @@ -843,12 +843,12 @@ describe('lib/core/decision_service', function() { getVariationStub.returns(null); }); - it('returns a decision with no variation', function() { + it('returns a decision with no variation and source rollout', function() { var decision = decisionServiceInstance.getVariationForFeature(feature, 'user1'); var expectedDecision = { experiment: null, variation: null, - decisionSource: null, + decisionSource: DECISION_SOURCES.ROLLOUT, }; assert.deepEqual(decision, expectedDecision); sinon.assert.calledWithExactly(mockLogger.log, LOG_LEVEL.DEBUG, 'DECISION_SERVICE: User user1 is not in any experiment on the feature test_feature_for_experiment.'); @@ -915,12 +915,12 @@ describe('lib/core/decision_service', function() { getVariationStub.returns(null); }); - it('returns a decision with no experiment and no variation', function() { + it('returns a decision with no experiment, no variation and source rollout', function() { var decision = decisionServiceInstance.getVariationForFeature(feature, 'user1'); var expectedDecision = { experiment: null, variation: null, - decisionSource: null, + decisionSource: DECISION_SOURCES.ROLLOUT, }; assert.deepEqual(decision, expectedDecision); sinon.assert.calledWithExactly(mockLogger.log, LOG_LEVEL.DEBUG, 'DECISION_SERVICE: User user1 is not in any experiment on the feature feature_with_group.'); @@ -932,7 +932,7 @@ describe('lib/core/decision_service', function() { var expectedDecision = { experiment: null, variation: null, - decisionSource: null, + decisionSource: DECISION_SOURCES.ROLLOUT, }; assert.deepEqual(decision, expectedDecision); sinon.assert.calledWithExactly(mockLogger.log, LOG_LEVEL.DEBUG, 'DECISION_SERVICE: User user1 is not in any experiment on the feature feature_exp_no_traffic.'); @@ -946,12 +946,12 @@ describe('lib/core/decision_service', function() { bucketUserIntoExperimentStub.returns(null); }); - it('returns a decision with no experiment and no variation', function() { + it('returns a decision with no experiment, no variation and source rollout', function() { var decision = decisionServiceInstance.getVariationForFeature(feature, 'user1'); var expectedDecision = { experiment: null, variation: null, - decisionSource: null, + decisionSource: DECISION_SOURCES.ROLLOUT, }; assert.deepEqual(decision, expectedDecision); sinon.assert.calledWithExactly(mockLogger.log, LOG_LEVEL.DEBUG, 'DECISION_SERVICE: User user1 is not in any experiment on the feature feature_with_group.'); @@ -1168,12 +1168,12 @@ describe('lib/core/decision_service', function() { bucketStub.returns(null); }); - it('returns a decision with no variation and no experiment', function() { + it('returns a decision with no variation, no experiment and source rollout', function() { var decision = decisionServiceInstance.getVariationForFeature(feature, 'user1'); var expectedDecision = { experiment: null, variation: null, - decisionSource: null, + decisionSource: DECISION_SOURCES.ROLLOUT, }; assert.deepEqual(decision, expectedDecision); sinon.assert.calledWithExactly(mockLogger.log, LOG_LEVEL.DEBUG, 'DECISION_SERVICE: User user1 does not meet conditions for targeting rule 1.'); @@ -1378,12 +1378,12 @@ describe('lib/core/decision_service', function() { feature = configObj.featureKeyMap.unused_flag; }); - it('returns a decision with no variation and no experiment', function() { + it('returns a decision with no variation, no experiment and source rollout', function() { var decision = decisionServiceInstance.getVariationForFeature(feature, 'user1'); var expectedDecision = { experiment: null, variation: null, - decisionSource: null, + decisionSource: DECISION_SOURCES.ROLLOUT, }; var expectedDecision = assert.deepEqual(decision, expectedDecision); sinon.assert.calledWithExactly(mockLogger.log, LOG_LEVEL.DEBUG, 'DECISION_SERVICE: Feature unused_flag is not attached to any experiments.'); diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index c436b8f17..49bddb896 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -487,7 +487,6 @@ Optimizely.prototype.isFeatureEnabled = function (featureKey, userId, attributes featureEnabled = false; } - var decisionSource = decision.decisionSource || DECISION_SOURCES.ROLLOUT; this.notificationCenter.sendNotifications( enums.NOTIFICATION_TYPES.DECISION, { @@ -497,7 +496,7 @@ Optimizely.prototype.isFeatureEnabled = function (featureKey, userId, attributes decisionInfo: { featureKey: featureKey, featureEnabled: featureEnabled, - source: decisionSource, + source: decision.decisionSource, sourceExperimentKey: experimentKey, sourceVariationKey: variationKey, } diff --git a/packages/optimizely-sdk/lib/optimizely/index.tests.js b/packages/optimizely-sdk/lib/optimizely/index.tests.js index 66c5f7acc..7a33f4df3 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.tests.js +++ b/packages/optimizely-sdk/lib/optimizely/index.tests.js @@ -2689,7 +2689,7 @@ describe('lib/optimizely', function() { sandbox.stub(optlyInstance.decisionService, 'getVariationForFeature').returns({ experiment: null, variation: null, - decisionSource: null, + decisionSource: DECISION_SOURCES.ROLLOUT, }); }); @@ -3166,7 +3166,7 @@ describe('lib/optimizely', function() { sandbox.stub(optlyInstance.decisionService, 'getVariationForFeature').returns({ experiment: null, variation: null, - decisionSource: null, + decisionSource: DECISION_SOURCES.ROLLOUT, }); }); @@ -3272,11 +3272,9 @@ describe('lib/optimizely', function() { optlyInstance.notificationCenter.addNotificationListener(enums.NOTIFICATION_TYPES.DECISION, decisionListener); var result = optlyInstance.getEnabledFeatures('test_user', attributes); assert.strictEqual(result.length, 3); - assert.isAbove(result.indexOf('test_feature_2'), -1); - assert.isAbove(result.indexOf('test_feature_for_experiment'), -1); - assert.isAbove(result.indexOf('shared_feature'), -1); + assert.deepEqual(result, ['test_feature_2', 'test_feature_for_experiment', 'shared_feature']); - assert.isTrue(decisionListener.getCall(0).calledWith({ + sinon.assert.calledWithExactly(decisionListener.getCall(0), { type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3287,8 +3285,8 @@ describe('lib/optimizely', function() { sourceExperimentKey: null, sourceVariationKey: null } - })); - assert.isTrue(decisionListener.getCall(1).calledWith({ + }); + sinon.assert.calledWithExactly(decisionListener.getCall(1), { type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3299,8 +3297,8 @@ describe('lib/optimizely', function() { sourceExperimentKey: null, sourceVariationKey: null } - })); - assert.isTrue(decisionListener.getCall(2).calledWith({ + }); + sinon.assert.calledWithExactly(decisionListener.getCall(2), { type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3311,8 +3309,8 @@ describe('lib/optimizely', function() { sourceExperimentKey: 'testing_my_feature', sourceVariationKey: 'variation' } - })); - assert.isTrue(decisionListener.getCall(3).calledWith({ + }); + sinon.assert.calledWithExactly(decisionListener.getCall(3), { type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3323,8 +3321,8 @@ describe('lib/optimizely', function() { sourceExperimentKey: null, sourceVariationKey: null } - })); - assert.isTrue(decisionListener.getCall(4).calledWith({ + }); + sinon.assert.calledWithExactly(decisionListener.getCall(4), { type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3335,8 +3333,8 @@ describe('lib/optimizely', function() { sourceExperimentKey: 'test_shared_feature', sourceVariationKey: 'treatment' } - })); - assert.isTrue(decisionListener.getCall(5).calledWith({ + }); + sinon.assert.calledWithExactly(decisionListener.getCall(5), { type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3347,8 +3345,8 @@ describe('lib/optimizely', function() { sourceExperimentKey: null, sourceVariationKey: null } - })); - assert.isTrue(decisionListener.getCall(6).calledWith({ + }); + sinon.assert.calledWithExactly(decisionListener.getCall(6), { type: DECISION_INFO_TYPES.FEATURE, userId: 'test_user', attributes: attributes, @@ -3359,7 +3357,7 @@ describe('lib/optimizely', function() { sourceExperimentKey: null, sourceVariationKey: null } - })); + }); }); }); From 9c3ec8a1a3c5072aa0583c829b0f0729c2613196 Mon Sep 17 00:00:00 2001 From: Sohail Hussain Date: Fri, 22 Mar 2019 11:44:31 -0700 Subject: [PATCH 09/10] Fixed case issue --- packages/optimizely-sdk/lib/utils/enums/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/optimizely-sdk/lib/utils/enums/index.js b/packages/optimizely-sdk/lib/utils/enums/index.js index 726df335a..3f47ad8ed 100644 --- a/packages/optimizely-sdk/lib/utils/enums/index.js +++ b/packages/optimizely-sdk/lib/utils/enums/index.js @@ -200,8 +200,8 @@ exports.DECISION_INFO_TYPES = { * Optimizely. */ exports.DECISION_SOURCES = { - EXPERIMENT: 'EXPERIMENT', - ROLLOUT: 'ROLLOUT', + EXPERIMENT: 'experiment', + ROLLOUT: 'rollout', }; /* From 6365632d49d7853dd2d581bf77924f51896284a4 Mon Sep 17 00:00:00 2001 From: Sohail Hussain Date: Fri, 22 Mar 2019 16:20:44 -0700 Subject: [PATCH 10/10] Revert "Fixed case issue" This reverts commit 9c3ec8a1a3c5072aa0583c829b0f0729c2613196. --- packages/optimizely-sdk/lib/utils/enums/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/optimizely-sdk/lib/utils/enums/index.js b/packages/optimizely-sdk/lib/utils/enums/index.js index 3f47ad8ed..726df335a 100644 --- a/packages/optimizely-sdk/lib/utils/enums/index.js +++ b/packages/optimizely-sdk/lib/utils/enums/index.js @@ -200,8 +200,8 @@ exports.DECISION_INFO_TYPES = { * Optimizely. */ exports.DECISION_SOURCES = { - EXPERIMENT: 'experiment', - ROLLOUT: 'rollout', + EXPERIMENT: 'EXPERIMENT', + ROLLOUT: 'ROLLOUT', }; /*