From 8c8450f6dc4ba15f2cb25e1dc88e3161c5ba3555 Mon Sep 17 00:00:00 2001 From: Ken Clary Date: Fri, 30 Jul 2021 10:23:23 -0400 Subject: [PATCH] feat: add explicit courserun_key parameter to /event endpoint We add 'courserun_key' (aka "course_id" though that's technically a misnomer) as an optional parameter to the /event endpoint url. If it is not present, it will still be parsed out of the url, if the url is of the right format. Additionally, Logger.log() in js adds this parameter to its /event call, pulling it from the $$course_id global. This provides opportunity for MFEs to (separately) provide the key without concern about url parsing. TNL-7752 --- common/djangoapps/track/contexts.py | 25 +++++++++++++------ .../djangoapps/track/tests/test_contexts.py | 17 +++++++++++++ common/djangoapps/track/views/__init__.py | 6 +++-- .../track/views/tests/test_views.py | 25 +++++++++++++++++++ common/static/js/spec/logger_spec.js | 5 ++++ common/static/js/src/logger.js | 1 + 6 files changed, 69 insertions(+), 10 deletions(-) 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); },