Merge branch 'master' into oakbani/return-default-value-when-featuree…

…nabled-false
optimizely · Apr 1, 2019 · 3aedc69 · 3aedc69
2 parents f22cc9d + 92490ee
commit 3aedc69
Show file tree

Hide file tree

Showing 4 changed files with 147 additions and 27 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -14,10 +14,13 @@ New Features:
       *unconditionally*, without first determining whether the user is
       targeted by a known experiment that uses the event. This may
       increase outbound network traffic.
-   -  In Optimizely results, conversion events sent by 3.0 SDKs are
-      automatically attributed to variations that the user has
-      previously seen, as long as our backend has actually received the
-      impression events for those variations.
+   -  In Optimizely results, conversion events sent by 3.0 SDKs don't
+      explicitly name the experiments and variations that are currently
+      targeted to the user. Instead, conversions are automatically
+      attributed to variations that the user has previously seen, as long
+      as those variations were served via 3.0 SDKs or by other clients
+      capable of automatic attribution, and as long as our backend
+      actually received the impression events for those variations.
    -  Altogether, this allows you to track conversion events and
       attribute them to variations even when you don’t know all of a
       user’s attribute values, and even if the user’s attribute values
@@ -70,6 +73,10 @@ New Features:
 Breaking Changes:
 ~~~~~~~~~~~~~~~~~
 
+-  Conversion events sent by 3.0 SDKs don't explicitly name the experiments
+   and variations that are currently targeted to the user, so these events
+   are unattributed in raw events data export. You must use the new *results*
+   export to determine the variations to which events have been attributed.
 -  Previously, notification listeners were only given string-valued user
    attributes because only strings could be passed into various method
    calls. That is no longer the case. You may pass non-string attribute

diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py
@@ -89,6 +89,13 @@ class NotificationTypes(object):
       Experiment experiment, str user_id, dict attributes (can be None), Variation variation, Event event
       TRACK notification listener has the following parameters:
       str event_key, str user_id, dict attributes (can be None), event_tags (can be None), Event event
+      DECISION notification listener has the following parameters:
+      DecisionInfoTypes type, str user_id, dict attributes (can be None), dict decision_info
   """
   ACTIVATE = "ACTIVATE:experiment, user_id, attributes, variation, event"
   TRACK = "TRACK:event_key, user_id, attributes, event_tags, event"
+  DECISION = "DECISON:type, user_id, attributes, decision_info"
+
+
+class DecisionInfoTypes(object):
+  EXPERIMENT = "experiment"
diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py
@@ -349,6 +349,7 @@ def get_variation(self, experiment_key, user_id, attributes=None):
       return None
 
     experiment = self.config.get_experiment_from_key(experiment_key)
+    variation_key = None
 
     if not experiment:
       self.logger.info('Experiment key "%s" is invalid. Not activating user "%s".' % (
@@ -362,9 +363,20 @@ def get_variation(self, experiment_key, user_id, attributes=None):
 
     variation = self.decision_service.get_variation(experiment, user_id, attributes)
     if variation:
-      return variation.key
-
-    return None
+      variation_key = variation.key
+
+    self.notification_center.send_notifications(
+      enums.NotificationTypes.DECISION,
+      enums.DecisionInfoTypes.EXPERIMENT,
+      user_id,
+      attributes or {},
+      {
+         'experiment_key': experiment_key,
+         'variation_key': variation_key
+      }
+    )
+
+    return variation_key
 
   def is_feature_enabled(self, feature_key, user_id, attributes=None):
     """ Returns true if the feature is enabled for the given user.

diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py
@@ -398,42 +398,92 @@ def test_remove_listener(self):
     self.assertEqual(0, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.TRACK]))
     self.assertEqual(0, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.ACTIVATE]))
 
-  def test_activate_listener(self):
-    """ Test that activate calls broadcast activate with proper parameters. """
+  def test_activate_and_decision_listener(self):
+    """ Test that activate calls broadcast activate and decision with proper parameters. """
 
     with mock.patch(
         'optimizely.decision_service.DecisionService.get_variation',
         return_value=self.project_config.get_variation_from_id('test_experiment', '111129')), \
       mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch, \
-      mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast_activate:
+      mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast:
       self.assertEqual('variation', self.optimizely.activate('test_experiment', 'test_user'))
 
-    mock_broadcast_activate.assert_called_once_with(enums.NotificationTypes.ACTIVATE,
-                                                    self.project_config.get_experiment_from_key('test_experiment'),
-                                                    'test_user', None,
-                                                    self.project_config.get_variation_from_id('test_experiment',
-                                                                                              '111129'),
-                                                    mock_dispatch.call_args[0][0])
+    self.assertEqual(mock_broadcast.call_count, 2)
 
