Merge pull request #19353 from edx/brian/DE-1121-email-context

Add page context to edx.bi.email.sent events

lms/djangoapps/discussion/tasks.py

@@ -5,7 +5,6 @@ pertaining to new discussion forum comments.
 import logging
 from urlparse import urljoin
 
-import analytics
 from celery import task
 from django.conf import settings
 from django.contrib.auth.models import User
@@ -16,6 +15,7 @@ from django_comment_common.models import DiscussionsIdMapping
 from edx_ace import ace
 from edx_ace.utils import date
 from edx_ace.recipient import Recipient
+from eventtracking import tracker
 from opaque_keys.edx.keys import CourseKey
 from lms.djangoapps.django_comment_client.utils import permalink, get_accessible_discussion_xblocks_by_course_id
 import lms.lib.comment_client as cc
@@ -24,6 +24,7 @@ from openedx.core.djangoapps.content.course_overviews.models import CourseOvervi
 from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
 from openedx.core.djangoapps.ace_common.message import BaseMessageType
 from openedx.core.lib.celery.task_utils import emulate_http_request
+from track import segment
 
 
 log = logging.getLogger(__name__)
@@ -85,15 +86,23 @@ def _track_notification_sent(message, context):
         'uuid': unicode(message.uuid),
         'send_uuid': unicode(message.send_uuid),
         'thread_id': context['thread_id'],
+        'course_id': unicode(context['course_id']),
         'thread_created_at': date.deserialize(context['thread_created_at']),
         'nonInteraction': 1,
     }
-    analytics.track(
-        user_id=context['thread_author_id'],
-        event='edx.bi.email.sent',
-        course_id=context['course_id'],
-        properties=properties
-    )
+    tracking_context = {
+        'host': context['site'].domain,
+        'path': '/',  # make up a value, in order to allow the host to be passed along.
+    }
+    # The event used to specify the user_id as being the recipient of the email (i.e. the thread_author_id).
+    # This has the effect of interrupting the actual chain of events for that author, if any, while the
+    # email-sent event should really be associated with the sender, since that is what triggers the event.
+    with tracker.get_tracker().context(properties['app_label'], tracking_context):
+        segment.track(
+            user_id=context['thread_author_id'],
+            event_name='edx.bi.email.sent',
+            properties=properties
+        )
 
 
 def _should_send_message(context):

lms/djangoapps/discussion/tests/test_tasks.py

@@ -288,6 +288,7 @@ class TaskTestCase(ModuleStoreTestCase):
             'uuid': 'uuid1',
             'send_uuid': 'uuid2',
             'thread_id': 'dummy_discussion_id',
+            'course_id': 'fake_course_edx',
             'thread_created_at': datetime(2000, 1, 1, 0, 0, 0)
         }
     ), (
@@ -305,6 +306,7 @@ class TaskTestCase(ModuleStoreTestCase):
             'uuid': 'uuid3',
             'send_uuid': 'uuid4',
             'thread_id': 'dummy_discussion_id2',
+            'course_id': 'fake_course_edx2',
             'thread_created_at': datetime(2000, 1, 1, 0, 0, 0)
         }
 
@@ -318,12 +320,13 @@ class TaskTestCase(ModuleStoreTestCase):
                 setattr(message, key, entry)
 
             test_props['nonInteraction'] = True
-
-            with mock.patch('analytics.track') as mock_analytics_track:
+            # Also augment context with site object, for setting segment context.
+            site = Site.objects.get_current()
+            context['site'] = site
+            with mock.patch('lms.djangoapps.discussion.tasks.segment.track') as mock_segment_track:
                 _track_notification_sent(message, context)
-                mock_analytics_track.assert_called_once_with(
+                mock_segment_track.assert_called_once_with(
                     user_id=context['thread_author_id'],
-                    event='edx.bi.email.sent',
-                    course_id=context['course_id'],
-                    properties=test_props
+                    event_name='edx.bi.email.sent',
+                    properties=test_props,
                 )

openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py

@@ -415,10 +415,10 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin):
             self.assertEqual(len(sent_messages), num_expected_messages)
 
             with self.assertNumQueries(NUM_QUERIES_PER_MESSAGE_DELIVERY):
-                with patch('analytics.track') as mock_analytics_track:
+                with patch('openedx.core.djangoapps.schedules.tasks.segment.track') as mock_segment_track:
                     with patch('edx_ace.channel.channels', return_value=channel_map):
                         self.deliver_task(*sent_messages[0])
-                        self.assertEqual(mock_analytics_track.call_count, 1)
+                        self.assertEqual(mock_segment_track.call_count, 1)
 
             self.assertEqual(mock_channel.deliver.call_count, 1)
             for (_name, (_msg, email), _kwargs) in mock_channel.deliver.mock_calls:

openedx/core/djangoapps/schedules/tasks.py

@@ -1,7 +1,6 @@
 import datetime
 import logging
 
-import analytics
 from celery import task
 from django.conf import settings
 from django.contrib.auth.models import User
@@ -16,12 +15,15 @@ from edx_ace import ace
 from edx_ace.message import Message
 from edx_ace.utils.date import deserialize, serialize
 from edx_django_utils.monitoring import set_custom_metric
+from eventtracking import tracker
 from opaque_keys.edx.keys import CourseKey
 
 from openedx.core.djangoapps.schedules import message_types
 from openedx.core.djangoapps.schedules.models import Schedule, ScheduleConfig
 from openedx.core.djangoapps.schedules import resolvers
 from openedx.core.lib.celery.task_utils import emulate_http_request
+from track import segment
+
 
 LOG = logging.getLogger(__name__)
 
@@ -224,11 +226,24 @@ def _track_message_sent(site, user, msg):
         properties['course_ids'] = course_ids[:10]
         properties['primary_course_id'] = course_ids[0]
 
-    analytics.track(
-        user_id=user.id,
-        event='edx.bi.email.sent',
-        properties=properties
-    )
+    tracking_context = {
+        'host': site.domain,
+        'path': '/',  # make up a value, in order to allow the host to be passed along.
+    }
+    # I wonder if the user of this event should be the recipient, as they are not the ones
+    # who took an action.  Rather, the system is acting, and they are the object.
+    # Admittedly that may be what 'nonInteraction' is meant to address.  But sessionization may
+    # get confused by these events if they're attributed in this way, because there's no way for
+    # this event to get context that would match with what the user might be doing at the moment.
+    # But the events do show up in GA being joined up with existing sessions (i.e. within a half
+    # hour in the past), so they don't always break sessions.  Not sure what happens after these.
+    # We can put the recipient_user_id into the properties, and then export as a custom dimension.
+    with tracker.get_tracker().context(msg.app_label, tracking_context):
+        segment.track(
+            user_id=user.id,
+            event_name='edx.bi.email.sent',
+            properties=properties,
+        )
 
 
 def _is_delivery_enabled(site, delivery_config_var, log_prefix):