From a85e7bad8f6c25b8aa6451aa68d28453d4674130 Mon Sep 17 00:00:00 2001 From: Ali Abbas Rizvi Date: Tue, 21 Aug 2018 13:01:12 -0700 Subject: [PATCH 1/2] fix(track): Send decisions for all experiments using an event when using track (#136) --- .travis.yml | 2 +- optimizely/event_builder.py | 44 +++++------ tests/base.py | 151 +++++++++++++++++++++++++++++++++++- tests/test_event_builder.py | 63 ++++++++++++++- 4 files changed, 234 insertions(+), 26 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8eacd7c7..e4ece294 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ language: python python: - "2.7" - "3.4" - - "3.5" + - "3.5.5" - "3.6" - "pypy" - "pypy3" diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index 087dc1bf..47d95106 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -243,40 +243,41 @@ def _get_required_params_for_conversion(self, event_key, event_tags, decisions): Returns: Dict consisting of the decisions and events info for conversion event. """ + snapshot = {} + snapshot[self.EventParams.DECISIONS] = [] for experiment_id, variation_id in decisions: - snapshot = {} + experiment = self.config.get_experiment_from_id(experiment_id) if variation_id: - snapshot[self.EventParams.DECISIONS] = [{ + snapshot[self.EventParams.DECISIONS].append({ self.EventParams.EXPERIMENT_ID: experiment_id, self.EventParams.VARIATION_ID: variation_id, self.EventParams.CAMPAIGN_ID: experiment.layerId - }] + }) - event_dict = { - self.EventParams.EVENT_ID: self.config.get_event(event_key).id, - self.EventParams.TIME: self._get_time(), - self.EventParams.KEY: event_key, - self.EventParams.UUID: str(uuid.uuid4()) - } - - if event_tags: - revenue_value = event_tag_utils.get_revenue_value(event_tags) - if revenue_value is not None: - event_dict[event_tag_utils.REVENUE_METRIC_TYPE] = revenue_value + event_dict = { + self.EventParams.EVENT_ID: self.config.get_event(event_key).id, + self.EventParams.TIME: self._get_time(), + self.EventParams.KEY: event_key, + self.EventParams.UUID: str(uuid.uuid4()) + } - numeric_value = event_tag_utils.get_numeric_value(event_tags, self.config.logger) - if numeric_value is not None: - event_dict[event_tag_utils.NUMERIC_METRIC_TYPE] = numeric_value + if event_tags: + revenue_value = event_tag_utils.get_revenue_value(event_tags) + if revenue_value is not None: + event_dict[event_tag_utils.REVENUE_METRIC_TYPE] = revenue_value - if len(event_tags) > 0: - event_dict[self.EventParams.TAGS] = event_tags + numeric_value = event_tag_utils.get_numeric_value(event_tags, self.config.logger) + if numeric_value is not None: + event_dict[event_tag_utils.NUMERIC_METRIC_TYPE] = numeric_value - snapshot[self.EventParams.EVENTS] = [event_dict] + if len(event_tags) > 0: + event_dict[self.EventParams.TAGS] = event_tags - return snapshot + snapshot[self.EventParams.EVENTS] = [event_dict] + return snapshot def create_impression_event(self, experiment, variation_id, user_id, attributes): """ Create impression Event to be sent to the logging endpoint. @@ -319,7 +320,6 @@ def create_conversion_event(self, event_key, user_id, attributes, event_tags, de conversion_params = self._get_required_params_for_conversion(event_key, event_tags, decisions) params[self.EventParams.USERS][0][self.EventParams.SNAPSHOTS].append(conversion_params) - return Event(self.EVENTS_URL, params, http_verb=self.HTTP_VERB, diff --git a/tests/base.py b/tests/base.py index 72b78c7a..05be7935 100644 --- a/tests/base.py +++ b/tests/base.py @@ -19,7 +19,7 @@ class BaseTest(unittest.TestCase): - def setUp(self): + def setUp(self, config_dict='config_dict'): self.config_dict = { 'revision': '42', 'version': '2', @@ -375,5 +375,152 @@ def setUp(self): }] } - self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict)) + self.config_dict_with_multiple_experiments = { + 'revision': '42', + 'version': '2', + 'events': [{ + 'key': 'test_event', + 'experimentIds': ['111127', '111130'], + 'id': '111095' + }, { + 'key': 'Total Revenue', + 'experimentIds': ['111127'], + 'id': '111096' + }], + 'experiments': [{ + 'key': 'test_experiment', + 'status': 'Running', + 'forcedVariations': { + 'user_1': 'control', + 'user_2': 'control' + }, + 'layerId': '111182', + 'audienceIds': ['11154'], + 'trafficAllocation': [{ + 'entityId': '111128', + 'endOfRange': 4000 + }, { + 'entityId': '', + 'endOfRange': 5000 + }, { + 'entityId': '111129', + 'endOfRange': 9000 + }], + 'id': '111127', + 'variations': [{ + 'key': 'control', + 'id': '111128' + }, { + 'key': 'variation', + 'id': '111129' + }] + }, { + 'key': 'test_experiment_2', + 'status': 'Running', + 'forcedVariations': { + 'user_1': 'control', + 'user_2': 'control' + }, + 'layerId': '111182', + 'audienceIds': ['11154'], + 'trafficAllocation': [{ + 'entityId': '111131', + 'endOfRange': 4000 + }, { + 'entityId': '', + 'endOfRange': 5000 + }, { + 'entityId': '111132', + 'endOfRange': 9000 + }], + 'id': '111130', + 'variations': [{ + 'key': 'control', + 'id': '111133' + }, { + 'key': 'variation', + 'id': '111134' + }] + }], + 'groups': [{ + 'id': '19228', + 'policy': 'random', + 'experiments': [{ + 'id': '32222', + 'key': 'group_exp_1', + 'status': 'Running', + 'audienceIds': [], + 'layerId': '111183', + 'variations': [{ + 'key': 'group_exp_1_control', + 'id': '28901' + }, { + 'key': 'group_exp_1_variation', + 'id': '28902' + }], + 'forcedVariations': { + 'user_1': 'group_exp_1_control', + 'user_2': 'group_exp_1_control' + }, + 'trafficAllocation': [{ + 'entityId': '28901', + 'endOfRange': 3000 + }, { + 'entityId': '28902', + 'endOfRange': 9000 + }] + }, { + 'id': '32223', + 'key': 'group_exp_2', + 'status': 'Running', + 'audienceIds': [], + 'layerId': '111184', + 'variations': [{ + 'key': 'group_exp_2_control', + 'id': '28905' + }, { + 'key': 'group_exp_2_variation', + 'id': '28906' + }], + 'forcedVariations': { + 'user_1': 'group_exp_2_control', + 'user_2': 'group_exp_2_control' + }, + 'trafficAllocation': [{ + 'entityId': '28905', + 'endOfRange': 8000 + }, { + 'entityId': '28906', + 'endOfRange': 10000 + }] + }], + 'trafficAllocation': [{ + 'entityId': '32222', + "endOfRange": 3000 + }, { + 'entityId': '32223', + 'endOfRange': 7500 + }] + }], + 'accountId': '12001', + 'attributes': [{ + 'key': 'test_attribute', + 'id': '111094' + }], + 'audiences': [{ + 'name': 'Test attribute users 1', + 'conditions': '["and", ["or", ["or", ' + '{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]', + 'id': '11154' + }, { + 'name': 'Test attribute users 2', + 'conditions': '["and", ["or", ["or", ' + '{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_2"}]]]', + 'id': '11159' + }], + 'projectId': '111001' + } + + config = getattr(self, config_dict) + self.optimizely = optimizely.Optimizely(json.dumps(config)) self.project_config = self.optimizely.config diff --git a/tests/test_event_builder.py b/tests/test_event_builder.py index 4a74929a..b9a748b5 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -42,7 +42,7 @@ def test_init(self): class EventBuilderTest(base.BaseTest): def setUp(self): - base.BaseTest.setUp(self) + base.BaseTest.setUp(self, 'config_dict_with_multiple_experiments') self.event_builder = self.optimizely.event_builder def _validate_event_object(self, event_obj, expected_url, expected_params, expected_verb, expected_headers): @@ -655,3 +655,64 @@ def test_create_conversion_event__with_invalid_event_tags(self): expected_params, event_builder.EventBuilder.HTTP_VERB, event_builder.EventBuilder.HTTP_HEADERS) + + def test_create_conversion_event__when_event_is_used_in_multiple_experiments(self): + """ Test that create_conversion_event creates Event object with + right params when multiple experiments use the same event. """ + + expected_params = { + 'client_version': version.__version__, + 'project_id': '111001', + 'visitors': [{ + 'attributes': [{ + 'entity_id': '111094', + 'type': 'custom', + 'value': 'test_value', + 'key': 'test_attribute' + }], + 'visitor_id': 'test_user', + 'snapshots': [{ + 'decisions': [{ + 'variation_id': '111129', + 'experiment_id': '111127', + 'campaign_id': '111182' + }, { + 'experiment_id': '111130', + 'variation_id': '111131', + 'campaign_id': '111182' + }], + 'events': [{ + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + 'tags': { + 'non-revenue': 'abc', + 'revenue': 4200, + 'value': 1.234 + }, + 'timestamp': 42123, + 'revenue': 4200, + 'value': 1.234, + 'key': 'test_event', + 'entity_id': '111095' + }] + }] + }], + 'account_id': '12001', + 'client_name': 'python-sdk', + 'anonymize_ip': False, + 'revision': '42' + } + + with mock.patch('time.time', return_value=42.123), \ + mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'): + event_obj = self.event_builder.create_conversion_event( + 'test_event', + 'test_user', + {'test_attribute': 'test_value'}, + {'revenue': 4200, 'value': 1.234, 'non-revenue': 'abc'}, + [('111127', '111129'), ('111130', '111131')] + ) + self._validate_event_object(event_obj, + event_builder.EventBuilder.EVENTS_URL, + expected_params, + event_builder.EventBuilder.HTTP_VERB, + event_builder.EventBuilder.HTTP_HEADERS) From 90e172d2a9704bf6c6ce1f2a03e1d3ed0343d9ab Mon Sep 17 00:00:00 2001 From: Ali Abbas Rizvi Date: Tue, 21 Aug 2018 13:58:30 -0700 Subject: [PATCH 2/2] Updating version for 2.1.1 release (#137) --- CHANGELOG.md | 5 +++++ optimizely/version.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 500c15cc..2d37192e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## 2.1.1 +August 21st, 2018 + +- Fix: record conversions for all experiments using an event when using track([#136](https://github.com/optimizely/python-sdk/pull/136)). + ## 2.1.0 July 2nd, 2018 diff --git a/optimizely/version.py b/optimizely/version.py index e91c0145..92bf4020 100644 --- a/optimizely/version.py +++ b/optimizely/version.py @@ -11,5 +11,5 @@ # See the License for the specific language governing permissions and # limitations under the License. -version_info = (2, 1, 0) +version_info = (2, 1, 1) __version__ = '.'.join(str(v) for v in version_info)