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
This commit is contained in:
Ken Clary
2021-07-30 10:23:23 -04:00
parent 67a33d47e1
commit bbb14a2c6a
7 changed files with 85 additions and 33 deletions

View File

@@ -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)

View File

@@ -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'),

View File

@@ -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'

View File

@@ -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,

View File

@@ -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 name="path_prefix1" content="">');
$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 name="path_prefix" content="">');
$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 name="path_prefix" content="testpath">');
$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

View File

@@ -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);
},

View File

@@ -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,
{