Skip to content

Commit c49a6eb

Browse files
rashidspmikeproeng37
authored andcommitted
chore: Picked commits from latest release to resolve issues in 2.3.x (#259)
1 parent ffd427c commit c49a6eb

File tree

8 files changed

+264
-15
lines changed

8 files changed

+264
-15
lines changed

packages/optimizely-sdk/CHANGELOG.MD

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
55
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
66

77
## [Unreleased]
8-
Changes that have landed but are not yet released.
8+
- 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))
9+
- Decision service: Added bucketing id in getVariationForRollout method ([#200](https://github.com/optimizely/javascript-sdk/pull/200))
10+
- Event tags: Do not exclude falsy revenue and event values in the event payload. ([#213](https://github.com/optimizely/javascript-sdk/pull/213))
911

1012
## [2.3.1] - November 14, 2018
1113

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2017-2018, Optimizely, Inc. and contributors *
2+
* Copyright 2017-2019, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -314,7 +314,7 @@ DecisionService.prototype._getVariationForFeatureExperiment = function(feature,
314314
var group = this.configObj.groupIdMap[feature.groupId];
315315
if (group) {
316316
experiment = this._getExperimentInGroup(group, userId);
317-
if (experiment) {
317+
if (experiment && feature.experimentIds.indexOf(experiment.id) !== -1) {
318318
variationKey = this.getVariation(experiment.key, userId, attributes);
319319
}
320320
}
@@ -383,6 +383,8 @@ DecisionService.prototype._getVariationForRollout = function(feature, userId, at
383383
};
384384
}
385385

386+
var bucketingId = this._getBucketingId(userId, attributes);
387+
386388
// The end index is length - 1 because the last experiment is assumed to be
387389
// "everyone else", which will be evaluated separately outside this loop
388390
var endIndex = rollout.experiments.length - 1;
@@ -400,7 +402,7 @@ DecisionService.prototype._getVariationForRollout = function(feature, userId, at
400402
}
401403

402404
this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.USER_MEETS_CONDITIONS_FOR_TARGETING_RULE, MODULE_NAME, userId, index + 1));
403-
bucketerParams = this.__buildBucketerParams(experiment.key, userId, userId);
405+
bucketerParams = this.__buildBucketerParams(experiment.key, bucketingId, userId);
404406
variationId = bucketer.bucket(bucketerParams);
405407
variation = this.configObj.variationIdMap[variationId];
406408
if (variation) {
@@ -418,7 +420,7 @@ DecisionService.prototype._getVariationForRollout = function(feature, userId, at
418420

419421
var everyoneElseExperiment = this.configObj.experimentKeyMap[rollout.experiments[endIndex].key];
420422
if (this.__checkIfUserIsInAudience(everyoneElseExperiment.key, userId, attributes)) {
421-
bucketerParams = this.__buildBucketerParams(everyoneElseExperiment.key, userId, userId);
423+
bucketerParams = this.__buildBucketerParams(everyoneElseExperiment.key, bucketingId, userId);
422424
variationId = bucketer.bucket(bucketerParams);
423425
variation = this.configObj.variationIdMap[variationId];
424426
if (variation) {

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

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2017-2018, Optimizely, Inc. and contributors *
2+
* Copyright 2017-2019, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -888,6 +888,18 @@ describe('lib/core/decision_service', function() {
888888
assert.deepEqual(decision, expectedDecision);
889889
sinon.assert.calledWithExactly(mockLogger.log, LOG_LEVEL.DEBUG, 'DECISION_SERVICE: User user1 is not in any experiment on the feature feature_with_group.');
890890
});
891+
892+
it('returns null decision for group experiment not referenced by the feature', function() {
893+
var noTrafficExpFeature = configObj.featureKeyMap.feature_exp_no_traffic;
894+
var decision = decisionServiceInstance.getVariationForFeature(noTrafficExpFeature, 'user1');
895+
var expectedDecision = {
896+
experiment: null,
897+
variation: null,
898+
decisionSource: null,
899+
};
900+
assert.deepEqual(decision, expectedDecision);
901+
sinon.assert.calledWithExactly(mockLogger.log, LOG_LEVEL.DEBUG, 'DECISION_SERVICE: User user1 is not in any experiment on the feature feature_exp_no_traffic.');
902+
});
891903
});
892904

893905
describe('user not bucketed into the group', function() {
@@ -1343,5 +1355,47 @@ describe('lib/core/decision_service', function() {
13431355
});
13441356
});
13451357
});
1358+
1359+
describe('_getVariationForRollout', function() {
1360+
var feature;
1361+
var configObj;
1362+
var decisionService;
1363+
var __buildBucketerParamsSpy;
1364+
1365+
beforeEach(function() {
1366+
configObj = projectConfig.createProjectConfig(testDataWithFeatures);
1367+
feature = configObj.featureKeyMap.test_feature;
1368+
decisionService = DecisionService.createDecisionService({
1369+
configObj: configObj,
1370+
logger: logger.createLogger({logLevel: LOG_LEVEL.INFO}),
1371+
});
1372+
__buildBucketerParamsSpy = sinon.spy(decisionService, '__buildBucketerParams');
1373+
});
1374+
1375+
afterEach(function() {
1376+
__buildBucketerParamsSpy.restore();
1377+
});
1378+
1379+
it('should call __buildBucketerParams with user Id when bucketing Id is not provided in the attributes', function () {
1380+
var attributes = { test_attribute: 'test_value' };
1381+
decisionService._getVariationForRollout(feature, 'testUser', attributes);
1382+
1383+
sinon.assert.callCount(__buildBucketerParamsSpy, 2);
1384+
sinon.assert.calledWithExactly(__buildBucketerParamsSpy, '594031', 'testUser', 'testUser');
1385+
sinon.assert.calledWithExactly(__buildBucketerParamsSpy, '594037', 'testUser', 'testUser');
1386+
});
1387+
1388+
it('should call __buildBucketerParams with bucketing Id when bucketing Id is provided in the attributes', function () {
1389+
var attributes = {
1390+
test_attribute: 'test_value',
1391+
$opt_bucketing_id: 'abcdefg'
1392+
};
1393+
decisionService._getVariationForRollout(feature, 'testUser', attributes);
1394+
1395+
sinon.assert.callCount(__buildBucketerParamsSpy, 2);
1396+
sinon.assert.calledWithExactly(__buildBucketerParamsSpy, '594031', 'abcdefg', 'testUser');
1397+
sinon.assert.calledWithExactly(__buildBucketerParamsSpy, '594037', 'abcdefg', 'testUser');
1398+
});
1399+
});
13461400
});
13471401
});

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2016-2018, Optimizely
2+
* Copyright 2016-2019, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -144,12 +144,12 @@ function getVisitorSnapshot(configObj, eventKey, eventTags, experimentsToVariati
144144

145145
if (eventTags) {
146146
var revenue = eventTagUtils.getRevenueValue(eventTags, logger);
147-
if (revenue) {
147+
if (revenue !== null) {
148148
eventDict[enums.RESERVED_EVENT_KEYWORDS.REVENUE] = revenue;
149149
}
150150

151151
var eventValue = eventTagUtils.getEventValue(eventTags, logger);
152-
if (eventValue) {
152+
if (eventValue !== null) {
153153
eventDict[enums.RESERVED_EVENT_KEYWORDS.VALUE] = eventValue;
154154
}
155155

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

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2016-2018, Optimizely
2+
* Copyright 2016-2019, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -1081,6 +1081,54 @@ describe('lib/core/event_builder', function() {
10811081
assert.deepEqual(actualParams, expectedParams);
10821082
});
10831083

1084+
it('should include revenue value of 0 in the event object', function() {
1085+
var expectedParams = {
1086+
url: 'https://logx.optimizely.com/v1/events',
1087+
httpVerb: 'POST',
1088+
params: {
1089+
'client_version': packageJSON.version,
1090+
'project_id': '111001',
1091+
'visitors': [{
1092+
'attributes': [],
1093+
'visitor_id': 'testUser',
1094+
'snapshots': [{
1095+
'events': [{
1096+
'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c',
1097+
'tags': {
1098+
'revenue': 0
1099+
},
1100+
'timestamp': Math.round(new Date().getTime()),
1101+
'revenue': 0,
1102+
'key': 'testEvent',
1103+
'entity_id': '111095'
1104+
}]
1105+
}]
1106+
}],
1107+
'account_id': '12001',
1108+
'client_name': 'node-sdk',
1109+
'revision': '42',
1110+
'anonymize_ip': false,
1111+
'enrich_decisions': true,
1112+
},
1113+
};
1114+
1115+
var eventOptions = {
1116+
clientEngine: 'node-sdk',
1117+
clientVersion: packageJSON.version,
1118+
configObj: configObj,
1119+
eventKey: 'testEvent',
1120+
eventTags: {
1121+
'revenue': 0,
1122+
},
1123+
logger: mockLogger,
1124+
userId: 'testUser',
1125+
};
1126+
1127+
var actualParams = eventBuilder.getConversionEvent(eventOptions);
1128+
1129+
assert.deepEqual(actualParams, expectedParams);
1130+
});
1131+
10841132
describe('and the revenue value is invalid', function() {
10851133
it('should not include the revenue value in the event object', function() {
10861134
var expectedParams = {
@@ -1194,6 +1242,54 @@ describe('lib/core/event_builder', function() {
11941242
assert.deepEqual(actualParams, expectedParams);
11951243
});
11961244

1245+
it('should include the falsy event values in the event object', function() {
1246+
var expectedParams = {
1247+
url: 'https://logx.optimizely.com/v1/events',
1248+
httpVerb: 'POST',
1249+
params: {
1250+
'client_version': packageJSON.version,
1251+
'project_id': '111001',
1252+
'visitors': [{
1253+
'attributes': [],
1254+
'visitor_id': 'testUser',
1255+
'snapshots': [{
1256+
'events': [{
1257+
'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c',
1258+
'tags': {
1259+
'value': '0.0'
1260+
},
1261+
'timestamp': Math.round(new Date().getTime()),
1262+
'value': 0.0,
1263+
'key': 'testEvent',
1264+
'entity_id': '111095'
1265+
}]
1266+
}]
1267+
}],
1268+
'account_id': '12001',
1269+
'client_name': 'node-sdk',
1270+
'revision': '42',
1271+
'anonymize_ip': false,
1272+
'enrich_decisions': true,
1273+
},
1274+
};
1275+
1276+
var eventOptions = {
1277+
clientEngine: 'node-sdk',
1278+
clientVersion: packageJSON.version,
1279+
configObj: configObj,
1280+
eventKey: 'testEvent',
1281+
eventTags: {
1282+
'value': '0.0',
1283+
},
1284+
logger: mockLogger,
1285+
userId: 'testUser',
1286+
};
1287+
1288+
var actualParams = eventBuilder.getConversionEvent(eventOptions);
1289+
1290+
assert.deepEqual(actualParams, expectedParams);
1291+
});
1292+
11971293
describe('and the event value is invalid', function() {
11981294
it('should not include the event value in the event object', function() {
11991295
var expectedParams = {

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2016-2018, Optimizely, Inc. and contributors *
2+
* Copyright 2016-2019, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -237,7 +237,6 @@ Optimizely.prototype.track = function(eventKey, userId, attributes, eventTags) {
237237

238238
// remove null values from eventTags
239239
eventTags = this.__filterEmptyValues(eventTags);
240-
241240
var conversionEventOptions = {
242241
attributes: attributes,
243242
clientEngine: this.clientEngine,

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2016-2018, Optimizely, Inc. and contributors *
2+
* Copyright 2016-2019, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -2955,7 +2955,7 @@ describe('lib/optimizely', function() {
29552955
assert.strictEqual(result.length, 2);
29562956
assert.isAbove(result.indexOf('test_feature'), -1);
29572957
assert.isAbove(result.indexOf('test_feature_for_experiment'), -1);
2958-
sinon.assert.callCount(optlyInstance.isFeatureEnabled, 6);
2958+
sinon.assert.callCount(optlyInstance.isFeatureEnabled, 7);
29592959
sinon.assert.calledWithExactly(
29602960
optlyInstance.isFeatureEnabled,
29612961
'test_feature',
@@ -2992,6 +2992,12 @@ describe('lib/optimizely', function() {
29922992
'user1',
29932993
attributes
29942994
);
2995+
sinon.assert.calledWithExactly(
2996+
optlyInstance.isFeatureEnabled,
2997+
'feature_exp_no_traffic',
2998+
'user1',
2999+
attributes
3000+
);
29953001
});
29963002
});
29973003

0 commit comments

Comments
 (0)