Merge pull request #304 from edx/fix/adam/log-answers-with-post

Fix/adam/log answers with post

AUTHORS

@@ -78,3 +78,4 @@ Peter Fogg <peter.p.fogg@gmail.com>
 Bethany LaPenta <lapentab@mit.edu>
 Renzo Lucioni <renzolucioni@gmail.com>
 Felix Sun <felixsun@mit.edu>
+Adam Palay <adam@edx.org>

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

cms/djangoapps/contentstore/tests/test_request_event.py

+"""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)

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

common/djangoapps/track/models.py

 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)

common/djangoapps/track/tests.py

+"""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"])

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)
 

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

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

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)
 
 

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