diff --git a/common/djangoapps/track/shim.py b/common/djangoapps/track/shim.py index a0849f962b..f8c7a61c20 100644 --- a/common/djangoapps/track/shim.py +++ b/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] diff --git a/common/djangoapps/track/tests/test_views.py b/common/djangoapps/track/tests/test_views.py index 4a66db35a4..8a77245c74 100644 --- a/common/djangoapps/track/tests/test_views.py +++ b/common/djangoapps/track/tests/test_views.py @@ -1,6 +1,7 @@ # 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,11 +54,45 @@ 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) @@ -77,6 +112,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 diff --git a/common/djangoapps/track/views.py b/common/djangoapps/track/views.py index 69be4a37cc..7a11fe7692 100644 --- a/common/djangoapps/track/views.py +++ b/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)