From 661ca6bd789a8ad593c918607cbf34af6101dc60 Mon Sep 17 00:00:00 2001 From: Muhammad Fahad Ahmed Date: Sat, 12 Jan 2019 00:21:05 +0500 Subject: [PATCH 1/6] fix(decision_service): Return null variation if experiment key does not exist in feature's experimentIds array. (#206) (cherry picked from commit cfea9cef970f06d2047fa78d7c1fa1316fbae289) --- .../lib/core/decision_service/index.js | 4 +- .../lib/core/decision_service/index.tests.js | 14 ++- .../lib/optimizely/index.tests.js | 10 +- .../optimizely-sdk/lib/tests/test_data.js | 92 ++++++++++++++++++- 4 files changed, 114 insertions(+), 6 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index 315e7ef3d..2a579fbb2 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); } } 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..b00df65cb 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() { 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', + }, }, }; From d6c32767c1608da423f2636fd56733e17d2d2685 Mon Sep 17 00:00:00 2001 From: Muhammad Fahad Ahmed Date: Mon, 14 Jan 2019 23:53:56 +0500 Subject: [PATCH 2/6] fix(decision-service): Adds bucketing id in getVariationForRollout method. (#200) (cherry picked from commit 9812af8d8edd5b15aaf04d70611242563569cda7) --- .../lib/core/decision_service/index.js | 6 ++- .../lib/core/decision_service/index.tests.js | 42 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index 2a579fbb2..a487503e5 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.js @@ -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 b00df65cb..29b81665c 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.tests.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.tests.js @@ -1355,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'); + }); + }); }); }); From aaf2e12eef920796ed0bbe8e5451b685c9c3771b Mon Sep 17 00:00:00 2001 From: Michael Ng Date: Mon, 28 Jan 2019 15:08:01 -0800 Subject: [PATCH 3/6] fix(event-tags): do not exclude falsy revenue and event values in the event payload (#213) (cherry picked from commit cac996b2d055fce9c213f0841a691fcf91f34049) --- .../lib/core/event_builder/index.js | 4 +- .../lib/core/event_builder/index.tests.js | 96 +++++++++++++++++++ .../optimizely-sdk/lib/optimizely/index.js | 1 - 3 files changed, 98 insertions(+), 3 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/event_builder/index.js b/packages/optimizely-sdk/lib/core/event_builder/index.js index bb06d2a8a..8f1853da4 100644 --- a/packages/optimizely-sdk/lib/core/event_builder/index.js +++ b/packages/optimizely-sdk/lib/core/event_builder/index.js @@ -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..10bc357d0 100644 --- a/packages/optimizely-sdk/lib/core/event_builder/index.tests.js +++ b/packages/optimizely-sdk/lib/core/event_builder/index.tests.js @@ -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..6c628f6cc 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -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, From 497a1b7cb89518cf7ff81599f74f70b41b8b37ff Mon Sep 17 00:00:00 2001 From: rashidsp Date: Mon, 22 Apr 2019 15:57:47 +0500 Subject: [PATCH 4/6] Updates headers --- packages/optimizely-sdk/lib/core/event_builder/index.js | 2 +- packages/optimizely-sdk/lib/core/event_builder/index.tests.js | 2 +- packages/optimizely-sdk/lib/optimizely/index.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/event_builder/index.js b/packages/optimizely-sdk/lib/core/event_builder/index.js index 8f1853da4..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. 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 10bc357d0..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. diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index 6c628f6cc..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. * From b98158eda8202b9169a2408f355240ab9298cc41 Mon Sep 17 00:00:00 2001 From: Owais Date: Fri, 3 May 2019 19:58:19 +0500 Subject: [PATCH 5/6] Updated CHANGELOG.MD --- packages/optimizely-sdk/CHANGELOG.MD | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/optimizely-sdk/CHANGELOG.MD b/packages/optimizely-sdk/CHANGELOG.MD index 2161d0d4a..509533265 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.(https://github.com/optimizely/javascript-sdk/pull/206) +- Decision service: Added bucketing id in getVariationForRollout method (https://github.com/optimizely/javascript-sdk/pull/200). +- Event tags: Do not exclude falsy revenue and event values in the event payload. (https://github.com/optimizely/javascript-sdk/pull/213) ## [2.3.1] - November 14, 2018 From 18c8bd5060103987ee8c4e1801f077420c0b29cc Mon Sep 17 00:00:00 2001 From: Owais Date: Fri, 3 May 2019 20:00:45 +0500 Subject: [PATCH 6/6] make PR links --- packages/optimizely-sdk/CHANGELOG.MD | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/optimizely-sdk/CHANGELOG.MD b/packages/optimizely-sdk/CHANGELOG.MD index 509533265..a4e492ed8 100644 --- a/packages/optimizely-sdk/CHANGELOG.MD +++ b/packages/optimizely-sdk/CHANGELOG.MD @@ -5,9 +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] -- Decision service: Return null variation if experiment key does not exist in feature's experimentIds array.(https://github.com/optimizely/javascript-sdk/pull/206) -- Decision service: Added bucketing id in getVariationForRollout method (https://github.com/optimizely/javascript-sdk/pull/200). -- Event tags: Do not exclude falsy revenue and event values in the event payload. (https://github.com/optimizely/javascript-sdk/pull/213) +- 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