Picked commits from latest release to resolve issues in 2.3.x

packages/optimizely-sdk/CHANGELOG.MD

@@ -5,7 +5,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
-Changes that have landed but are not yet released.
+- Decision service: Return null variation if experiment key does not exist in feature's experimentIds array. ([#206](https://github.com/optimizely/javascript-sdk/pull/206))
+- Decision service: Added bucketing id in getVariationForRollout method ([#200](https://github.com/optimizely/javascript-sdk/pull/200))
+- Event tags: Do not exclude falsy revenue and event values in the event payload. ([#213](https://github.com/optimizely/javascript-sdk/pull/213))
 
 ## [2.3.1] - November 14, 2018
 

packages/optimizely-sdk/lib/core/decision_service/index.js

@@ -1,5 +1,5 @@
 /****************************************************************************
- * Copyright 2017-2018, Optimizely, Inc. and contributors                   *
+ * Copyright 2017-2019, Optimizely, Inc. and contributors                   *
  *                                                                          *
  * Licensed under the Apache License, Version 2.0 (the "License");          *
  * you may not use this file except in compliance with the License.         *
@@ -314,7 +314,7 @@ DecisionService.prototype._getVariationForFeatureExperiment = function(feature,
     var group = this.configObj.groupIdMap[feature.groupId];
     if (group) {
       experiment = this._getExperimentInGroup(group, userId);
-      if (experiment) {
+      if (experiment && feature.experimentIds.indexOf(experiment.id) !== -1) {
         variationKey = this.getVariation(experiment.key, userId, attributes);
       }
     }
@@ -383,6 +383,8 @@ DecisionService.prototype._getVariationForRollout = function(feature, userId, at
     };
   }
 
+  var bucketingId = this._getBucketingId(userId, attributes);
+
   // The end index is length - 1 because the last experiment is assumed to be
   // "everyone else", which will be evaluated separately outside this loop
   var endIndex = rollout.experiments.length - 1;
@@ -400,7 +402,7 @@ DecisionService.prototype._getVariationForRollout = function(feature, userId, at
     }
 
     this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.USER_MEETS_CONDITIONS_FOR_TARGETING_RULE, MODULE_NAME, userId, index + 1));
-    bucketerParams = this.__buildBucketerParams(experiment.key, userId, userId);
+    bucketerParams = this.__buildBucketerParams(experiment.key, bucketingId, userId);
     variationId = bucketer.bucket(bucketerParams);
     variation = this.configObj.variationIdMap[variationId];
     if (variation) {
@@ -418,7 +420,7 @@ DecisionService.prototype._getVariationForRollout = function(feature, userId, at
 
   var everyoneElseExperiment = this.configObj.experimentKeyMap[rollout.experiments[endIndex].key];
   if (this.__checkIfUserIsInAudience(everyoneElseExperiment.key, userId, attributes)) {
-    bucketerParams = this.__buildBucketerParams(everyoneElseExperiment.key, userId, userId);
+    bucketerParams = this.__buildBucketerParams(everyoneElseExperiment.key, bucketingId, userId);
     variationId = bucketer.bucket(bucketerParams);
     variation = this.configObj.variationIdMap[variationId];
     if (variation) {

packages/optimizely-sdk/lib/core/decision_service/index.tests.js

@@ -1,5 +1,5 @@
 /****************************************************************************
- * Copyright 2017-2018, Optimizely, Inc. and contributors                        *
+ * Copyright 2017-2019, Optimizely, Inc. and contributors                        *
  *                                                                          *
  * Licensed under the Apache License, Version 2.0 (the "License");          *
  * you may not use this file except in compliance with the License.         *
@@ -888,6 +888,18 @@ describe('lib/core/decision_service', function() {
             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.');
           });
+
+          it('returns null decision for group experiment not referenced by the feature', function() {
+            var noTrafficExpFeature = configObj.featureKeyMap.feature_exp_no_traffic;
+            var decision = decisionServiceInstance.getVariationForFeature(noTrafficExpFeature, 'user1');
+            var expectedDecision = {
+              experiment: null,
+              variation: null,
+              decisionSource: null,
+            };
+            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.');
+          });
         });
 
         describe('user not bucketed into the group', function() {
@@ -1343,5 +1355,47 @@ describe('lib/core/decision_service', function() {
         });
       });
     });
+
+    describe('_getVariationForRollout', function() {
+      var feature;
+      var configObj;
+      var decisionService;
+      var __buildBucketerParamsSpy;
+
+      beforeEach(function() {
+        configObj = projectConfig.createProjectConfig(testDataWithFeatures);
+        feature = configObj.featureKeyMap.test_feature;
+        decisionService = DecisionService.createDecisionService({
+          configObj: configObj,
+          logger: logger.createLogger({logLevel: LOG_LEVEL.INFO}),
+        });
+        __buildBucketerParamsSpy = sinon.spy(decisionService, '__buildBucketerParams');
+      });
+
+      afterEach(function() {
+        __buildBucketerParamsSpy.restore();
+      });
+
+      it('should call __buildBucketerParams with user Id when bucketing Id is not provided in the attributes', function () {
+        var attributes = { test_attribute: 'test_value' };
+        decisionService._getVariationForRollout(feature, 'testUser', attributes);
+
+        sinon.assert.callCount(__buildBucketerParamsSpy, 2);
+        sinon.assert.calledWithExactly(__buildBucketerParamsSpy, '594031', 'testUser', 'testUser');
+        sinon.assert.calledWithExactly(__buildBucketerParamsSpy, '594037', 'testUser', 'testUser');
+      });
+
+      it('should call __buildBucketerParams with bucketing Id when bucketing Id is provided in the attributes', function () {
+        var attributes = {
+          test_attribute: 'test_value',
+          $opt_bucketing_id: 'abcdefg' 
+        };
+        decisionService._getVariationForRollout(feature, 'testUser', attributes);
+
+        sinon.assert.callCount(__buildBucketerParamsSpy, 2);
+        sinon.assert.calledWithExactly(__buildBucketerParamsSpy, '594031', 'abcdefg', 'testUser');
+        sinon.assert.calledWithExactly(__buildBucketerParamsSpy, '594037', 'abcdefg', 'testUser');
+      });
+    });
   });
 });

packages/optimizely-sdk/lib/core/event_builder/index.js

@@ -1,5 +1,5 @@
 /**
- * Copyright 2016-2018, Optimizely
+ * Copyright 2016-2019, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -144,12 +144,12 @@ function getVisitorSnapshot(configObj, eventKey, eventTags, experimentsToVariati
 
   if (eventTags) {
     var revenue = eventTagUtils.getRevenueValue(eventTags, logger);
-    if (revenue) {
+    if (revenue !== null) {
       eventDict[enums.RESERVED_EVENT_KEYWORDS.REVENUE] = revenue;
     }
 
     var eventValue = eventTagUtils.getEventValue(eventTags, logger);
-    if (eventValue) {
+    if (eventValue !== null) {
       eventDict[enums.RESERVED_EVENT_KEYWORDS.VALUE] = eventValue;
     }
 

packages/optimizely-sdk/lib/core/event_builder/index.tests.js

@@ -1,5 +1,5 @@
 /**
- * Copyright 2016-2018, Optimizely
+ * Copyright 2016-2019, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -1081,6 +1081,54 @@ describe('lib/core/event_builder', function() {
             assert.deepEqual(actualParams, expectedParams);
           });
 
+          it('should include revenue value of 0 in the event object', function() {
+            var expectedParams = {
+              url: 'https://logx.optimizely.com/v1/events',
+              httpVerb: 'POST',
+              params: {
+                'client_version': packageJSON.version,
+                'project_id': '111001',
+                'visitors': [{
+                  'attributes': [],
+                  'visitor_id': 'testUser',
+                  'snapshots': [{
+                    'events': [{
+                      'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c',
+                      'tags': {
+                        'revenue': 0
+                      },
+                      'timestamp': Math.round(new Date().getTime()),
+                      'revenue': 0,
+                      'key': 'testEvent',
+                      'entity_id': '111095'
+                    }]
+                  }]
+                }],
+                'account_id': '12001',
+                'client_name': 'node-sdk',
+                'revision': '42',
+                'anonymize_ip': false,
+                'enrich_decisions': true,
+              },
+            };
+
+            var eventOptions = {
+              clientEngine: 'node-sdk',
+              clientVersion: packageJSON.version,
+              configObj: configObj,
+              eventKey: 'testEvent',
+              eventTags: {
+                'revenue': 0,
+              },
+              logger: mockLogger,
+              userId: 'testUser',
+            };
+
+            var actualParams = eventBuilder.getConversionEvent(eventOptions);
+
+            assert.deepEqual(actualParams, expectedParams);
+          });
+
           describe('and the revenue value is invalid', function() {
             it('should not include the revenue value in the event object', function() {
               var expectedParams = {
@@ -1194,6 +1242,54 @@ describe('lib/core/event_builder', function() {
             assert.deepEqual(actualParams, expectedParams);
           });
 
+          it('should include the falsy event values in the event object', function() {
+            var expectedParams = {
+              url: 'https://logx.optimizely.com/v1/events',
+              httpVerb: 'POST',
+              params: {
+                'client_version': packageJSON.version,
+                'project_id': '111001',
+                'visitors': [{
+                  'attributes': [],
+                  'visitor_id': 'testUser',
+                  'snapshots': [{
+                    'events': [{
+                      'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c',
+                      'tags': {
+                        'value': '0.0'
+                      },
+                      'timestamp': Math.round(new Date().getTime()),
+                      'value': 0.0,
+                      'key': 'testEvent',
+                      'entity_id': '111095'
+                    }]
+                  }]
+                }],
+                'account_id': '12001',
+                'client_name': 'node-sdk',
+                'revision': '42',
+                'anonymize_ip': false,
+                'enrich_decisions': true,
+              },
+            };
+
+            var eventOptions = {
+              clientEngine: 'node-sdk',
+              clientVersion: packageJSON.version,
+              configObj: configObj,
+              eventKey: 'testEvent',
+              eventTags: {
+                'value': '0.0',
+              },
+              logger: mockLogger,
+              userId: 'testUser',
+            };
+
+            var actualParams = eventBuilder.getConversionEvent(eventOptions);
+
+            assert.deepEqual(actualParams, expectedParams);
+          });
+
           describe('and the event value is invalid', function() {
             it('should not include the event value in the event object', function() {
               var expectedParams = {

packages/optimizely-sdk/lib/optimizely/index.js

@@ -1,5 +1,5 @@
 /****************************************************************************
- * Copyright 2016-2018, Optimizely, Inc. and contributors                   *
+ * Copyright 2016-2019, Optimizely, Inc. and contributors                   *
  *                                                                          *
  * Licensed under the Apache License, Version 2.0 (the "License");          *
  * you may not use this file except in compliance with the License.         *
@@ -237,7 +237,6 @@ Optimizely.prototype.track = function(eventKey, userId, attributes, eventTags) {
 
       // remove null values from eventTags      
       eventTags = this.__filterEmptyValues(eventTags);
-
       var conversionEventOptions = {
         attributes: attributes,
         clientEngine: this.clientEngine,

packages/optimizely-sdk/lib/optimizely/index.tests.js

@@ -1,5 +1,5 @@
 /****************************************************************************
- * Copyright 2016-2018, Optimizely, Inc. and contributors                   *
+ * Copyright 2016-2019, Optimizely, Inc. and contributors                   *
  *                                                                          *
  * Licensed under the Apache License, Version 2.0 (the "License");          *
  * you may not use this file except in compliance with the License.         *
@@ -2955,7 +2955,7 @@ describe('lib/optimizely', function() {
         assert.strictEqual(result.length, 2);
         assert.isAbove(result.indexOf('test_feature'), -1);
         assert.isAbove(result.indexOf('test_feature_for_experiment'), -1);
-        sinon.assert.callCount(optlyInstance.isFeatureEnabled, 6);
+        sinon.assert.callCount(optlyInstance.isFeatureEnabled, 7);
         sinon.assert.calledWithExactly(
           optlyInstance.isFeatureEnabled,
           'test_feature',
@@ -2992,6 +2992,12 @@ describe('lib/optimizely', function() {
           'user1',
           attributes
         );
+        sinon.assert.calledWithExactly(
+          optlyInstance.isFeatureEnabled,
+          'feature_exp_no_traffic',
+          'user1',
+          attributes
+        );
       });
     });