From d4abf1f1497412ef35b14af150d34f0c3ccd9e2a Mon Sep 17 00:00:00 2001 From: Jonathan Burgess Date: Tue, 21 Aug 2018 13:03:28 +0100 Subject: [PATCH 1/5] event params will include event if in multiple experiments. --- optimizely/event_builder.py | 16 ++-- tests/base.py | 154 +++++++++++++++++++++++++++++++++++- tests/test_event_builder.py | 81 ++++++++++++++++++- 3 files changed, 239 insertions(+), 12 deletions(-) diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index 087dc1bf..f349857d 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -243,17 +243,18 @@ 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 = {} 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.setdefault(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, @@ -273,10 +274,9 @@ def _get_required_params_for_conversion(self, event_key, event_tags, decisions): if len(event_tags) > 0: event_dict[self.EventParams.TAGS] = event_tags + snapshot.setdefault(self.EventParams.EVENTS, []).append(event_dict) - snapshot[self.EventParams.EVENTS] = [event_dict] - - return snapshot + 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 +319,7 @@ 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) - + print(params) return Event(self.EVENTS_URL, params, http_verb=self.HTTP_VERB, diff --git a/tests/base.py b/tests/base.py index 72b78c7a..333531f4 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', @@ -374,6 +374,154 @@ def setUp(self): 'variables': [], }] } - - 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..663f9e30 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,82 @@ 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 event tags are provided. """ + + # base.BaseTest.setUp(self) + # self.event_builder = self.optimizely.event_builder + + 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' + }, + { + '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 fb242355a62c04c3491b9b0452a441b5cc1d8f6e Mon Sep 17 00:00:00 2001 From: Jonathan Burgess Date: Tue, 21 Aug 2018 13:15:41 +0100 Subject: [PATCH 2/5] pep8 --- tests/base.py | 7 +++---- tests/test_event_builder.py | 10 ++++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/tests/base.py b/tests/base.py index 333531f4..05be7935 100644 --- a/tests/base.py +++ b/tests/base.py @@ -374,7 +374,7 @@ def setUp(self, config_dict='config_dict'): 'variables': [], }] } - + self.config_dict_with_multiple_experiments = { 'revision': '42', 'version': '2', @@ -414,8 +414,7 @@ def setUp(self, config_dict='config_dict'): 'key': 'variation', 'id': '111129' }] - }, - { + }, { 'key': 'test_experiment_2', 'status': 'Running', 'forcedVariations': { @@ -521,7 +520,7 @@ def setUp(self, config_dict='config_dict'): }], '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 663f9e30..38033629 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -679,10 +679,9 @@ def test_create_conversion_event__when_event_is_used_in_multiple_experiments(sel 'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182' - }, - { - 'experiment_id': '111130', - 'variation_id': '111131', + }, { + 'experiment_id': '111130', + 'variation_id': '111131', 'campaign_id': '111182' } ], @@ -698,8 +697,7 @@ def test_create_conversion_event__when_event_is_used_in_multiple_experiments(sel 'value': 1.234, 'key': 'test_event', 'entity_id': '111095' - }, - { + }, { 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', 'tags': { 'non-revenue': 'abc', From f432988a959518609971fc7d8479852d628b6e06 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 21 Aug 2018 10:10:46 -0700 Subject: [PATCH 3/5] Updating event format --- optimizely/event_builder.py | 34 ++++++++++++++++------------------ tests/test_event_builder.py | 18 +----------------- 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index f349857d..7a40cf5b 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -249,33 +249,32 @@ def _get_required_params_for_conversion(self, event_key, event_tags, decisions): experiment = self.config.get_experiment_from_id(experiment_id) if variation_id: - snapshot.setdefault(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()) - } + 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 + 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 - 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 + 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 len(event_tags) > 0: - event_dict[self.EventParams.TAGS] = event_tags - snapshot.setdefault(self.EventParams.EVENTS, []).append(event_dict) + if len(event_tags) > 0: + event_dict[self.EventParams.TAGS] = event_tags + snapshot.setdefault(self.EventParams.EVENTS, []).append(event_dict) return snapshot def create_impression_event(self, experiment, variation_id, user_id, attributes): @@ -319,7 +318,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) - print(params) return Event(self.EVENTS_URL, params, http_verb=self.HTTP_VERB, diff --git a/tests/test_event_builder.py b/tests/test_event_builder.py index 38033629..0650a0fe 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -660,9 +660,6 @@ def test_create_conversion_event__when_event_is_used_in_multiple_experiments(sel """ Test that create_conversion_event creates Event object with right params when event tags are provided. """ - # base.BaseTest.setUp(self) - # self.event_builder = self.optimizely.event_builder - expected_params = { 'client_version': version.__version__, 'project_id': '111001', @@ -683,8 +680,7 @@ def test_create_conversion_event__when_event_is_used_in_multiple_experiments(sel 'experiment_id': '111130', 'variation_id': '111131', 'campaign_id': '111182' - } - ], + }], 'events': [{ 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', 'tags': { @@ -697,18 +693,6 @@ def test_create_conversion_event__when_event_is_used_in_multiple_experiments(sel 'value': 1.234, 'key': 'test_event', 'entity_id': '111095' - }, { - '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' }] }] }], From f1157b0368868b9eae0d8a433c7886ac96fed346 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 21 Aug 2018 10:22:58 -0700 Subject: [PATCH 4/5] Updating comment --- tests/test_event_builder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_event_builder.py b/tests/test_event_builder.py index 0650a0fe..b9a748b5 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -657,8 +657,8 @@ def test_create_conversion_event__with_invalid_event_tags(self): 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 event tags are provided. """ + """ Test that create_conversion_event creates Event object with + right params when multiple experiments use the same event. """ expected_params = { 'client_version': version.__version__, From 00c1ba9a736fbbe369f9e3feae88d4426c9d3a93 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 21 Aug 2018 11:50:44 -0700 Subject: [PATCH 5/5] Addressing feedback --- optimizely/event_builder.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index 7a40cf5b..47d95106 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -244,12 +244,14 @@ def _get_required_params_for_conversion(self, event_key, event_tags, decisions): Dict consisting of the decisions and events info for conversion event. """ snapshot = {} + snapshot[self.EventParams.DECISIONS] = [] + for experiment_id, variation_id in decisions: experiment = self.config.get_experiment_from_id(experiment_id) if variation_id: - snapshot.setdefault(self.EventParams.DECISIONS, []).append({ + snapshot[self.EventParams.DECISIONS].append({ self.EventParams.EXPERIMENT_ID: experiment_id, self.EventParams.VARIATION_ID: variation_id, self.EventParams.CAMPAIGN_ID: experiment.layerId @@ -274,7 +276,7 @@ def _get_required_params_for_conversion(self, event_key, event_tags, decisions): if len(event_tags) > 0: event_dict[self.EventParams.TAGS] = event_tags - snapshot.setdefault(self.EventParams.EVENTS, []).append(event_dict) + snapshot[self.EventParams.EVENTS] = [event_dict] return snapshot def create_impression_event(self, experiment, variation_id, user_id, attributes):