Test fixes, err response fixes, address review comments

common/lib/xmodule/xmodule/peer_grading_module.py

@@ -217,7 +217,7 @@ class PeerGradingModule(XModule):
         required = set(['location'])
         success, message = self._check_required(get, required)
         if not success:
-            return _err_response(message)
+            return self._err_response(message)
         grader_id = self.system.anonymous_student_id
         location = get['location']
 
@@ -296,7 +296,7 @@ class PeerGradingModule(XModule):
         required = set(['location'])
         success, message = self._check_required(get, required)
         if not success:
-            return _err_response(message)
+            return self._err_response(message)
         grader_id = self.system.anonymous_student_id
 
         location = get['location']
@@ -339,7 +339,7 @@ class PeerGradingModule(XModule):
         required = set(['location'])
         success, message = self._check_required(get, required)
         if not success:
-            return _err_response(message)
+            return self._err_response(message)
 
         grader_id = self.system.anonymous_student_id
 
@@ -381,7 +381,7 @@ class PeerGradingModule(XModule):
         required = set(['location', 'submission_id', 'submission_key', 'score', 'feedback', 'rubric_scores[]'])
         success, message = self._check_required(get, required)
         if not success:
-            return _err_response(message)
+            return self._err_response(message)
         grader_id = self.system.anonymous_student_id
 
         location = get['location']
@@ -397,7 +397,7 @@ class PeerGradingModule(XModule):
             return response
         except GradingServiceError:
             log.exception("Error saving calibration grade, location: {0}, submission_id: {1}, submission_key: {2}, grader_id: {3}".format(location, submission_id, submission_key, grader_id))
