Merge pull request #4566 from mulby/remove-duplicate-context-fields

Remove duplicated data from legacy events

common/djangoapps/track/shim.py

@@ -18,9 +18,9 @@ class LegacyFieldMappingProcessor(object):
             for field in CONTEXT_FIELDS_TO_INCLUDE:
                 if field in context:
                     event[field] = context[field]
-                    del context[field]
                 else:
                     event[field] = ''
+            remove_shim_context(event)
 
         if 'event_type' in event.get('context', {}):
             event['event_type'] = event['context']['event_type']
@@ -40,3 +40,11 @@ class LegacyFieldMappingProcessor(object):
 
         event['event_source'] = 'server'
         event['page'] = None
+
+
+def remove_shim_context(event):
+    if 'context' in event:
+        context = event['context']
+        for field in CONTEXT_FIELDS_TO_INCLUDE:
+            if field in context:
+                del context[field]

common/djangoapps/track/tests/test_views.py

 # pylint: disable=missing-docstring,maybe-no-member
 
 from track import views
+from track.middleware import TrackMiddleware
 from mock import patch, sentinel
 from freezegun import freeze_time
 
@@ -53,12 +54,46 @@ class TestTrackViews(TestCase):
             'context': {
                 'course_id': 'foo/bar/baz',
                 'org_id': 'foo',
-                'session': sentinel.session,
             },
         }
         self.mock_tracker.send.assert_called_once_with(expected_event)
 
     @freeze_time(expected_time)
+    def test_user_track_with_middleware(self):
+        middleware = TrackMiddleware()
+        request = self.request_factory.get('/event', {
+            'page': self.url_with_course,
+            'event_type': sentinel.event_type,
+            'event': {}
+        })
+        middleware.process_request(request)
+        try:
+            views.user_track(request)
+
+            expected_event = {
+                'username': 'anonymous',
+                'session': '',
+                'ip': '127.0.0.1',
+                'event_source': 'browser',
+                'event_type': str(sentinel.event_type),
+                'event': '{}',
+                'agent': '',
+                'page': self.url_with_course,
+                'time': expected_time,
+                'host': 'testserver',
+                'context': {
+                    'course_id': 'foo/bar/baz',
+                    'org_id': 'foo',
+                    'user_id': '',
+                    'path': u'/event'
+                },
+            }
+        finally:
+            middleware.process_response(request, None)
+
+        self.mock_tracker.send.assert_called_once_with(expected_event)
+
+    @freeze_time(expected_time)
     def test_server_track(self):
         request = self.request_factory.get(self.path_with_course)
         views.server_track(request, str(sentinel.event_type), '{}')
@@ -78,6 +113,38 @@ class TestTrackViews(TestCase):
         self.mock_tracker.send.assert_called_once_with(expected_event)
 
     @freeze_time(expected_time)
+    def test_server_track_with_middleware(self):
+        middleware = TrackMiddleware()
+        request = self.request_factory.get(self.path_with_course)
+        middleware.process_request(request)
+        # The middleware emits an event, reset the mock to ignore it since we aren't testing that feature.
+        self.mock_tracker.reset_mock()
+        try:
+            views.server_track(request, str(sentinel.event_type), '{}')
+
+            expected_event = {
+                'username': 'anonymous',
+                'ip': '127.0.0.1',
+                'event_source': 'server',
+                'event_type': str(sentinel.event_type),
+                'event': '{}',
+                'agent': '',
+                'page': None,
+                'time': expected_time,
+                'host': 'testserver',
+                'context': {
+                    'user_id': '',
+                    'course_id': u'foo/bar/baz',
+                    'org_id': 'foo',
+                    'path': u'/courses/foo/bar/baz/xmod/'
+                },
+            }
+        finally:
+            middleware.process_response(request, None)
+
+        self.mock_tracker.send.assert_called_once_with(expected_event)
+
+    @freeze_time(expected_time)
     def test_server_track_with_no_request(self):
         request = None
         views.server_track(request, str(sentinel.event_type), '{}')

common/djangoapps/track/views.py

@@ -12,6 +12,7 @@ from edxmako.shortcuts import render_to_response
 
 from track import tracker
 from track import contexts
+from track import shim
 from track.models import TrackingLog
 from eventtracking import tracker as eventtracker
 
@@ -59,6 +60,10 @@ def user_track(request):
             "context": context,
         }
 
+    # Some duplicated fields are passed into event-tracking via the context by track.middleware.
+    # Remove them from the event here since they are captured elsewhere.
+    shim.remove_shim_context(event)
+
     log_event(event)
 
     return HttpResponse('success')
@@ -92,6 +97,10 @@ def server_track(request, event_type, event, page=None):
         "context": eventtracker.get_tracker().resolve_context(),
     }
 
+    # Some duplicated fields are passed into event-tracking via the context by track.middleware.
+    # Remove them from the event here since they are captured elsewhere.
+    shim.remove_shim_context(event)
+
     log_event(event)