diff --git a/common/djangoapps/track/contexts.py b/common/djangoapps/track/contexts.py index db7923b1b8..8bbd40ed39 100644 --- a/common/djangoapps/track/contexts.py +++ b/common/djangoapps/track/contexts.py @@ -20,19 +20,27 @@ def course_context_from_url(url, course_id_string=None): url = url or '' course_id = None - if course_id_string is None: + if not course_id_string: match = COURSE_REGEX.match(url) if match: course_id_string = match.group('course_id') - 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 + 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=str(course_id_string)[0:256] + ), + exc_info=True + ) return course_context_from_course_id(course_id) 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 9021183224..b111623e2a 100644 --- a/common/djangoapps/track/views/__init__.py +++ b/common/djangoapps/track/views/__init__.py @@ -79,16 +79,17 @@ 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: try: data = json.loads(data) - _add_user_id_for_username(data) except ValueError: pass + _add_user_id_for_username(data) + course_id_string = data.get('courserun_key') if (data and 'courserun_key' in data) else None + context_override = contexts.course_context_from_url(page, course_id_string) context_override['username'] = username context_override['event_source'] = 'browser' diff --git a/common/djangoapps/track/views/tests/test_views.py b/common/djangoapps/track/views/tests/test_views.py index 2ffc754908..b12b37efc6 100644 --- a/common/djangoapps/track/views/tests/test_views.py +++ b/common/djangoapps/track/views/tests/test_views.py @@ -42,6 +42,32 @@ 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': { + 'courserun_key': 'explicit/course/id' + }, + '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 03c0ed18e5..3c92005200 100644 --- a/common/static/js/spec/logger_spec.js +++ b/common/static/js/spec/logger_spec.js @@ -10,14 +10,13 @@ // proper deprecation and notification for external authors. it('can send a request to log event', function() { spyOn(jQuery, 'ajaxWithPrefix'); - Logger.log('example', 'data'); + Logger.log('example'); expect(jQuery.ajaxWithPrefix).toHaveBeenCalledWith({ url: '/event', type: 'POST', data: { event_type: 'example', - event: '"data"', - courserun_key: 'edx/999/test', + event: '{"courserun_key":"edX/999/test"}', page: window.location.href }, async: true @@ -26,14 +25,13 @@ it('can send a request with custom options to log event', function() { spyOn(jQuery, 'ajaxWithPrefix'); - Logger.log('example', 'data', null, {type: 'GET', async: false}); + Logger.log('example', null, null, {type: 'GET', async: false}); expect(jQuery.ajaxWithPrefix).toHaveBeenCalledWith({ url: '/event', type: 'GET', data: { event_type: 'example', - event: '"data"', - courserun_key: 'edx/999/test', + event: '{"courserun_key":"edX/999/test"}', page: window.location.href }, async: false @@ -61,14 +59,13 @@ $meta_tag = $(''); $meta_tag.appendTo('body'); spyOn(jQuery, 'ajax'); - Logger.log('example', 'data'); + Logger.log('example'); expect(jQuery.ajax).toHaveBeenCalledWith({ url: 'undefined/event', type: 'POST', data: { event_type: 'example', - event: '"data"', - courserun_key: 'edx/999/test', + event: '{"courserun_key":"edX/999/test"}', page: window.location.href }, async: true @@ -79,14 +76,13 @@ $meta_tag = $(''); $meta_tag.appendTo('body'); spyOn(jQuery, 'ajax'); - Logger.log('example', 'data'); + Logger.log('example'); expect(jQuery.ajax).toHaveBeenCalledWith({ url: '/event', type: 'POST', data: { event_type: 'example', - event: '"data"', - courserun_key: 'edx/999/test', + event: '{"courserun_key":"edX/999/test"}', page: window.location.href }, async: true @@ -97,14 +93,13 @@ $meta_tag = $(''); $meta_tag.appendTo('body'); spyOn(jQuery, 'ajax'); - Logger.log('example', 'data'); + Logger.log('example'); expect(jQuery.ajax).toHaveBeenCalledWith({ url: 'testpath/event', type: 'POST', data: { event_type: 'example', - event: '"data"', - courserun_key: 'edx/999/test', + event: '{"courserun_key":"edX/999/test"}', page: window.location.href }, async: true @@ -127,24 +122,24 @@ }); it('can listen to events when the element name is unknown', function() { - Logger.log('example', 'data'); - expect(this.callbacks[0]).toHaveBeenCalledWith('example', 'data', null); - expect(this.callbacks[1]).toHaveBeenCalledWith('example', 'data', null); + Logger.log('example'); + expect(this.callbacks[0]).toHaveBeenCalledWith('example', undefined, null); + expect(this.callbacks[1]).toHaveBeenCalledWith('example', undefined, null); expect(this.callbacks[2]).not.toHaveBeenCalled(); expect(this.callbacks[3]).not.toHaveBeenCalled(); }); it('can listen to events when the element name is known', function() { - Logger.log('example', 'data', 'element'); + Logger.log('example', null, 'element'); expect(this.callbacks[0]).not.toHaveBeenCalled(); expect(this.callbacks[1]).not.toHaveBeenCalled(); - expect(this.callbacks[2]).toHaveBeenCalledWith('example', 'data', 'element'); + expect(this.callbacks[2]).toHaveBeenCalledWith('example', null, 'element'); expect(this.callbacks[3]).not.toHaveBeenCalled(); }); it('can catch exceptions', function() { var callback = function() { - Logger.log('exception', 'data'); + Logger.log('exception'); }; Logger.listen('exception', null, function() { throw new Error(); @@ -188,7 +183,6 @@ data: { event_type: 'page_close', event: '', - courserun_key: 'edx/999/test', page: window.location.href }, async: false diff --git a/common/static/js/src/logger.js b/common/static/js/src/logger.js index c031b55e0e..7fb1d7f5ae 100644 --- a/common/static/js/src/logger.js +++ b/common/static/js/src/logger.js @@ -56,10 +56,15 @@ } } // Regardless of whether any callbacks were made, log this event. + if (typeof $$course_id !== 'undefined') { + if (!data) { + data = {} + } + data.courserun_key = $$course_id; + } return sendRequest({ event_type: eventType, event: JSON.stringify(data), - courserun_key: $$course_id, page: window.location.href }, requestOptions); }, diff --git a/lms/static/js/spec/edxnotes/views/note_item_spec.js b/lms/static/js/spec/edxnotes/views/note_item_spec.js index 624d97f5dc..a0a84b8a09 100644 --- a/lms/static/js/spec/edxnotes/views/note_item_spec.js +++ b/lms/static/js/spec/edxnotes/views/note_item_spec.js @@ -119,7 +119,8 @@ define([ { note_id: 'id-123', component_usage_id: 'usage_id-123', - view: 'Test View' + view: 'Test View', + courserun_key: 'edX/999/test' }, null, {