Remove duplicated data from legacy events

When event-tracking was fully integrated into the platform, some data was replicated in the context in the middleware and then later extracted and moved the old location by event-tracking. The legacy code path was not updated to remove this transient shim data from the context resulting in duplication of the information. This patch ensures that the transient information is removed from the context before emitting the event.

Fixes: AN-2369

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)