diff --git a/optimizely/event/user_event_factory.py b/optimizely/event/user_event_factory.py index 38217883..fb5c70ed 100644 --- a/optimizely/event/user_event_factory.py +++ b/optimizely/event/user_event_factory.py @@ -49,11 +49,13 @@ def create_impression_event( if activated_experiment: experiment_id = activated_experiment.id - if variation_id and experiment_id: - variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id) - # need this condition when we send events involving forced decisions - elif variation_id and flag_key: + if variation_id and flag_key: + # need this condition when we send events involving forced decisions + # (F-to-D or E-to-D with any ruleKey/variationKey combinations) variation = project_config.get_flag_variation(flag_key, 'id', variation_id) + elif variation_id and experiment_id: + variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id) + event_context = user_event.EventContext( project_config.account_id, project_config.project_id, project_config.revision, project_config.anonymize_ip, ) diff --git a/optimizely/project_config.py b/optimizely/project_config.py index 494df542..82da17c9 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -612,8 +612,8 @@ def get_variation_from_id_by_experiment_id(self, experiment_id, variation_id): variation_id in self.variation_id_map_by_experiment_id[experiment_id]): return self.variation_id_map_by_experiment_id[experiment_id][variation_id] - self.logger.error('Variation with id "%s" not defined in the datafile for experiment "%s".', - variation_id, experiment_id) + self.logger.error('Variation with id "%s" not defined in the datafile for experiment "%s".' % + (variation_id, experiment_id)) return {} @@ -628,8 +628,8 @@ def get_variation_from_key_by_experiment_id(self, experiment_id, variation_key): variation_key in self.variation_key_map_by_experiment_id[experiment_id]): return self.variation_key_map_by_experiment_id[experiment_id][variation_key] - self.logger.error('Variation with key "%s" not defined in the datafile for experiment "%s".', - variation_key, experiment_id) + self.logger.error('Variation with key "%s" not defined in the datafile for experiment "%s".' % + (variation_key, experiment_id)) return {} @@ -661,8 +661,9 @@ def get_flag_variation(self, flag_key, variation_attribute, target_value): return None variations = self.flag_variations_map.get(flag_key) - for variation in variations: - if getattr(variation, variation_attribute) == target_value: - return variation + if variations: + for variation in variations: + if getattr(variation, variation_attribute) == target_value: + return variation return None diff --git a/tests/test_event_factory.py b/tests/test_event_factory.py index 2e8a6192..ec92a3dd 100644 --- a/tests/test_event_factory.py +++ b/tests/test_event_factory.py @@ -75,7 +75,7 @@ def test_create_impression_event(self): { 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', - 'metadata': {'flag_key': 'flag_key', + 'metadata': {'flag_key': '', 'rule_key': 'rule_key', 'rule_type': 'experiment', 'variation_key': 'variation', @@ -107,7 +107,7 @@ def test_create_impression_event(self): self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', - 'flag_key', + '', 'rule_key', 'experiment', False, @@ -138,7 +138,7 @@ def test_create_impression_event__with_attributes(self): { 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', - 'metadata': {'flag_key': 'flag_key', + 'metadata': {'flag_key': '', 'rule_key': 'rule_key', 'rule_type': 'experiment', 'variation_key': 'variation', @@ -171,7 +171,7 @@ def test_create_impression_event__with_attributes(self): self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', - 'flag_key', + '', 'rule_key', 'experiment', True, @@ -200,7 +200,7 @@ def test_create_impression_event_when_attribute_is_not_in_datafile(self): { 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', - 'metadata': {'flag_key': 'flag_key', + 'metadata': {'flag_key': '', 'rule_key': 'rule_key', 'rule_type': 'experiment', 'variation_key': 'variation', @@ -233,7 +233,7 @@ def test_create_impression_event_when_attribute_is_not_in_datafile(self): self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', - 'flag_key', + '', 'rule_key', 'experiment', True, @@ -265,7 +265,7 @@ def test_create_impression_event_calls_is_attribute_valid(self): { 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', - 'metadata': {'flag_key': 'flag_key', + 'metadata': {'flag_key': '', 'flag_type': 'experiment', 'variation_key': 'variation'}, } @@ -313,7 +313,7 @@ def side_effect(*args, **kwargs): self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', - 'flag_key', + '', 'experiment', 'test_user', attributes, @@ -353,7 +353,7 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_enabled( { 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', - 'metadata': {'flag_key': 'flag_key', + 'metadata': {'flag_key': '', 'rule_key': 'rule_key', 'rule_type': 'experiment', 'variation_key': 'variation', @@ -388,7 +388,7 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_enabled( self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', - 'flag_key', + '', 'rule_key', 'experiment', False, @@ -425,7 +425,7 @@ def test_create_impression_event__with_empty_attributes_when_bot_filtering_is_en { 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', - 'metadata': {'flag_key': 'flag_key', + 'metadata': {'flag_key': '', 'rule_key': 'rule_key', 'rule_type': 'experiment', 'variation_key': 'variation', @@ -460,7 +460,7 @@ def test_create_impression_event__with_empty_attributes_when_bot_filtering_is_en self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', - 'flag_key', + '', 'rule_key', 'experiment', False, @@ -503,7 +503,7 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_disabled { 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', - 'metadata': {'flag_key': 'flag_key', + 'metadata': {'flag_key': '', 'rule_key': 'rule_key', 'rule_type': 'experiment', 'variation_key': 'variation', @@ -538,7 +538,7 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_disabled self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', - 'flag_key', + '', 'rule_key', 'experiment', True, diff --git a/tests/test_user_context.py b/tests/test_user_context.py index 4b88e87a..dc52c648 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -1521,6 +1521,31 @@ def test_should_return_valid_experiment_decision_after_setting_forced_decision(s self.assertEqual(decide_decision.reasons, expected_reasons) + def test_should_return_valid_decision_after_setting_variation_of_different_experiment_in_forced_decision(self): + """ + Should return valid decision after setting setting variation of different experiment in forced decision. + """ + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) + user_context = opt_obj.create_user_context("test_user", {}) + + context = OptimizelyUserContext.OptimizelyDecisionContext('test_feature_in_experiment_and_rollout', + 'group_exp_2') + decision = OptimizelyUserContext.OptimizelyForcedDecision('211129') + + status = user_context.set_forced_decision(context, decision) + self.assertTrue(status) + status = user_context.get_forced_decision(context) + self.assertEqual(status.variation_key, '211129') + + decide_decision = user_context.decide('test_feature_in_experiment_and_rollout', ['INCLUDE_REASONS']) + + self.assertEqual(decide_decision.variation_key, '211129') + self.assertEqual(decide_decision.rule_key, 'group_exp_2') + self.assertTrue(decide_decision.enabled) + self.assertEqual(decide_decision.flag_key, 'test_feature_in_experiment_and_rollout') + self.assertEqual(decide_decision.user_context.user_id, 'test_user') + self.assertEqual(decide_decision.user_context.get_user_attributes(), {}) + def test_should_return_valid_decision_after_setting_invalid_delivery_rule_variation_in_forced_decision(self): """ Should return valid decision after setting invalid delivery rule variation in forced decision. diff --git a/tests/test_user_event_factory.py b/tests/test_user_event_factory.py index e723a823..009ef05d 100644 --- a/tests/test_user_event_factory.py +++ b/tests/test_user_event_factory.py @@ -28,7 +28,7 @@ def test_impression_event(self): variation = self.project_config.get_variation_from_id(experiment.key, '111128') user_id = 'test_user' - impression_event = UserEventFactory.create_impression_event(project_config, experiment, '111128', 'flag_key', + impression_event = UserEventFactory.create_impression_event(project_config, experiment, '111128', '', 'rule_key', 'rule_type', True, user_id, None) self.assertEqual(self.project_config.project_id, impression_event.event_context.project_id) @@ -51,7 +51,7 @@ def test_impression_event__with_attributes(self): user_attributes = {'test_attribute': 'test_value', 'boolean_key': True} impression_event = UserEventFactory.create_impression_event( - project_config, experiment, '111128', 'flag_key', 'rule_key', 'rule_type', True, user_id, user_attributes + project_config, experiment, '111128', '', 'rule_key', 'rule_type', True, user_id, user_attributes ) expected_attrs = EventFactory.build_attribute_list(user_attributes, project_config)