diff --git a/AUTHORS b/AUTHORS index 9bb4ede121..03959ca00d 100644 --- a/AUTHORS +++ b/AUTHORS @@ -78,3 +78,4 @@ Peter Fogg Bethany LaPenta Renzo Lucioni Felix Sun +Adam Palay diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ff900d6161..0b50efd677 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +Common: Student information is now passed to the tracking log via POST instead of GET. + Common: Add tests for documentation generation to test suite Blades: User answer now preserved (and changeable) after clicking "show answer" in choice problems diff --git a/cms/djangoapps/contentstore/tests/test_request_event.py b/cms/djangoapps/contentstore/tests/test_request_event.py new file mode 100644 index 0000000000..a235c71568 --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_request_event.py @@ -0,0 +1,36 @@ +"""Tests for CMS's requests to logs""" +from django.test import TestCase +from django.core.urlresolvers import reverse +from contentstore.views.requests import event as cms_user_track + + +class CMSLogTest(TestCase): + """ + Tests that request to logs from CMS return 204s + """ + + def test_post_answers_to_log(self): + """ + Checks that student answer requests submitted to cms's "/event" url + via POST are correctly returned as 204s + """ + requests = [ + {"event": "my_event", "event_type": "my_event_type", "page": "my_page"}, + {"event": "{'json': 'object'}", "event_type": unichr(512), "page": "my_page"} + ] + for request_params in requests: + response = self.client.post(reverse(cms_user_track), request_params) + self.assertEqual(response.status_code, 204) + + def test_get_answers_to_log(self): + """ + Checks that student answer requests submitted to cms's "/event" url + via GET are correctly returned as 204s + """ + requests = [ + {"event": "my_event", "event_type": "my_event_type", "page": "my_page"}, + {"event": "{'json': 'object'}", "event_type": unichr(512), "page": "my_page"} + ] + for request_params in requests: + response = self.client.get(reverse(cms_user_track), request_params) + self.assertEqual(response.status_code, 204) diff --git a/cms/envs/test.py b/cms/envs/test.py index 8fe9652c07..86925caff6 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -140,3 +140,6 @@ SEGMENT_IO_KEY = '***REMOVED***' MITX_FEATURES['STUDIO_NPS_SURVEY'] = False MITX_FEATURES['ENABLE_SERVICE_STATUS'] = True + +# Enabling SQL tracking logs for testing on common/djangoapps/track +MITX_FEATURES['ENABLE_SQL_TRACKING_LOGS'] = True diff --git a/common/djangoapps/track/models.py b/common/djangoapps/track/models.py index b6a16706c1..1ac7656244 100644 --- a/common/djangoapps/track/models.py +++ b/common/djangoapps/track/models.py @@ -1,9 +1,8 @@ from django.db import models -from django.db import models - class TrackingLog(models.Model): + """Defines the fields that are stored in the tracking log database""" dtcreated = models.DateTimeField('creation date', auto_now_add=True) username = models.CharField(max_length=32, blank=True) ip = models.CharField(max_length=32, blank=True) @@ -16,6 +15,9 @@ class TrackingLog(models.Model): host = models.CharField(max_length=64, blank=True) def __unicode__(self): - s = "[%s] %s@%s: %s | %s | %s | %s" % (self.time, self.username, self.ip, self.event_source, - self.event_type, self.page, self.event) - return s + fmt = ( + u"[{self.time}] {self.username}@{self.ip}: " + u"{self.event_source}| {self.event_type} | " + u"{self.page} | {self.event}" + ) + return fmt.format(self=self) diff --git a/common/djangoapps/track/tests.py b/common/djangoapps/track/tests.py new file mode 100644 index 0000000000..bfa84a620f --- /dev/null +++ b/common/djangoapps/track/tests.py @@ -0,0 +1,56 @@ +"""Tests for student tracking""" +from django.test import TestCase +from django.core.urlresolvers import reverse, NoReverseMatch +from track.models import TrackingLog +from track.views import user_track +from nose.plugins.skip import SkipTest + + +class TrackingTest(TestCase): + """ + Tests that tracking logs correctly handle events + """ + + def test_post_answers_to_log(self): + """ + Checks that student answer requests submitted to track.views via POST + are correctly logged in the TrackingLog db table + """ + requests = [ + {"event": "my_event", "event_type": "my_event_type", "page": "my_page"}, + {"event": "{'json': 'object'}", "event_type": unichr(512), "page": "my_page"} + ] + for request_params in requests: + try: # because /event maps to two different views in lms and cms, we're only going to test lms here + response = self.client.post(reverse(user_track), request_params) + except NoReverseMatch: + raise SkipTest() + self.assertEqual(response.status_code, 200) + self.assertEqual(response.content, 'success') + tracking_logs = TrackingLog.objects.order_by('-dtcreated') + log = tracking_logs[0] + self.assertEqual(log.event, request_params["event"]) + self.assertEqual(log.event_type, request_params["event_type"]) + self.assertEqual(log.page, request_params["page"]) + + def test_get_answers_to_log(self): + """ + Checks that student answer requests submitted to track.views via GET + are correctly logged in the TrackingLog db table + """ + requests = [ + {"event": "my_event", "event_type": "my_event_type", "page": "my_page"}, + {"event": "{'json': 'object'}", "event_type": unichr(512), "page": "my_page"} + ] + for request_params in requests: + try: # because /event maps to two different views in lms and cms, we're only going to test lms here + response = self.client.get(reverse(user_track), request_params) + except NoReverseMatch: + raise SkipTest() + self.assertEqual(response.status_code, 200) + self.assertEqual(response.content, 'success') + tracking_logs = TrackingLog.objects.order_by('-dtcreated') + log = tracking_logs[0] + self.assertEqual(log.event, request_params["event"]) + self.assertEqual(log.event_type, request_params["event_type"]) + self.assertEqual(log.page, request_params["page"]) diff --git a/common/djangoapps/track/views.py b/common/djangoapps/track/views.py index 221bab5468..b65f9fa043 100644 --- a/common/djangoapps/track/views.py +++ b/common/djangoapps/track/views.py @@ -34,9 +34,10 @@ def log_event(event): def user_track(request): """ - Log when GET call to "event" URL is made by a user. + Log when POST call to "event" URL is made by a user. Uses request.REQUEST + to allow for GET calls. - GET call should provide "event_type", "event", and "page" arguments. + GET or POST call should provide "event_type", "event", and "page" arguments. """ try: # TODO: Do the same for many of the optional META parameters username = request.user.username @@ -59,13 +60,14 @@ def user_track(request): "session": scookie, "ip": request.META['REMOTE_ADDR'], "event_source": "browser", - "event_type": request.GET['event_type'], - "event": request.GET['event'], + "event_type": request.REQUEST['event_type'], + "event": request.REQUEST['event'], "agent": agent, - "page": request.GET['page'], + "page": request.REQUEST['page'], "time": datetime.datetime.now(UTC).isoformat(), "host": request.META['SERVER_NAME'], - } + } + log_event(event) return HttpResponse('success') @@ -92,7 +94,7 @@ def server_track(request, event_type, event, page=None): "page": page, "time": datetime.datetime.now(UTC).isoformat(), "host": request.META['SERVER_NAME'], - } + } if event_type.startswith("/event_logs") and request.user.is_staff: # don't log return @@ -136,7 +138,7 @@ def task_track(request_info, task_info, event_type, event, page=None): "page": page, "time": datetime.datetime.utcnow().isoformat(), "host": request_info.get('host', 'unknown') - } + } log_event(event) diff --git a/common/static/coffee/spec/logger_spec.coffee b/common/static/coffee/spec/logger_spec.coffee index 7fe734d8b5..4a53b8c455 100644 --- a/common/static/coffee/spec/logger_spec.coffee +++ b/common/static/coffee/spec/logger_spec.coffee @@ -14,9 +14,9 @@ describe 'Logger', -> expect(analytics.track).toHaveBeenCalledWith 'seq_goto', value: 'data' it 'send a request to log event', -> - spyOn $, 'getWithPrefix' + spyOn $, 'postWithPrefix' Logger.log 'example', 'data' - expect($.getWithPrefix).toHaveBeenCalledWith '/event', + expect($.postWithPrefix).toHaveBeenCalledWith '/event', event_type: 'example' event: '"data"' page: window.location.href diff --git a/common/static/coffee/src/logger.coffee b/common/static/coffee/src/logger.coffee index 6eaa497255..dffc14e067 100644 --- a/common/static/coffee/src/logger.coffee +++ b/common/static/coffee/src/logger.coffee @@ -28,7 +28,7 @@ class @Logger callback(event_type, data, element) # Regardless of whether any callbacks were made, log this event. - $.getWithPrefix '/event', + $.postWithPrefix '/event', event_type: event_type event: JSON.stringify(data) page: window.location.href diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 5c12725d0a..4cafb0979d 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -61,9 +61,9 @@ def make_track_function(request): ''' import track.views - def f(event_type, event): + def function(event_type, event): return track.views.server_track(request, event_type, event, page='x_module') - return f + return function def toc_for_course(user, request, course, active_chapter, active_section, model_data_cache): @@ -171,9 +171,9 @@ def get_xqueue_callback_url_prefix(request): should go back to the LMS, not to the worker. """ prefix = '{proto}://{host}'.format( - proto=request.META.get('HTTP_X_FORWARDED_PROTO', 'https' if request.is_secure() else 'http'), - host=request.get_host() - ) + proto=request.META.get('HTTP_X_FORWARDED_PROTO', 'https' if request.is_secure() else 'http'), + host=request.get_host() + ) return settings.XQUEUE_INTERFACE.get('callback_url', prefix) diff --git a/lms/envs/test.py b/lms/envs/test.py index d335fcd600..f23be52a51 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -29,6 +29,9 @@ MITX_FEATURES['ENABLE_SERVICE_STATUS'] = True MITX_FEATURES['ENABLE_HINTER_INSTRUCTOR_VIEW'] = True +# Enabling SQL tracking logs for testing on common/djangoapps/track +MITX_FEATURES['ENABLE_SQL_TRACKING_LOGS'] = True + # Need wiki for courseware views to work. TODO (vshnayder): shouldn't need it. WIKI_ENABLED = True