diff --git a/packages/optimizely-sdk/CHANGELOG.MD b/packages/optimizely-sdk/CHANGELOG.MD index 2161d0d4a..a4e492ed8 100644 --- a/packages/optimizely-sdk/CHANGELOG.MD +++ b/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 diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index 315e7ef3d..a487503e5 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.js +++ b/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) { 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 bff837d64..29b81665c 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.tests.js +++ b/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'); + }); + }); }); }); diff --git a/packages/optimizely-sdk/lib/core/event_builder/index.js b/packages/optimizely-sdk/lib/core/event_builder/index.js index bb06d2a8a..dbec9a382 100644 --- a/packages/optimizely-sdk/lib/core/event_builder/index.js +++ b/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; } diff --git a/packages/optimizely-sdk/lib/core/event_builder/index.tests.js b/packages/optimizely-sdk/lib/core/event_builder/index.tests.js index 7e0dd7798..856314954 100644 --- a/packages/optimizely-sdk/lib/core/event_builder/index.tests.js +++ b/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 = { diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index 8c81d2347..35e770d7e 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/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, diff --git a/packages/optimizely-sdk/lib/optimizely/index.tests.js b/packages/optimizely-sdk/lib/optimizely/index.tests.js index 7ba66fcf5..ec53adb8c 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.tests.js +++ b/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 + ); }); }); diff --git a/packages/optimizely-sdk/lib/tests/test_data.js b/packages/optimizely-sdk/lib/tests/test_data.js index 71b7fa724..f5c6f412a 100644 --- a/packages/optimizely-sdk/lib/tests/test_data.js +++ b/packages/optimizely-sdk/lib/tests/test_data.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. @@ -417,6 +417,15 @@ var configWithFeatures = { 'id': '599110', 'experimentIds': [], 'variables': [] + }, + { + 'rolloutId': '', + 'key': 'feature_exp_no_traffic', + 'id': '4482920079', + 'experimentIds': [ + '12115595439' + ], + 'variables': [] } ], 'experiments': [ @@ -625,6 +634,74 @@ var configWithFeatures = { 'entityId': '599082' } ] + }, + { + 'policy': 'random', + 'id': '595025', + 'experiments': [ + { + 'trafficAllocation': [ + { + 'endOfRange': 10000, + 'entityId': '12098126627' + } + ], + 'layerId': '595005', + 'forcedVariations': {}, + 'audienceIds': [], + 'variations': [ + { + 'key': 'all_traffic_variation', + 'id': '12098126627', + 'variables': [] + }, + { + 'key': 'no_traffic_variation', + 'id': '12098126628', + 'variables': [] + } + ], + 'status': 'Running', + 'key': 'all_traffic_experiment', + 'id': '12198292375' + }, + { + 'trafficAllocation': [ + { + 'endOfRange': 5000, + 'entityId': '12098126629' + }, + { + 'endOfRange': 10000, + 'entityId': '12098126630' + } + ], + 'layerId': '12187694826', + 'forcedVariations': {}, + 'audienceIds': [], + 'variations': [ + { + 'key': 'variation_5000', + 'id': '12098126629', + 'variables': [] + }, + { + 'key': 'variation_10000', + 'id': '12098126630', + 'variables': [] + } + ], + 'status': 'Running', + 'key': 'no_traffic_experiment', + 'id': '12115595439' + } + ], + 'trafficAllocation': [ + { + 'endOfRange': 10000, + 'entityId': '12198292375' + } + ] } ], 'attributes': [ @@ -1321,6 +1398,10 @@ var datafileWithFeaturesExpectedData = { }, 599080: {}, 599081: {}, + 12098126627: {}, + 12098126628: {}, + 12098126629: {}, + 12098126630: {}, }, featureKeyMap: { @@ -1535,6 +1616,15 @@ var datafileWithFeaturesExpectedData = { 'variables': [], variableKeyMap: {}, }, + feature_exp_no_traffic: { + 'rolloutId': '', + 'key': 'feature_exp_no_traffic', + 'id': '4482920079', + 'experimentIds': ['12115595439'], + 'variables': [], + variableKeyMap: {}, + groupId: '595025', + }, }, };