fix(bucketer): Don't log invalid variation ID warning when bucketed into a traffic allocation range with empty string entityId (#549)

Summary:

Fix an issue introduced in #515. When bucket returns empty string, we shouldn't log a warning about invalid variation ID. Empty string is a valid value for the entityId property of traffic allocation range objects. There is no invalid variation ID in this situation.

Test plan:

Manually tested with A/B tests and rollouts. Added new unit test.

Issues:

OASIS-6890

Author: mjc1283

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

@@ -114,8 +114,10 @@ export var bucket = function(bucketerParams) {
   var entityId = this._findBucket(bucketValue, bucketerParams.trafficAllocationConfig);
 
   if (!bucketerParams.variationIdMap.hasOwnProperty(entityId)) {
-    var invalidVariationIdLogMessage = sprintf(LOG_MESSAGES.INVALID_VARIATION_ID, MODULE_NAME);
-    bucketerParams.logger.log(LOG_LEVEL.WARNING, invalidVariationIdLogMessage);
+    if (entityId) {
+      var invalidVariationIdLogMessage = sprintf(LOG_MESSAGES.INVALID_VARIATION_ID, MODULE_NAME);
+      bucketerParams.logger.log(LOG_LEVEL.WARNING, invalidVariationIdLogMessage);
+    }
     return null;
   }
 
@@ -128,7 +130,7 @@ export var bucket = function(bucketerParams) {
  * @param {string} bucketingId  Bucketing ID
  * @param {string} userId       ID of user to be bucketed into experiment
  * @param {Object} logger       Logger implementation
- * @return {string} ID of experiment if user is bucketed into experiment within the group, null otherwise
+ * @return {string|null} ID of experiment if user is bucketed into experiment within the group, null otherwise
  */
 export var bucketUserIntoExperiment = function(group, bucketingId, userId, logger) {
   var bucketingKey = sprintf('%s%s', bucketingId, group.id);
@@ -148,7 +150,7 @@ export var bucketUserIntoExperiment = function(group, bucketingId, userId, logge
  * @param  {Object[]} trafficAllocationConfig
  * @param  {number}   trafficAllocationConfig[].endOfRange
  * @param  {number}   trafficAllocationConfig[].entityId
- * @return {string}   Entity ID for bucketing if bucket value is within traffic allocation boundaries, null otherwise
+ * @return {string|null}   Entity ID for bucketing if bucket value is within traffic allocation boundaries, null otherwise
  */
 export var _findBucket = function(bucketValue, trafficAllocationConfig) {
   for (var i = 0; i < trafficAllocationConfig.length; i++) {

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

@@ -286,6 +286,15 @@ describe('lib/core/bucketer', function() {
           bucketerParamsTest1.userId = 'ppid1';
           expect(bucketer.bucket(bucketerParamsTest1)).to.equal(null);
         });
+
+        it('should not log an invalid variation ID warning', function() {
+          bucketer.bucket(bucketerParams)
+          const foundInvalidVariationWarning = createdLogger.log.getCalls().some((call) => {
+            const message = call.args[1];
+            return message.includes('Bucketed into an invalid variation ID')
+          });
+          expect(foundInvalidVariationWarning).to.equal(false);
+        });
       });
 
       describe('when the traffic allocation has invalid variation ids', function() {