-            return _err_response('Could not connect to grading service')
+            return self._err_response('Could not connect to grading service')
 
     def peer_grading(self, get = None):
         '''

lms/djangoapps/open_ended_grading/tests.py

@@ -23,6 +23,7 @@ from mitxmako.shortcuts import render_to_string
 import logging
 log = logging.getLogger(__name__)
 from override_settings import override_settings
+from django.http import QueryDict
 
 
 @override_settings(MODULESTORE=ct.TEST_DATA_XML_MODULESTORE)
@@ -100,6 +101,7 @@ class TestStaffGradingService(ct.PageLoader):
                 'submission_id': '123',
                 'location': self.location,
                 'rubric_scores[]': ['1', '2']}
+
         r = self.check_for_post_code(200, url, data)
         d = json.loads(r.content)
         self.assertTrue(d['success'], str(d))
@@ -138,11 +140,12 @@ class TestPeerGradingService(ct.PageLoader):
         
         self.course_id = "edX/toy/2012_Fall"
         self.toy = modulestore().get_course(self.course_id)
+        location = "i4x://edX/toy/peergrading/init"
 
         self.mock_service = peer_grading_service.MockPeerGradingService()
-        self.system = ModuleSystem(None, None, None, render_to_string, None)
-        self.descriptor = peer_grading_module.PeerGradingDescriptor()
-        location = "i4x://edX/toy/peergrading/init"
+        self.system = ModuleSystem(location, None, None, render_to_string, None)
+        self.descriptor = peer_grading_module.PeerGradingDescriptor(self.system)
+
         self.peer_module = peer_grading_module.PeerGradingModule(self.system,location,"<peergrading/>",self.descriptor)
         self.peer_module.peer_gs = self.mock_service
         self.logout()
@@ -153,7 +156,7 @@ class TestPeerGradingService(ct.PageLoader):
         data = {'location': self.location}
 
         r = self.peer_module.get_next_submission(data)
-        d = json.loads(r.content)
+        d = json.loads(r)
         self.assertTrue(d['success'])
         self.assertIsNotNone(d['submission_id'])
         self.assertIsNotNone(d['prompt'])
@@ -163,41 +166,35 @@ class TestPeerGradingService(ct.PageLoader):
     def test_get_next_submission_missing_location(self):
         data = {}
         r = self.peer_module.get_next_submission(data)
-        d = json.loads(r.content)
+        d = json.loads(r)
         self.assertFalse(d['success'])
         self.assertEqual(d['error'], "Missing required keys: location")
 
     def test_save_grade_success(self):
-
-        data = {'location': self.location, 
-                'submission_id': '1', 
-                'submission_key': 'fake key', 
-                'score': '2',
-                'feedback': 'This is feedback',
-                'rubric_scores[]': [1, 2],
-                'submission_flagged' : False}
-        r = self.peer_module.save_grade(data)
+        data = 'rubric_scores[]=1|rubric_scores[]=2|location=' + location + '|submission_id=1|submission_key=fake key|score=2|feedback=feedback|submission_flagged=False'
+        qdict = QueryDict(data.replace("|","&"))
+        r = self.peer_module.save_grade(qdict)
         d = json.loads(r.content)
         self.assertTrue(d['success'])
 
     def test_save_grade_missing_keys(self):
         data = {}
         r = self.peer_module.save_grade(data)
-        d = json.loads(r.content)
+        d = json.loads(r)
         self.assertFalse(d['success'])
         self.assertTrue(d['error'].find('Missing required keys:') > -1)
 
     def test_is_calibrated_success(self):
         data = {'location': self.location}
         r = self.peer_module.is_student_calibrated(data)
-        d = json.loads(r.content)
+        d = json.loads(r)
         self.assertTrue(d['success'])
         self.assertTrue('calibrated' in d)
 
     def test_is_calibrated_failure(self):
         data = {}
         r = self.peer_module.is_student_calibrated(data)
-        d = json.loads(r.content)
+        d = json.loads(r)
         self.assertFalse(d['success'])
         self.assertFalse('calibrated' in d)
 
@@ -205,7 +202,7 @@ class TestPeerGradingService(ct.PageLoader):
         data = {'location': self.location}
 
         r = self.peer_module.show_calibration_essay(data)
-        d = json.loads(r.content)
+        d = json.loads(r)
         self.assertTrue(d['success'])
         self.assertIsNotNone(d['submission_id'])
         self.assertIsNotNone(d['prompt'])
@@ -216,7 +213,7 @@ class TestPeerGradingService(ct.PageLoader):
         data = {}
 
         r = self.peer_module.show_calibration_essay(data)
-        d = json.loads(r.content)
+        d = json.loads(r)
 
         self.assertFalse(d['success'])
         self.assertEqual(d['error'], "Missing required keys: location")
@@ -229,14 +226,14 @@ class TestPeerGradingService(ct.PageLoader):
                 'feedback': 'This is feedback',
                 'rubric_scores[]': [1, 2]}
         r = self.peer_module.save_calibration_essay(data)
-        d = json.loads(r.content)
+        d = json.loads(r)
         self.assertTrue(d['success'])
         self.assertTrue('actual_score' in d)
 
     def test_save_calibration_essay_missing_keys(self):
         data = {}
         r = self.peer_module.save_calibration_essay(data)
-        d = json.loads(r.content)
+        d = json.loads(r)
         self.assertFalse(d['success'])
         self.assertTrue(d['error'].find('Missing required keys:') > -1)
         self.assertFalse('actual_score' in d)

lms/djangoapps/open_ended_grading/views.py

@@ -90,13 +90,7 @@ def peer_grading(request, course_id):
     base_course_url  = reverse('courses')
     try:
         problem_url_parts = search.path_to_location(modulestore(), course.id, pg_location)
-        problem_url = base_course_url + "/"
-        for z in xrange(0,len(problem_url_parts)):
-            part = problem_url_parts[z]
-            if part is not None:
-                if z==1:
-                    problem_url += "courseware/"
-                problem_url += part + "/"
+        problem_url = generate_problem_url(problem_url_parts, base_course_url)
 
         return HttpResponseRedirect(problem_url)
     except:
@@ -104,6 +98,22 @@ def peer_grading(request, course_id):
         log.error(error_message + "Current course is: {0}".format(course_id))
         return HttpResponse(error_message)
 
+def generate_problem_url(problem_url_parts, base_course_url):
+    """
+    From a list of problem url parts generated by search.path_to_location and a base course url, generates a url to a problem
+    @param problem_url_parts: Output of search.path_to_location
+    @param base_course_url: Base url of a given course
+    @return: A path to the problem
+    """
+    problem_url = base_course_url + "/"
+    for z in xrange(0,len(problem_url_parts)):
+        part = problem_url_parts[z]
+        if part is not None:
+            if z==1:
+                problem_url += "courseware/"
+            problem_url += part + "/"
+
+
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
 def student_problem_list(request, course_id):
     '''
@@ -130,14 +140,7 @@ def student_problem_list(request, course_id):
 
     for i in xrange(0,len(problem_list)):
         problem_url_parts = search.path_to_location(modulestore(), course.id, problem_list[i]['location'])
-        problem_url = base_course_url + "/"
-        for z in xrange(0,len(problem_url_parts)):
-            part = problem_url_parts[z]
-            if part is not None:
-                if z==1:
-                    problem_url += "courseware/"
-                problem_url += part + "/"
-
+        problem_url = generate_problem_url(problem_url_parts, base_course_url)
         problem_list[i].update({'actual_url' : problem_url})
 
     """