From 3382200f9414ed53055d4852fca87afcf0e1deca Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 19 Jun 2018 10:00:03 -0700 Subject: [PATCH 1/4] Sending event when variation of an experiment has feature disabled --- optimizely/optimizely.py | 10 +++++--- tests/test_optimizely.py | 54 +++++++++++++++++++++++++++++----------- 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index a229422e..0a19a6c7 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -388,8 +388,7 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): return False decision = self.decision_service.get_variation_for_feature(feature, user_id, attributes) - if decision.variation and decision.variation.featureEnabled: - self.logger.info('Feature "%s" is enabled for user "%s".' % (feature_key, user_id)) + if decision.variation: # Send event if Decision came from an experiment. if decision.source == decision_service.DECISION_SOURCE_EXPERIMENT: self._send_impression_event(decision.experiment, @@ -397,7 +396,12 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): user_id, attributes) - return True + if decision.variation.featureEnabled: + self.logger.info('Feature "%s" is enabled for user "%s".' % (feature_key, user_id)) + else: + self.logger.info('Feature "%s" is not enabled for user "%s".' % (feature_key, user_id)) + + return decision.variation.featureEnabled self.logger.info('Feature "%s" is not enabled for user "%s".' % (feature_key, user_id)) return False diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 32d41c20..f2e34d4d 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -1181,10 +1181,9 @@ def test_is_feature_enabled__returns_false_for_invalid_feature(self): # Check that no event is sent self.assertEqual(0, mock_dispatch_event.call_count) - def test_is_feature_enabled__returns_true_for_feature_experiment_if_property_featureEnabled_is_true(self): + def test_is_feature_enabled__returns_true_for_feature_experiment_if_feature_enabled_for_variation(self): """ Test that the feature is enabled for the user if bucketed into variation of an experiment and - the variation's featureEnabled property is True. - Also confirm that impression event is dispatched. """ + the variation's featureEnabled property is True. Also confirm that impression event is dispatched. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) project_config = opt_obj.config @@ -1240,10 +1239,9 @@ def test_is_feature_enabled__returns_true_for_feature_experiment_if_property_fea 'https://logx.optimizely.com/v1/events', expected_params, 'POST', {'Content-Type': 'application/json'}) - def test_is_feature_enabled__returns_false_for_feature_experiment_if_property_featureEnabled_is_false(self): + def test_is_feature_enabled__returns_false_for_feature_experiment_if_feature_disabled_for_variation(self): """ Test that the feature is disabled for the user if bucketed into variation of an experiment and - the variation's featureEnabled property is False. - Also confirm that impression event is not dispatched. """ + the variation's featureEnabled property is False. Also confirm that impression event is dispatched. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) project_config = opt_obj.config @@ -1268,13 +1266,42 @@ def test_is_feature_enabled__returns_false_for_feature_experiment_if_property_fe mock_decision.assert_called_once_with(feature, 'test_user', None) - # Check that impression event is not sent - self.assertEqual(0, mock_dispatch_event.call_count) + # Check that impression event is sent + expected_params = { + 'account_id': '12001', + 'project_id': '111111', + 'visitors': [{ + 'visitor_id': 'test_user', + 'attributes': [], + 'snapshots': [{ + 'decisions': [{ + 'variation_id': '111128', + 'experiment_id': '111127', + 'campaign_id': '111182' + }], + 'events': [{ + 'timestamp': 42000, + 'entity_id': '111182', + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + 'key': 'campaign_activated', + }] + }] + }], + 'client_version': version.__version__, + 'client_name': 'python-sdk', + 'anonymize_ip': False, + 'revision': '1' + } + # Check that impression event is sent + self.assertEqual(1, mock_dispatch_event.call_count) + self._validate_event_object(mock_dispatch_event.call_args[0][0], + 'https://logx.optimizely.com/v1/events', + expected_params, 'POST', {'Content-Type': 'application/json'}) + - def test_is_feature_enabled__returns_true_for_feature_rollout_if_property_featureEnabled_is_true(self): + def test_is_feature_enabled__returns_true_for_feature_rollout_if_feature_enabled(self): """ Test that the feature is enabled for the user if bucketed into variation of a rollout and - the variation's featureEnabled property is True. - Also confirm that no impression event is dispatched. """ + the variation's featureEnabled property is True. Also confirm that no impression event is dispatched. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) project_config = opt_obj.config @@ -1302,10 +1329,9 @@ def test_is_feature_enabled__returns_true_for_feature_rollout_if_property_featur # Check that impression event is not sent self.assertEqual(0, mock_dispatch_event.call_count) - def test_is_feature_enabled__returns_false_for_feature_rollout_if_property_featureEnabled_is_false(self): + def test_is_feature_enabled__returns_false_for_feature_rollout_if_feature_disabled(self): """ Test that the feature is disabled for the user if bucketed into variation of a rollout and - the variation's featureEnabled property is False. - Also confirm that no impression event is dispatched. """ + the variation's featureEnabled property is False. Also confirm that no impression event is dispatched. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) project_config = opt_obj.config From 9beb072d70c9c4b2f0bbfedd4975477f2c43e74f Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 19 Jun 2018 10:12:30 -0700 Subject: [PATCH 2/4] Lint fix --- tests/test_optimizely.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index f2e34d4d..651a964f 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -1297,8 +1297,7 @@ def test_is_feature_enabled__returns_false_for_feature_experiment_if_feature_dis self._validate_event_object(mock_dispatch_event.call_args[0][0], 'https://logx.optimizely.com/v1/events', expected_params, 'POST', {'Content-Type': 'application/json'}) - - += def test_is_feature_enabled__returns_true_for_feature_rollout_if_feature_enabled(self): """ Test that the feature is enabled for the user if bucketed into variation of a rollout and the variation's featureEnabled property is True. Also confirm that no impression event is dispatched. """ From 27fd7703de923e2d3314724c5bb9ad30cc9d789c Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 19 Jun 2018 10:12:47 -0700 Subject: [PATCH 3/4] nit --- tests/test_optimizely.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 651a964f..fcc57743 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -1297,7 +1297,7 @@ def test_is_feature_enabled__returns_false_for_feature_experiment_if_feature_dis self._validate_event_object(mock_dispatch_event.call_args[0][0], 'https://logx.optimizely.com/v1/events', expected_params, 'POST', {'Content-Type': 'application/json'}) -= + def test_is_feature_enabled__returns_true_for_feature_rollout_if_feature_enabled(self): """ Test that the feature is enabled for the user if bucketed into variation of a rollout and the variation's featureEnabled property is True. Also confirm that no impression event is dispatched. """ From 3edd029f7f16d7412068af62c5d9e0a7e85f3b49 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 19 Jun 2018 10:26:28 -0700 Subject: [PATCH 4/4] Small modification --- optimizely/optimizely.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 0a19a6c7..854dd78d 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -398,10 +398,10 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): if decision.variation.featureEnabled: self.logger.info('Feature "%s" is enabled for user "%s".' % (feature_key, user_id)) + return True else: self.logger.info('Feature "%s" is not enabled for user "%s".' % (feature_key, user_id)) - - return decision.variation.featureEnabled + return False self.logger.info('Feature "%s" is not enabled for user "%s".' % (feature_key, user_id)) return False