Revert "Preventing events from being sent when user is part of a rollout (#85)"

This reverts commit c2a8349.

Author: aliabbasrizvi

optimizely/decision_service.py

@@ -22,9 +22,7 @@
 from .user_profile import UserProfile
 
 
-Decision = namedtuple('Decision', 'experiment variation source')
-DECISION_SOURCE_EXPERIMENT = 'experiment'
-DECISION_SOURCE_ROLLOUT = 'rollout'
+Decision = namedtuple('Decision', 'experiment variation')
 RESERVED_BUCKETING_ID_ATTRIBUTE = '$opt_bucketing_id'
 
 
@@ -212,7 +210,7 @@ def get_variation_for_rollout(self, rollout, user_id, attributes=None):
         if variation:
           self.logger.log(enums.LogLevels.DEBUG,
                           'User "%s" is in variation %s of experiment %s.' % (user_id, variation.key, experiment.key))
-          return Decision(experiment, variation, DECISION_SOURCE_ROLLOUT)
+          return Decision(experiment, variation)
         else:
           # Evaluate no further rules
           self.logger.log(enums.LogLevels.DEBUG,
@@ -231,9 +229,9 @@ def get_variation_for_rollout(self, rollout, user_id, attributes=None):
         if variation:
           self.logger.log(enums.LogLevels.DEBUG,
                           'User "%s" meets conditions for targeting rule "Everyone Else".' % user_id)
-          return Decision(everyone_else_experiment, variation, DECISION_SOURCE_ROLLOUT)
+          return Decision(everyone_else_experiment, variation)
 
-    return Decision(None, None, DECISION_SOURCE_ROLLOUT)
+    return Decision(None, None)
 
   def get_experiment_in_group(self, group, bucketing_id):
     """ Determine which experiment in the group the user is bucketed into.
@@ -307,4 +305,4 @@ def get_variation_for_feature(self, feature, user_id, attributes=None):
       rollout = self.config.get_rollout_from_id(feature.rolloutId)
       return self.get_variation_for_rollout(rollout, user_id, attributes)
 
-    return Decision(experiment, variation, DECISION_SOURCE_EXPERIMENT)
+    return Decision(experiment, variation)

optimizely/optimizely.py

@@ -382,13 +382,10 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None):
     decision = self.decision_service.get_variation_for_feature(feature, user_id, attributes)
     if decision.variation:
       self.logger.log(enums.LogLevels.INFO, 'Feature "%s" is enabled for user "%s".' % (feature_key, user_id))
-      # Send event if Decision came from an experiment.
-      if decision.source == decision_service.DECISION_SOURCE_EXPERIMENT:
-        self._send_impression_event(decision.experiment,
-                                    decision.variation,
-                                    user_id,
-                                    attributes)
-
+      self._send_impression_event(decision.experiment,
+                                  decision.variation,
+                                  user_id,
+                                  attributes)
       return True
 
     self.logger.log(enums.LogLevels.INFO, 'Feature "%s" is not enabled for user "%s".' % (feature_key, user_id))

tests/test_decision_service.py

@@ -377,7 +377,7 @@ def test_get_variation_for_rollout__returns_none_if_no_experiments(self, mock_lo
     """ Test that get_variation_for_rollout returns None if there are no experiments (targeting rules). """
 
     no_experiment_rollout = self.project_config.get_rollout_from_id('201111')
-    self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_ROLLOUT),
+    self.assertEqual(decision_service.Decision(None, None),
                      self.decision_service.get_variation_for_rollout(no_experiment_rollout, 'test_user'))
 
     # Assert no log messages were generated
@@ -393,8 +393,7 @@ def test_get_variation_for_rollout__returns_decision_if_user_in_rollout(self, mo
       mock.patch('optimizely.bucketer.Bucketer.bucket',
                  return_value=self.project_config.get_variation_from_id('211127', '211129')) as mock_bucket:
       self.assertEqual(decision_service.Decision(self.project_config.get_experiment_from_id('211127'),
-                                                 self.project_config.get_variation_from_id('211127', '211129'),
-                                                 decision_service.DECISION_SOURCE_ROLLOUT),
+                                                 self.project_config.get_variation_from_id('211127', '211129')),
                        self.decision_service.get_variation_for_rollout(rollout, 'test_user'))
 
     # Check all log messages
@@ -415,8 +414,7 @@ def test_get_variation_for_rollout__calls_bucket_with_bucketing_id(self, mock_lo
       mock.patch('optimizely.bucketer.Bucketer.bucket',
                  return_value=self.project_config.get_variation_from_id('211127', '211129')) as mock_bucket:
       self.assertEqual(decision_service.Decision(self.project_config.get_experiment_from_id('211127'),
-                                                 self.project_config.get_variation_from_id('211127', '211129'),
-                                                 decision_service.DECISION_SOURCE_ROLLOUT),
+                                                 self.project_config.get_variation_from_id('211127', '211129')),
                        self.decision_service.get_variation_for_rollout(rollout,
                                                                        'test_user',
                                                                        {'$opt_bucketing_id': 'user_bucket_value'}))
@@ -440,7 +438,7 @@ def test_get_variation_for_rollout__skips_to_everyone_else_rule(self, mock_loggi
 
     with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True) as mock_audience_check,\
       mock.patch('optimizely.bucketer.Bucketer.bucket', return_value=None):
-      self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_ROLLOUT),
+      self.assertEqual(decision_service.Decision(None, None),
                        self.decision_service.get_variation_for_rollout(rollout, 'test_user'))
 
     # Check that after first experiment, it skips to the last experiment to check
@@ -463,7 +461,7 @@ def test_get_variation_for_rollout__returns_none_for_user_not_in_rollout(self, m
     rollout = self.project_config.get_rollout_from_id('211111')
 
     with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=False) as mock_audience_check:
-      self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_ROLLOUT),
+      self.assertEqual(decision_service.Decision(None, None),
                        self.decision_service.get_variation_for_rollout(rollout, 'test_user'))
 
     # Check that all experiments in rollout layer were checked
@@ -489,10 +487,8 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_experiment(
     expected_variation = self.project_config.get_variation_from_id('test_experiment', '111129')
     with mock.patch('optimizely.decision_service.DecisionService.get_variation',
                     return_value=expected_variation) as mock_decision:
-      self.assertEqual(decision_service.Decision(expected_experiment,
-                                                 expected_variation,
-                                                 decision_service.DECISION_SOURCE_EXPERIMENT),
-                       self.decision_service.get_variation_for_feature(feature, 'test_user'))
+      self.assertEqual(decision_service.Decision(expected_experiment, expected_variation),
+                       self.decision_service.get_variation_for_feature(feature, 'user1'))
 
     mock_decision.assert_called_once_with(
       self.project_config.get_experiment_from_key('test_experiment'), 'test_user', None
@@ -531,10 +527,8 @@ def test_get_variation_for_feature__returns_variation_if_user_not_in_experiment_
       'optimizely.helpers.audience.is_user_in_experiment',
       side_effect=[False, True]) as mock_audience_check, \
       mock.patch('optimizely.bucketer.Bucketer.bucket', return_value=expected_variation):
-      self.assertEqual(decision_service.Decision(expected_experiment,
-                                                 expected_variation,
-                                                 decision_service.DECISION_SOURCE_ROLLOUT),
-                       self.decision_service.get_variation_for_feature(feature, 'test_user'))
+      self.assertEqual(decision_service.Decision(expected_experiment, expected_variation),
+                       self.decision_service.get_variation_for_feature(feature, 'user1'))
 
     self.assertEqual(2, mock_audience_check.call_count)
     mock_audience_check.assert_any_call(self.project_config,
@@ -555,10 +549,8 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_group(self,
       return_value=self.project_config.get_experiment_from_key('group_exp_1')) as mock_get_experiment_in_group, \
       mock.patch('optimizely.decision_service.DecisionService.get_variation',
                  return_value=expected_variation) as mock_decision:
-      self.assertEqual(decision_service.Decision(expected_experiment,
-                                                 expected_variation,
-                                                 decision_service.DECISION_SOURCE_EXPERIMENT),
-                       self.decision_service.get_variation_for_feature(feature, 'test_user'))
+      self.assertEqual(decision_service.Decision(expected_experiment, expected_variation),
+                       self.decision_service.get_variation_for_feature(feature, 'user1'))
 
     mock_get_experiment_in_group.assert_called_once_with(self.project_config.get_group('19228'), 'test_user')
     mock_decision.assert_called_once_with(self.project_config.get_experiment_from_key('group_exp_1'), 'test_user', None)
@@ -572,8 +564,8 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_group(self, _):
     with mock.patch('optimizely.decision_service.DecisionService.get_experiment_in_group',
                     return_value=None) as mock_get_experiment_in_group, \
       mock.patch('optimizely.decision_service.DecisionService.get_variation') as mock_decision:
-      self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_EXPERIMENT),
-                       self.decision_service.get_variation_for_feature(feature, 'test_user'))
+      self.assertEqual(decision_service.Decision(None, None),
+                       self.decision_service.get_variation_for_feature(feature, 'user1'))
 
     mock_get_experiment_in_group.assert_called_once_with(self.project_config.get_group('19228'), 'test_user')
     self.assertFalse(mock_decision.called)
@@ -585,10 +577,8 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self
     expected_experiment = self.project_config.get_experiment_from_key('test_experiment')
 
     with mock.patch('optimizely.decision_service.DecisionService.get_variation', return_value=None) as mock_decision:
-      self.assertEqual(decision_service.Decision(expected_experiment,
-                                                 None,
-                                                 decision_service.DECISION_SOURCE_EXPERIMENT),
-                       self.decision_service.get_variation_for_feature(feature, 'test_user'))
+      self.assertEqual(decision_service.Decision(expected_experiment, None),
+                       self.decision_service.get_variation_for_feature(feature, 'user1'))
 
     mock_decision.assert_called_once_with(
       self.project_config.get_experiment_from_key('test_experiment'), 'test_user', None
@@ -603,10 +593,8 @@ def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_no
 
     with mock.patch('optimizely.decision_service.DecisionService.get_experiment_in_group',
                     return_value=self.project_config.get_experiment_from_key('group_exp_2')) as mock_decision:
-      self.assertEqual(decision_service.Decision(expected_experiment,
-                                                 None,
-                                                 decision_service.DECISION_SOURCE_EXPERIMENT),
-                       self.decision_service.get_variation_for_feature(feature, 'test_user'))
+      self.assertEqual(decision_service.Decision(expected_experiment, None),
+                       self.decision_service.get_variation_for_feature(feature, 'user_1'))
 
     mock_decision.assert_called_once_with(self.project_config.get_group('19228'), 'test_user')