From cd8bd9097173018b123fc46b7fcad16b70f52f1f Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 30 Apr 2020 17:28:57 -0700 Subject: [PATCH 1/8] implemented the api. decision notification listeners pending. --- .../optimizely-sdk/lib/optimizely/index.js | 90 ++++++ .../lib/optimizely/index.tests.js | 291 ++++++++++++++++++ 2 files changed, 381 insertions(+) diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index e2fadba9d..29d9e8354 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -957,6 +957,96 @@ Optimizely.prototype.getFeatureVariableJson = function(featureKey, variableKey, } }; +Optimizely.prototype.getAllFeatureVariables = function(featureKey, userId, attributes) { + try { + if (!this.__isValidInstance()) { + this.logger.log(LOG_LEVEL.ERROR, sprintf(LOG_MESSAGES.INVALID_OBJECT, MODULE_NAME, 'getAllFeatureVariables')); + return null; + } + + if (!this.__validateInputs({ feature_key: featureKey, user_id: userId }, attributes)) { + return null; + } + + var configObj = this.projectConfigManager.getConfig(); + if (!configObj) { + return null; + } + + var featureFlag = projectConfig.getFeatureFromKey(configObj, featureKey, this.logger); + if (!featureFlag) { + return null; + } + + var decision = this.decisionService.getVariationForFeature(configObj, featureFlag, userId, attributes); + var featureEnabled = decision.variation !== null ? decision.variation.featureEnabled : false; + var logger = this.logger; + var allVariables = {}; + + featureFlag.variables.forEach(function (variable) { + var variableValue = variable.defaultValue; + if (decision.variation !== null) { + var value = projectConfig.getVariableValueForVariation(configObj, variable, decision.variation, logger); + if (value !== null) { + if (featureEnabled) { + variableValue = value; + logger.log( + LOG_LEVEL.INFO, + sprintf( + LOG_MESSAGES.USER_RECEIVED_VARIABLE_VALUE, + MODULE_NAME, + variable.key, + featureFlag.key, + variableValue, + userId + ) + ); + } else { + logger.log( + LOG_LEVEL.INFO, + sprintf( + LOG_MESSAGES.FEATURE_NOT_ENABLED_RETURN_DEFAULT_VARIABLE_VALUE, + MODULE_NAME, + featureFlag.key, + userId, + variable.key + ) + ); + } + } else { + logger.log( + LOG_LEVEL.INFO, + sprintf( + LOG_MESSAGES.VARIABLE_NOT_USED_RETURN_DEFAULT_VARIABLE_VALUE, + MODULE_NAME, + variable.key, + decision.variation.key + ) + ); + } + } else { + logger.log( + LOG_LEVEL.INFO, + sprintf( + LOG_MESSAGES.USER_RECEIVED_DEFAULT_VARIABLE_VALUE, + MODULE_NAME, + userId, + variable.key, + featureFlag.key + ) + ); + } + var typeCastedValue = projectConfig.getTypeCastValue(variableValue, variable.type, logger); + allVariables[variable.key] = typeCastedValue; + }); + return allVariables; + } catch (e) { + this.logger.log(LOG_LEVEL.ERROR, e.message); + this.errorHandler.handleError(e); + return null; + } +}; + /** * Returns OptimizelyConfig object containing experiments and features data * @return {Object} diff --git a/packages/optimizely-sdk/lib/optimizely/index.tests.js b/packages/optimizely-sdk/lib/optimizely/index.tests.js index 50baafa94..84dbb672c 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.tests.js +++ b/packages/optimizely-sdk/lib/optimizely/index.tests.js @@ -4885,6 +4885,47 @@ describe('lib/optimizely', function() { ); }); + it('returns the right values from getAllFeatureVariables', function() { + var result = optlyInstance.getAllFeatureVariables('test_feature_for_experiment', 'user1', { + test_attribute: 'test_value', + }); + assert.deepEqual(result, { + is_button_animated: true, + button_width: 20.25, + num_buttons: 2, + button_txt: 'Buy me NOW', + button_info: { + num_buttons: 1, + text: 'first variation', + }, + }); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Value for variable "is_button_animated" of feature flag "test_feature_for_experiment" is true for user "user1"' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Value for variable "button_width" of feature flag "test_feature_for_experiment" is 20.25 for user "user1"' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Value for variable "num_buttons" of feature flag "test_feature_for_experiment" is 2 for user "user1"' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Value for variable "button_txt" of feature flag "test_feature_for_experiment" is Buy me NOW for user "user1"' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Value for variable "button_info" of feature flag "test_feature_for_experiment" is { "num_buttons": 1, "text": "first variation"} for user "user1"' + ); + }); + describe('when the variable is not used in the variation', function() { beforeEach(function() { sandbox.stub(projectConfig, 'getVariableValueForVariation').returns(null); @@ -5033,6 +5074,47 @@ describe('lib/optimizely', function() { 'OPTIMIZELY: Variable "button_info" is not used in variation "variation". Returning default value.' ); }); + + it('returns the right values from getAllFeatureVariables', function() { + var result = optlyInstance.getAllFeatureVariables('test_feature_for_experiment', 'user1', { + test_attribute: 'test_value', + }); + assert.deepEqual(result, { + is_button_animated: false, + button_width: 50.55, + num_buttons: 10, + button_txt: 'Buy me', + button_info: { + num_buttons: 0, + text: "default value", + }, + }); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Variable "is_button_animated" is not used in variation "variation". Returning default value.' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Variable "button_width" is not used in variation "variation". Returning default value.' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Variable "num_buttons" is not used in variation "variation". Returning default value.' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Variable "button_txt" is not used in variation "variation". Returning default value.' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Variable "button_info" is not used in variation "variation". Returning default value.' + ); + }); }); }); @@ -5187,6 +5269,47 @@ describe('lib/optimizely', function() { 'OPTIMIZELY: Feature "test_feature_for_experiment" is not enabled for user user1. Returning default value for variable "button_info".' ); }); + + it('returns the right values from getAllFeatureVariables', function() { + var result = optlyInstance.getAllFeatureVariables('test_feature_for_experiment', 'user1', { + test_attribute: 'test_value', + }); + assert.deepEqual(result, { + is_button_animated: false, + button_width: 50.55, + num_buttons: 10, + button_txt: 'Buy me', + button_info: { + num_buttons: 0, + text: "default value", + }, + }); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Feature "test_feature_for_experiment" is not enabled for user user1. Returning default value for variable "is_button_animated".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Feature "test_feature_for_experiment" is not enabled for user user1. Returning default value for variable "button_width".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Feature "test_feature_for_experiment" is not enabled for user user1. Returning default value for variable "num_buttons".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Feature "test_feature_for_experiment" is not enabled for user user1. Returning default value for variable "button_txt".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Feature "test_feature_for_experiment" is not enabled for user user1. Returning default value for variable "button_info".' + ); + }); }); }); @@ -5331,6 +5454,47 @@ describe('lib/optimizely', function() { ); }); + it('returns the right values from getAllFeatureVariables', function() { + var result = optlyInstance.getAllFeatureVariables('test_feature', 'user1', { + test_attribute: 'test_value', + }); + assert.deepEqual(result, { + new_content: true, + price: 4.99, + lasers: 395, + message: 'Hello audience', + message_info: { + count: 2, + message: 'Hello audience', + }, + }); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Value for variable "new_content" of feature flag "test_feature" is true for user "user1"' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Value for variable "price" of feature flag "test_feature" is 4.99 for user "user1"' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Value for variable "lasers" of feature flag "test_feature" is 395 for user "user1"' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Value for variable "message" of feature flag "test_feature" is Hello audience for user "user1"' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Value for variable "message_info" of feature flag "test_feature" is { "count": 2, "message": "Hello audience" } for user "user1"' + ); + }); + describe('when the variable is not used in the variation', function() { beforeEach(function() { sandbox.stub(projectConfig, 'getVariableValueForVariation').returns(null); @@ -5461,6 +5625,47 @@ describe('lib/optimizely', function() { 'OPTIMIZELY: Variable "message_info" is not used in variation "594032". Returning default value.' ); }); + + it('returns the right values from getAllFeatureVariables', function() { + var result = optlyInstance.getAllFeatureVariables('test_feature', 'user1', { + test_attribute: 'test_value', + }); + assert.deepEqual(result, { + new_content: false, + price: 14.99, + lasers: 400, + message: 'Hello', + message_info: { + count: 1, + message: 'Hello', + }, + }); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Variable "new_content" is not used in variation "594032". Returning default value.' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Variable "price" is not used in variation "594032". Returning default value.' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Variable "lasers" is not used in variation "594032". Returning default value.' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Variable "message" is not used in variation "594032". Returning default value.' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Variable "message_info" is not used in variation "594032". Returning default value.' + ); + }); }); }); @@ -5603,6 +5808,47 @@ describe('lib/optimizely', function() { 'OPTIMIZELY: Feature "test_feature" is not enabled for user user1. Returning default value for variable "message_info".' ); }); + + it('returns the right values from getAllFeatureVariables', function() { + var result = optlyInstance.getAllFeatureVariables('test_feature', 'user1', { + test_attribute: 'test_value', + }); + assert.deepEqual(result, { + new_content: false, + price: 14.99, + lasers: 400, + message: 'Hello', + message_info: { + count: 1, + message: 'Hello', + }, + }); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Feature "test_feature" is not enabled for user user1. Returning default value for variable "new_content".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Feature "test_feature" is not enabled for user user1. Returning default value for variable "price".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Feature "test_feature" is not enabled for user user1. Returning default value for variable "lasers".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Feature "test_feature" is not enabled for user user1. Returning default value for variable "message".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: Feature "test_feature" is not enabled for user user1. Returning default value for variable "message_info".' + ); + }); }); }); @@ -5744,6 +5990,51 @@ describe('lib/optimizely', function() { ); }); + it('returns the right values from getAllFeatureVariables', function() { + var result = optlyInstance.getAllFeatureVariables('test_feature_for_experiment', 'user1', { + test_attribute: 'test_value', + }); + assert.deepEqual(result, { + is_button_animated: false, + button_width: 50.55, + num_buttons: 10, + button_txt: 'Buy me', + button_info: { + num_buttons: 0, + text: 'default value', + }, + }); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: User "user1" is not in any variation or rollout rule. Returning default value for variable "is_button_animated" of feature flag "test_feature_for_experiment".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: User "user1" is not in any variation or rollout rule. Returning default value for variable "button_width" of feature flag "test_feature_for_experiment".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: User "user1" is not in any variation or rollout rule. Returning default value for variable "num_buttons" of feature flag "test_feature_for_experiment".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: User "user1" is not in any variation or rollout rule. Returning default value for variable "num_buttons" of feature flag "test_feature_for_experiment".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: User "user1" is not in any variation or rollout rule. Returning default value for variable "button_txt" of feature flag "test_feature_for_experiment".' + ); + sinon.assert.calledWith( + createdLogger.log, + LOG_LEVEL.INFO, + 'OPTIMIZELY: User "user1" is not in any variation or rollout rule. Returning default value for variable "button_info" of feature flag "test_feature_for_experiment".' + ); + }); }); it('returns null from getFeatureVariable if user id is null when variable type is boolean', function() { From be1eb74787aedcd3cee8e5c0618ae631210277df Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 30 Apr 2020 19:35:03 -0700 Subject: [PATCH 2/8] added notifications and its tests --- .../optimizely-sdk/lib/optimizely/index.js | 31 +++ .../lib/optimizely/index.tests.js | 208 +++++++++++++++++- .../optimizely-sdk/lib/utils/enums/index.js | 1 + 3 files changed, 234 insertions(+), 6 deletions(-) diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index 29d9e8354..bf0d98fbe 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -957,6 +957,16 @@ Optimizely.prototype.getFeatureVariableJson = function(featureKey, variableKey, } }; +/** + * Returns values for all the json variables attached to the given feature + * flag. + * @param {string} featureKey Key of the feature whose variables are being + * accessed + * @param {string} userId ID for the user + * @param {Object} attributes Optional user attributes + * @return {object|null} Object cointaining all the variables, or null if the + * feature key is invalid + */ Optimizely.prototype.getAllFeatureVariables = function(featureKey, userId, attributes) { try { if (!this.__isValidInstance()) { @@ -1039,6 +1049,27 @@ Optimizely.prototype.getAllFeatureVariables = function(featureKey, userId, attri var typeCastedValue = projectConfig.getTypeCastValue(variableValue, variable.type, logger); allVariables[variable.key] = typeCastedValue; }); + + var sourceInfo = {}; + if (decision.decisionSource === DECISION_SOURCES.FEATURE_TEST) { + sourceInfo = { + experimentKey: decision.experiment.key, + variationKey: decision.variation.key, + }; + } + this.notificationCenter.sendNotifications(NOTIFICATION_TYPES.DECISION, { + type: DECISION_NOTIFICATION_TYPES.ALL_FEATURE_VARIABLES, + userId: userId, + attributes: attributes || {}, + decisionInfo: { + featureKey: featureKey, + featureEnabled: featureEnabled, + source: decision.decisionSource, + variableValues: allVariables, + sourceInfo: sourceInfo, + }, + }); + return allVariables; } catch (e) { this.logger.log(LOG_LEVEL.ERROR, e.message); diff --git a/packages/optimizely-sdk/lib/optimizely/index.tests.js b/packages/optimizely-sdk/lib/optimizely/index.tests.js index 84dbb672c..e01a3895a 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.tests.js +++ b/packages/optimizely-sdk/lib/optimizely/index.tests.js @@ -3170,7 +3170,49 @@ describe('lib/optimizely', function() { }, }); }); - }); + + it('returns the right value from getAllFeatureVariables and send notification with featureEnabled true', function() { + var result = optlyInstance.getAllFeatureVariables( + 'test_feature_for_experiment', + 'user1', + { test_attribute: 'test_value' } + ); + assert.deepEqual(result, { + is_button_animated: true, + button_width: 20.25, + num_buttons: 2, + button_txt: 'Buy me NOW', + button_info: { + num_buttons: 1, + text: 'first variation', + }, + }); + sinon.assert.calledWith(decisionListener, { + type: DECISION_NOTIFICATION_TYPES.ALL_FEATURE_VARIABLES, + userId: 'user1', + attributes: { test_attribute: 'test_value' }, + decisionInfo: { + featureKey: 'test_feature_for_experiment', + featureEnabled: true, + variableValues: { + is_button_animated: true, + button_width: 20.25, + num_buttons: 2, + button_txt: 'Buy me NOW', + button_info: { + num_buttons: 1, + text: 'first variation', + }, + }, + source: DECISION_SOURCES.FEATURE_TEST, + sourceInfo: { + experimentKey: 'testing_my_feature', + variationKey: 'variation', + }, + }, + }); + }); + }); describe('when the variation is toggled OFF', function() { beforeEach(function() { @@ -3326,6 +3368,48 @@ describe('lib/optimizely', function() { }, }); }); + + it('returns the right value from getAllFeatureVariables and send notification with featureEnabled false', function() { + var result = optlyInstance.getAllFeatureVariables( + 'test_feature_for_experiment', + 'user1', + { test_attribute: 'test_value' } + ); + assert.deepEqual(result, { + is_button_animated: false, + button_width: 50.55, + num_buttons: 10, + button_txt: 'Buy me', + button_info: { + num_buttons: 0, + text: 'default value', + }, + }); + sinon.assert.calledWith(decisionListener, { + type: DECISION_NOTIFICATION_TYPES.ALL_FEATURE_VARIABLES, + userId: 'user1', + attributes: { test_attribute: 'test_value' }, + decisionInfo: { + featureKey: 'test_feature_for_experiment', + featureEnabled: false, + variableValues: { + is_button_animated: false, + button_width: 50.55, + num_buttons: 10, + button_txt: 'Buy me', + button_info: { + num_buttons: 0, + text: 'default value', + }, + }, + source: DECISION_SOURCES.FEATURE_TEST, + sourceInfo: { + experimentKey: 'testing_my_feature', + variationKey: 'variation2', + }, + }, + }); + }); }); }); @@ -3565,6 +3649,45 @@ describe('lib/optimizely', function() { }, }); }); + + it('returns the right value from getAllFeatureVariables and send notification with featureEnabled true', function() { + var result = optlyInstance.getAllFeatureVariables( + 'test_feature', + 'user1', + { test_attribute: 'test_value' } + ); + assert.deepEqual(result, { + new_content: true, + price: 4.99, + lasers: 395, + message: 'Hello audience', + message_info: { + count: 2, + message: 'Hello audience', + }, + }); + sinon.assert.calledWith(decisionListener, { + type: DECISION_NOTIFICATION_TYPES.ALL_FEATURE_VARIABLES, + userId: 'user1', + attributes: { test_attribute: 'test_value' }, + decisionInfo: { + featureKey: 'test_feature', + featureEnabled: true, + variableValues: { + new_content: true, + price: 4.99, + lasers: 395, + message: 'Hello audience', + message_info: { + count: 2, + message: 'Hello audience', + }, + }, + source: DECISION_SOURCES.ROLLOUT, + sourceInfo: {}, + }, + }); + }); }); describe('when the variation is toggled OFF', function() { @@ -3799,6 +3922,45 @@ describe('lib/optimizely', function() { }, }); }); + + it('returns the default value from getAllFeatureVariables and send notification with featureEnabled false', function() { + var result = optlyInstance.getAllFeatureVariables( + 'test_feature', + 'user1', + { test_attribute: 'test_value' } + ); + assert.deepEqual(result, { + new_content: false, + price: 14.99, + lasers: 400, + message: 'Hello', + message_info: { + count: 1, + message: 'Hello', + }, + }); + sinon.assert.calledWith(decisionListener, { + type: DECISION_NOTIFICATION_TYPES.ALL_FEATURE_VARIABLES, + userId: 'user1', + attributes: { test_attribute: 'test_value' }, + decisionInfo: { + featureKey: 'test_feature', + featureEnabled: false, + variableValues: { + new_content: false, + price: 14.99, + lasers: 400, + message: 'Hello', + message_info: { + count: 1, + message: 'Hello', + }, + }, + source: DECISION_SOURCES.ROLLOUT, + sourceInfo: {}, + }, + }); + }); }); }); @@ -4050,6 +4212,45 @@ describe('lib/optimizely', function() { }, }); }); + + it('returns the default value from getAllFeatureVariables and send notification with featureEnabled false', function() { + var result = optlyInstance.getAllFeatureVariables( + 'test_feature_for_experiment', + 'user1', + { test_attribute: 'test_value' } + ); + assert.deepEqual(result, { + is_button_animated: false, + button_width: 50.55, + num_buttons: 10, + button_txt: 'Buy me', + button_info: { + num_buttons: 0, + text: 'default value', + } + }); + sinon.assert.calledWith(decisionListener, { + type: DECISION_NOTIFICATION_TYPES.ALL_FEATURE_VARIABLES, + userId: 'user1', + attributes: { test_attribute: 'test_value' }, + decisionInfo: { + featureKey: 'test_feature_for_experiment', + featureEnabled: false, + variableValues: { + is_button_animated: false, + button_width: 50.55, + num_buttons: 10, + button_txt: 'Buy me', + button_info: { + num_buttons: 0, + text: 'default value', + } + }, + source: DECISION_SOURCES.ROLLOUT, + sourceInfo: {}, + }, + }); + }); }); }); }); @@ -6019,11 +6220,6 @@ describe('lib/optimizely', function() { LOG_LEVEL.INFO, 'OPTIMIZELY: User "user1" is not in any variation or rollout rule. Returning default value for variable "num_buttons" of feature flag "test_feature_for_experiment".' ); - sinon.assert.calledWith( - createdLogger.log, - LOG_LEVEL.INFO, - 'OPTIMIZELY: User "user1" is not in any variation or rollout rule. Returning default value for variable "num_buttons" of feature flag "test_feature_for_experiment".' - ); sinon.assert.calledWith( createdLogger.log, LOG_LEVEL.INFO, diff --git a/packages/optimizely-sdk/lib/utils/enums/index.js b/packages/optimizely-sdk/lib/utils/enums/index.js index a664b7687..660308eb3 100644 --- a/packages/optimizely-sdk/lib/utils/enums/index.js +++ b/packages/optimizely-sdk/lib/utils/enums/index.js @@ -188,6 +188,7 @@ export var DECISION_NOTIFICATION_TYPES = { FEATURE: 'feature', FEATURE_TEST: 'feature-test', FEATURE_VARIABLE: 'feature-variable', + ALL_FEATURE_VARIABLES: 'all-feature-variables', }; /* From a6165c673affae619ad9b8260189010ac241b690 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Thu, 30 Apr 2020 19:37:57 -0700 Subject: [PATCH 3/8] fixed a spelling mistake --- packages/optimizely-sdk/lib/optimizely/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index bf0d98fbe..3a754584b 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -964,7 +964,7 @@ Optimizely.prototype.getFeatureVariableJson = function(featureKey, variableKey, * accessed * @param {string} userId ID for the user * @param {Object} attributes Optional user attributes - * @return {object|null} Object cointaining all the variables, or null if the + * @return {object|null} Object containing all the variables, or null if the * feature key is invalid */ Optimizely.prototype.getAllFeatureVariables = function(featureKey, userId, attributes) { From 018bccac889f8e295e245c2aea5a343bfa936b25 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Fri, 1 May 2020 17:49:46 -0700 Subject: [PATCH 4/8] refactored to remove redundant code and simplified logic --- .../optimizely-sdk/lib/optimizely/index.js | 211 ++++++++---------- 1 file changed, 88 insertions(+), 123 deletions(-) diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index 3a754584b..14d8f966c 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -744,72 +744,17 @@ Optimizely.prototype._getFeatureVariableForType = function(featureKey, variableK return null; } - if (!variableType) { - variableType = variable.type; - } else if (variable.type !== variableType) { + if (variableType && variable.type !== variableType) { this.logger.log( LOG_LEVEL.WARNING, sprintf(LOG_MESSAGES.VARIABLE_REQUESTED_WITH_WRONG_TYPE, MODULE_NAME, variableType, variable.type) ); return null; } - - var featureEnabled = false; - var variableValue = variable.defaultValue; - var decision = this.decisionService.getVariationForFeature(configObj, featureFlag, userId, attributes); - - if (decision.variation !== null) { - featureEnabled = decision.variation.featureEnabled; - var value = projectConfig.getVariableValueForVariation(configObj, variable, decision.variation, this.logger); - if (value !== null) { - if (featureEnabled === true) { - variableValue = value; - this.logger.log( - LOG_LEVEL.INFO, - sprintf( - LOG_MESSAGES.USER_RECEIVED_VARIABLE_VALUE, - MODULE_NAME, - variableKey, - featureFlag.key, - variableValue, - userId - ) - ); - } else { - this.logger.log( - LOG_LEVEL.INFO, - sprintf( - LOG_MESSAGES.FEATURE_NOT_ENABLED_RETURN_DEFAULT_VARIABLE_VALUE, - MODULE_NAME, - featureFlag.key, - userId, - variableKey - ) - ); - } - } else { - this.logger.log( - LOG_LEVEL.INFO, - sprintf( - LOG_MESSAGES.VARIABLE_NOT_USED_RETURN_DEFAULT_VARIABLE_VALUE, - MODULE_NAME, - variableKey, - decision.variation.key - ) - ); - } - } else { - this.logger.log( - LOG_LEVEL.INFO, - sprintf( - LOG_MESSAGES.USER_RECEIVED_DEFAULT_VARIABLE_VALUE, - MODULE_NAME, - userId, - variableKey, - featureFlag.key - ) - ); - } + + var decision = this.decisionService.getVariationForFeature(configObj, featureFlag, userId, attributes); + var featureEnabled = decision.variation !== null ? decision.variation.featureEnabled : false; + var variableValue = this._getFeatureVariableValueFromVariation(featureKey, featureEnabled, decision.variation, variable, userId); var sourceInfo = {}; if (decision.decisionSource === DECISION_SOURCES.FEATURE_TEST) { @@ -818,8 +763,7 @@ Optimizely.prototype._getFeatureVariableForType = function(featureKey, variableK variationKey: decision.variation.key, }; } - - var typeCastedValue = projectConfig.getTypeCastValue(variableValue, variableType, this.logger); + this.notificationCenter.sendNotifications(NOTIFICATION_TYPES.DECISION, { type: DECISION_NOTIFICATION_TYPES.FEATURE_VARIABLE, userId: userId, @@ -829,14 +773,89 @@ Optimizely.prototype._getFeatureVariableForType = function(featureKey, variableK featureEnabled: featureEnabled, source: decision.decisionSource, variableKey: variableKey, - variableValue: typeCastedValue, - variableType: variableType, + variableValue: variableValue, + variableType: variable.type, sourceInfo: sourceInfo, }, }); - return typeCastedValue; + return variableValue; }; +Optimizely.prototype._getFeatureVariableValueFromVariation = function(featureKey, featureEnabled, variation, variable, userId) { + var configObj = this.projectConfigManager.getConfig(); + if (!configObj) { + return null; + } + + var variableValue = variable.defaultValue; + var valueFromVariation = null; + + if (variation) { + valueFromVariation = projectConfig.getVariableValueForVariation(configObj, variable, variation, this.logger); + if (valueFromVariation && featureEnabled) { + variableValue = valueFromVariation; + } + } + + // User is not in any variation + if (!variation) { + this.logger.log( + LOG_LEVEL.INFO, + sprintf( + LOG_MESSAGES.USER_RECEIVED_DEFAULT_VARIABLE_VALUE, + MODULE_NAME, + userId, + variable.key, + featureKey + ) + ); + } + + // When variable value is in variation and feature is enabled + if (variation && valueFromVariation && featureEnabled) { + this.logger.log( + LOG_LEVEL.INFO, + sprintf( + LOG_MESSAGES.USER_RECEIVED_VARIABLE_VALUE, + MODULE_NAME, + variable.key, + featureKey, + variableValue, + userId + ) + ); + } + + // When variable value is in variation but feature is not enabled + if (variation && valueFromVariation && !featureEnabled) { + this.logger.log( + LOG_LEVEL.INFO, + sprintf( + LOG_MESSAGES.FEATURE_NOT_ENABLED_RETURN_DEFAULT_VARIABLE_VALUE, + MODULE_NAME, + featureKey, + userId, + variable.key + ) + ); + } + + // When varible is not used in variation + if (variation && !valueFromVariation) { + this.logger.log( + LOG_LEVEL.INFO, + sprintf( + LOG_MESSAGES.VARIABLE_NOT_USED_RETURN_DEFAULT_VARIABLE_VALUE, + MODULE_NAME, + variable.key, + variation.key + ) + ); + } + + return projectConfig.getTypeCastValue(variableValue, variable.type, this.logger); +} + /** * Returns value for the given boolean variable attached to the given feature * flag. @@ -958,7 +977,7 @@ Optimizely.prototype.getFeatureVariableJson = function(featureKey, variableKey, }; /** - * Returns values for all the json variables attached to the given feature + * Returns values for all the variables attached to the given feature * flag. * @param {string} featureKey Key of the feature whose variables are being * accessed @@ -989,66 +1008,12 @@ Optimizely.prototype.getAllFeatureVariables = function(featureKey, userId, attri } var decision = this.decisionService.getVariationForFeature(configObj, featureFlag, userId, attributes); - var featureEnabled = decision.variation !== null ? decision.variation.featureEnabled : false; - var logger = this.logger; + var featureEnabled = decision.variation !== null ? decision.variation.featureEnabled : false; var allVariables = {}; featureFlag.variables.forEach(function (variable) { - var variableValue = variable.defaultValue; - if (decision.variation !== null) { - var value = projectConfig.getVariableValueForVariation(configObj, variable, decision.variation, logger); - if (value !== null) { - if (featureEnabled) { - variableValue = value; - logger.log( - LOG_LEVEL.INFO, - sprintf( - LOG_MESSAGES.USER_RECEIVED_VARIABLE_VALUE, - MODULE_NAME, - variable.key, - featureFlag.key, - variableValue, - userId - ) - ); - } else { - logger.log( - LOG_LEVEL.INFO, - sprintf( - LOG_MESSAGES.FEATURE_NOT_ENABLED_RETURN_DEFAULT_VARIABLE_VALUE, - MODULE_NAME, - featureFlag.key, - userId, - variable.key - ) - ); - } - } else { - logger.log( - LOG_LEVEL.INFO, - sprintf( - LOG_MESSAGES.VARIABLE_NOT_USED_RETURN_DEFAULT_VARIABLE_VALUE, - MODULE_NAME, - variable.key, - decision.variation.key - ) - ); - } - } else { - logger.log( - LOG_LEVEL.INFO, - sprintf( - LOG_MESSAGES.USER_RECEIVED_DEFAULT_VARIABLE_VALUE, - MODULE_NAME, - userId, - variable.key, - featureFlag.key - ) - ); - } - var typeCastedValue = projectConfig.getTypeCastValue(variableValue, variable.type, logger); - allVariables[variable.key] = typeCastedValue; - }); + allVariables[variable.key] = this._getFeatureVariableValueFromVariation(featureKey, featureEnabled, decision.variation, variable, userId); + }.bind(this)); var sourceInfo = {}; if (decision.decisionSource === DECISION_SOURCES.FEATURE_TEST) { From 41e9a8d12e430348e4b25f57d53eb877160e94e5 Mon Sep 17 00:00:00 2001 From: fayyazarshad Date: Mon, 4 May 2020 17:31:12 +0500 Subject: [PATCH 5/8] Added types of both methods --- packages/optimizely-sdk/lib/index.d.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/optimizely-sdk/lib/index.d.ts b/packages/optimizely-sdk/lib/index.d.ts index 7a695f24f..acba098fc 100644 --- a/packages/optimizely-sdk/lib/index.d.ts +++ b/packages/optimizely-sdk/lib/index.d.ts @@ -89,6 +89,17 @@ declare module '@optimizely/optimizely-sdk' { userId: string, attributes?: UserAttributes ): string | null; + getFeatureVariableJson( + featureKey: string, + variableKey: string, + userId: string, + attributes?: UserAttributes + ): any; + getAllFeatureVariables( + featureKey: string, + userId: string, + attributes?: UserAttributes + ): any; getOptimizelyConfig(): OptimizelyConfig | null; onReady(options?: { timeout?: number }): Promise<{ success: boolean; reason?: string }>; close(): Promise<{ success: boolean; reason?: string }>; From 3e29a4fc23aa3cc72120701e1340ea13dbf5c8df Mon Sep 17 00:00:00 2001 From: fayyazarshad Date: Mon, 4 May 2020 17:41:17 +0500 Subject: [PATCH 6/8] added null defaults --- packages/optimizely-sdk/lib/index.d.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/optimizely-sdk/lib/index.d.ts b/packages/optimizely-sdk/lib/index.d.ts index acba098fc..bf58c3bcf 100644 --- a/packages/optimizely-sdk/lib/index.d.ts +++ b/packages/optimizely-sdk/lib/index.d.ts @@ -94,12 +94,12 @@ declare module '@optimizely/optimizely-sdk' { variableKey: string, userId: string, attributes?: UserAttributes - ): any; + ): any | null; getAllFeatureVariables( featureKey: string, userId: string, attributes?: UserAttributes - ): any; + ): any | null; getOptimizelyConfig(): OptimizelyConfig | null; onReady(options?: { timeout?: number }): Promise<{ success: boolean; reason?: string }>; close(): Promise<{ success: boolean; reason?: string }>; From 906e4c823d4b53c03525405caa5b2fc78066b31b Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Tue, 5 May 2020 12:30:09 -0700 Subject: [PATCH 7/8] incorporated review feedback --- packages/optimizely-sdk/lib/index.d.ts | 4 +- .../optimizely-sdk/lib/optimizely/index.js | 93 ++++++++----------- 2 files changed, 42 insertions(+), 55 deletions(-) diff --git a/packages/optimizely-sdk/lib/index.d.ts b/packages/optimizely-sdk/lib/index.d.ts index bf58c3bcf..ab0621f86 100644 --- a/packages/optimizely-sdk/lib/index.d.ts +++ b/packages/optimizely-sdk/lib/index.d.ts @@ -94,12 +94,12 @@ declare module '@optimizely/optimizely-sdk' { variableKey: string, userId: string, attributes?: UserAttributes - ): any | null; + ): unknown; getAllFeatureVariables( featureKey: string, userId: string, attributes?: UserAttributes - ): any | null; + ): unknown; getOptimizelyConfig(): OptimizelyConfig | null; onReady(options?: { timeout?: number }): Promise<{ success: boolean; reason?: string }>; close(): Promise<{ success: boolean; reason?: string }>; diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index 14d8f966c..f22e933db 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -788,17 +788,46 @@ Optimizely.prototype._getFeatureVariableValueFromVariation = function(featureKey } var variableValue = variable.defaultValue; - var valueFromVariation = null; - - if (variation) { - valueFromVariation = projectConfig.getVariableValueForVariation(configObj, variable, variation, this.logger); - if (valueFromVariation && featureEnabled) { - variableValue = valueFromVariation; + if (variation !== null) { + var value = projectConfig.getVariableValueForVariation(configObj, variable, variation, this.logger); + if (value !== null) { + if (featureEnabled) { + variableValue = value; + this.logger.log( + LOG_LEVEL.INFO, + sprintf( + LOG_MESSAGES.USER_RECEIVED_VARIABLE_VALUE, + MODULE_NAME, + variable.key, + featureKey, + variableValue, + userId + ) + ); + } else { + this.logger.log( + LOG_LEVEL.INFO, + sprintf( + LOG_MESSAGES.FEATURE_NOT_ENABLED_RETURN_DEFAULT_VARIABLE_VALUE, + MODULE_NAME, + featureKey, + userId, + variable.key + ) + ); + } + } else { + this.logger.log( + LOG_LEVEL.INFO, + sprintf( + LOG_MESSAGES.VARIABLE_NOT_USED_RETURN_DEFAULT_VARIABLE_VALUE, + MODULE_NAME, + variable.key, + variation.key + ) + ); } - } - - // User is not in any variation - if (!variation) { + } else { this.logger.log( LOG_LEVEL.INFO, sprintf( @@ -811,48 +840,6 @@ Optimizely.prototype._getFeatureVariableValueFromVariation = function(featureKey ); } - // When variable value is in variation and feature is enabled - if (variation && valueFromVariation && featureEnabled) { - this.logger.log( - LOG_LEVEL.INFO, - sprintf( - LOG_MESSAGES.USER_RECEIVED_VARIABLE_VALUE, - MODULE_NAME, - variable.key, - featureKey, - variableValue, - userId - ) - ); - } - - // When variable value is in variation but feature is not enabled - if (variation && valueFromVariation && !featureEnabled) { - this.logger.log( - LOG_LEVEL.INFO, - sprintf( - LOG_MESSAGES.FEATURE_NOT_ENABLED_RETURN_DEFAULT_VARIABLE_VALUE, - MODULE_NAME, - featureKey, - userId, - variable.key - ) - ); - } - - // When varible is not used in variation - if (variation && !valueFromVariation) { - this.logger.log( - LOG_LEVEL.INFO, - sprintf( - LOG_MESSAGES.VARIABLE_NOT_USED_RETURN_DEFAULT_VARIABLE_VALUE, - MODULE_NAME, - variable.key, - variation.key - ) - ); - } - return projectConfig.getTypeCastValue(variableValue, variable.type, this.logger); } @@ -1040,7 +1027,7 @@ Optimizely.prototype.getAllFeatureVariables = function(featureKey, userId, attri this.logger.log(LOG_LEVEL.ERROR, e.message); this.errorHandler.handleError(e); return null; - } + } }; /** From dafdf8fce777ad69c34174f13b70f490410b4b53 Mon Sep 17 00:00:00 2001 From: Zeeshan Ashraf Date: Wed, 6 May 2020 09:41:57 -0700 Subject: [PATCH 8/8] added comments to the helper method --- packages/optimizely-sdk/lib/optimizely/index.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index f22e933db..12e5f3282 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -781,6 +781,22 @@ Optimizely.prototype._getFeatureVariableForType = function(featureKey, variableK return variableValue; }; +/** + * Helper method to get the non type-casted value for a variable attached to a + * feature flag. Returns appropriate variable value depending on whether there + * was a matching variation, feature was enabled or not or varible was part of the + * available variation or not. Also logs the appropriate message explaining how it + * evaluated the value of the variable. + * + * @param {string} featureKey Key of the feature whose variable's value is + * being accessed + * @param {boolean} featureEnabled Boolean indicating if feature is enabled or not + * @param {object} variation variation returned by decision service + * @param {object} variable varible whose value is being evaluated + * @param {string} userId ID for the user + * @return {string|null} String value of the variable or null if the config Obj + * is null + */ Optimizely.prototype._getFeatureVariableValueFromVariation = function(featureKey, featureEnabled, variation, variable, userId) { var configObj = this.projectConfigManager.getConfig(); if (!configObj) {