-  def test_activate_listener_with_attr(self):
-    """ Test that activate calls broadcast activate with proper parameters. """
+    mock_broadcast.assert_has_calls([
+      mock.call(
+        enums.NotificationTypes.DECISION,
+        'experiment',
+        'test_user',
+        {},
+        {
+          'experiment_key': 'test_experiment',
+          'variation_key': 'variation'
+        }
+      ),
+      mock.call(
+        enums.NotificationTypes.ACTIVATE,
+        self.project_config.get_experiment_from_key('test_experiment'),
+        'test_user', None,
+        self.project_config.get_variation_from_id('test_experiment', '111129'),
+        mock_dispatch.call_args[0][0]
+      )
+    ])
+
+  def test_activate_and_decision_listener_with_attr(self):
+    """ Test that activate calls broadcast activate and decision with proper parameters. """
 
     with mock.patch(
         'optimizely.decision_service.DecisionService.get_variation',
         return_value=self.project_config.get_variation_from_id('test_experiment', '111129')), \
       mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch, \
-      mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast_activate:
+      mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast:
       self.assertEqual('variation',
                        self.optimizely.activate('test_experiment', 'test_user', {'test_attribute': 'test_value'}))
 
-    mock_broadcast_activate.assert_called_once_with(enums.NotificationTypes.ACTIVATE,
-                                                    self.project_config.get_experiment_from_key('test_experiment'),
-                                                    'test_user', {'test_attribute': 'test_value'},
-                                                    self.project_config.get_variation_from_id(
-                                                      'test_experiment', '111129'
-                                                    ),
-                                                    mock_dispatch.call_args[0][0]
-                                                    )
+    self.assertEqual(mock_broadcast.call_count, 2)
+
+    mock_broadcast.assert_has_calls([
+      mock.call(
+        enums.NotificationTypes.DECISION,
+        'experiment',
+        'test_user',
+        {'test_attribute': 'test_value'},
+        {
+          'experiment_key': 'test_experiment',
+          'variation_key': 'variation'
+        }
+      ),
+      mock.call(
+        enums.NotificationTypes.ACTIVATE,
+        self.project_config.get_experiment_from_key('test_experiment'),
+        'test_user', {'test_attribute': 'test_value'},
+        self.project_config.get_variation_from_id('test_experiment', '111129'),
+        mock_dispatch.call_args[0][0]
+      )
+    ])
+
+  def test_decision_listener__user_not_in_experiment(self):
+    """ Test that activate calls broadcast decision with variation_key 'None' \
+    when user not in experiment. """
+
+    with mock.patch(
+        'optimizely.decision_service.DecisionService.get_variation',
+        return_value=None), \
+      mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event'), \
+      mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast_decision:
+      self.assertEqual(None, self.optimizely.activate('test_experiment', 'test_user'))
+
+    mock_broadcast_decision.assert_called_once_with(
+      enums.NotificationTypes.DECISION,
+      'experiment',
+      'test_user',
+      {},
+      {
+        'experiment_key': 'test_experiment',
+        'variation_key': None
+      }
+     )
 
   def test_track_listener(self):
     """ Test that track calls notification broadcaster. """
@@ -1395,6 +1445,50 @@ def test_track__invalid_user_id(self):
       self.assertIsNone(self.optimizely.track('test_event', 99))
     mock_client_logging.error.assert_called_once_with('Provided "user_id" is in an invalid format.')
 
+  def test_get_variation(self):
+    """ Test that get_variation returns valid variation and broadcasts decision with proper parameters. """
+
+    with mock.patch(
+        'optimizely.decision_service.DecisionService.get_variation',
+        return_value=self.project_config.get_variation_from_id('test_experiment', '111129')), \
+      mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast:
+      self.assertEqual('variation', self.optimizely.get_variation('test_experiment', 'test_user'))
+
+    self.assertEqual(mock_broadcast.call_count, 1)
+
+    mock_broadcast.assert_called_once_with(
+      enums.NotificationTypes.DECISION,
+      'experiment',
+      'test_user',
+      {},
+      {
+        'experiment_key': 'test_experiment',
+        'variation_key': 'variation'
+      }
+    )
+
+  def test_get_variation__returns_none(self):
+    """ Test that get_variation returns no variation and broadcasts decision with proper parameters. """
+
+    with mock.patch(
+        'optimizely.decision_service.DecisionService.get_variation', return_value=None), \
+      mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast:
+      self.assertEqual(None, self.optimizely.get_variation('test_experiment', 'test_user',
+                                                            attributes={'test_attribute': 'test_value'}))
+
+    self.assertEqual(mock_broadcast.call_count, 1)
+
+    mock_broadcast.assert_called_once_with(
+      enums.NotificationTypes.DECISION,
+      'experiment',
+      'test_user',
+      {'test_attribute': 'test_value'},
+      {
+        'experiment_key': 'test_experiment',
+        'variation_key': None
+      }
+    )
+
   def test_get_variation__invalid_object(self):
     """ Test that get_variation logs error if Optimizely object is not created correctly. """