diff --git a/common/djangoapps/track/contexts.py b/common/djangoapps/track/contexts.py index 531b62b385..8bbd40ed39 100644 --- a/common/djangoapps/track/contexts.py +++ b/common/djangoapps/track/contexts.py @@ -12,23 +12,32 @@ from openedx.core.lib.request_utils import COURSE_REGEX log = logging.getLogger(__name__) -def course_context_from_url(url): +def course_context_from_url(url, course_id_string=None): """ - Extracts the course_context from the given `url` and passes it on to - `course_context_from_course_id()`. + If course_id_string string is not present, extracts it from the given `url`. Either way, then passes + it on to `course_context_from_course_id()`. """ url = url or '' - - match = COURSE_REGEX.match(url) course_id = None - if match: - course_id_string = match.group('course_id') + + if not course_id_string: + match = COURSE_REGEX.match(url) + if match: + course_id_string = match.group('course_id') + if not course_id_string: + log.debug( + 'no course_id found in "{url}"'.format( + url=str(url)[0:256] + ), + exc_info=True + ) + else: try: course_id = CourseKey.from_string(course_id_string) except InvalidKeyError: log.warning( 'unable to parse course_id "{course_id}"'.format( - course_id=course_id_string + course_id=str(course_id_string)[0:256] ), exc_info=True ) diff --git a/common/djangoapps/track/tests/test_contexts.py b/common/djangoapps/track/tests/test_contexts.py index e216c60fc0..fc3a7d97d0 100644 --- a/common/djangoapps/track/tests/test_contexts.py +++ b/common/djangoapps/track/tests/test_contexts.py @@ -13,6 +13,23 @@ class TestContexts(TestCase): # lint-amnesty, pylint: disable=missing-class-doc SPLIT_COURSE_ID = 'course-v1:test+course_name+course_run' ORG_ID = 'test' + @ddt.data( + (COURSE_ID, ''), + (COURSE_ID, '/more/stuff'), + (COURSE_ID, '?format=json'), + (SPLIT_COURSE_ID, ''), + (SPLIT_COURSE_ID, '/more/stuff'), + (SPLIT_COURSE_ID, '?format=json') + ) + @ddt.unpack + def test_explicit_course_id(self, course_id, postfix): + url = f'http://foo.bar.com/courses/not-a-course-id{postfix}' + self.assert_parses_explicit_course_id(url, course_id) + + def assert_parses_explicit_course_id(self, url, course_id): + assert contexts.course_context_from_url(url, course_id_string=course_id) ==\ + {'course_id': course_id, 'org_id': self.ORG_ID, 'enterprise_uuid': ''} + @ddt.data( (COURSE_ID, ''), (COURSE_ID, '/more/stuff'), diff --git a/common/djangoapps/track/views/__init__.py b/common/djangoapps/track/views/__init__.py index 2d3deb3bde..9021183224 100644 --- a/common/djangoapps/track/views/__init__.py +++ b/common/djangoapps/track/views/__init__.py @@ -69,7 +69,8 @@ def user_track(request): """ Log when POST call to "event" URL is made by a user. - GET or POST call should provide "event_type", "event", and "page" arguments. + GET or POST call should provide "event_type", "event", and "page" arguments. It may optionally provide + a "courserun_key" argument (otherwise may be extracted from the page). """ try: username = request.user.username @@ -78,6 +79,7 @@ def user_track(request): name = _get_request_value(request, 'event_type') data = _get_request_value(request, 'event', {}) + course_id_string = _get_request_value(request, 'courserun_key', None) page = _get_request_value(request, 'page') if isinstance(data, str) and len(data) > 0: @@ -87,7 +89,7 @@ def user_track(request): except ValueError: pass - context_override = contexts.course_context_from_url(page) + context_override = contexts.course_context_from_url(page, course_id_string) context_override['username'] = username context_override['event_source'] = 'browser' context_override['page'] = page diff --git a/common/djangoapps/track/views/tests/test_views.py b/common/djangoapps/track/views/tests/test_views.py index 2ffc754908..baeef8cb7d 100644 --- a/common/djangoapps/track/views/tests/test_views.py +++ b/common/djangoapps/track/views/tests/test_views.py @@ -42,6 +42,31 @@ class TestTrackViews(EventTrackingTestCase): # lint-amnesty, pylint: disable=mi } def test_user_track(self): + request = self.request_factory.get('/event', { + 'page': self.url_with_course, + 'event_type': sentinel.event_type, + 'event': '{}', + 'courserun_key': 'explicit/course/id' + }) + + views.user_track(request) + + actual_event = self.get_event() + expected_event = { + 'context': { + 'course_id': 'explicit/course/id', + 'org_id': 'explicit', + 'event_source': 'browser', + 'page': self.url_with_course, + 'username': 'anonymous' + }, + 'data': {}, + 'timestamp': FROZEN_TIME, + 'name': str(sentinel.event_type) + } + assert_event_matches(expected_event, actual_event) + + def test_user_track_with_implicit_course_id(self): request = self.request_factory.get('/event', { 'page': self.url_with_course, 'event_type': sentinel.event_type, diff --git a/common/static/js/spec/logger_spec.js b/common/static/js/spec/logger_spec.js index 2939420465..9acc06d969 100644 --- a/common/static/js/spec/logger_spec.js +++ b/common/static/js/spec/logger_spec.js @@ -17,6 +17,7 @@ data: { event_type: 'example', event: '"data"', + courserun_key: 'edX/999/test', page: window.location.href }, async: true @@ -32,6 +33,7 @@ data: { event_type: 'example', event: '"data"', + courserun_key: 'edX/999/test', page: window.location.href }, async: false @@ -66,6 +68,7 @@ data: { event_type: 'example', event: '"data"', + courserun_key: 'edX/999/test', page: window.location.href }, async: true @@ -83,6 +86,7 @@ data: { event_type: 'example', event: '"data"', + courserun_key: 'edX/999/test', page: window.location.href }, async: true @@ -100,6 +104,7 @@ data: { event_type: 'example', event: '"data"', + courserun_key: 'edX/999/test', page: window.location.href }, async: true diff --git a/common/static/js/src/logger.js b/common/static/js/src/logger.js index 2eb0710148..e30b0c7703 100644 --- a/common/static/js/src/logger.js +++ b/common/static/js/src/logger.js @@ -59,6 +59,7 @@ return sendRequest({ event_type: eventType, event: JSON.stringify(data), + courserun_key: typeof $$course_id !== 'undefined' ? $$course_id : null, page: window.location.href }, requestOptions); },