Address code review feedback

common/lib/xmodule/xmodule/combined_open_ended_module.py

@@ -145,7 +145,7 @@ class CombinedOpenEndedModule(XModule):
             rubric_feedback = rubric_renderer.render_rubric(stringify_children(definition['rubric']))
         except RubricParsingError:
             log.error("Failed to parse rubric in location: {1}".format(location))
-            raise Exception("CombinedOpenEnded: could not render given rubric")
+            raise 
         #Static data is passed to the child modules to render
         self.static_data = {
             'max_score': self._max_score,

lms/djangoapps/open_ended_grading/grading_service.py

@@ -106,26 +106,29 @@ class GradingService(object):
     def _render_rubric(self, response, view_only=False):
         """
         Given an HTTP Response with the key 'rubric', render out the html
-        required to display the rubric
+        required to display the rubric and put it back into the response
+
+        returns the updated response as a dictionary that can be serialized later
+
         """
         try:
             response_json = json.loads(response)
-            if response_json.has_key('rubric'):
+            if 'rubric' in response_json:
                 rubric = response_json['rubric']
                 rubric_renderer = CombinedOpenEndedRubric(self.system, False)
                 rubric_html = rubric_renderer.render_rubric(rubric)
                 response_json['rubric'] = rubric_html
-            return json.dumps(response_json)
+            return response_json
         # if we can't parse the rubric into HTML, 
         except etree.XMLSyntaxError, RubricParsingError:
             log.exception("Cannot parse rubric string. Raw string: {0}"
                           .format(rubric))
-            return json.dumps({'success': False,
-                               'error': 'Error displaying submission'})
+            return {'success': False,
+                               'error': 'Error displaying submission'}
         except ValueError:
             log.exception("Error parsing response: {0}".format(response))
-            return json.dumps({'success': False,
-                                'error': "Error displaying submission"}) 
+            return {'success': False,
+                                'error': "Error displaying submission"} 
 
 
 

lms/djangoapps/open_ended_grading/peer_grading_service.py

@@ -86,7 +86,7 @@ class PeerGradingService(GradingService):
     def get_next_submission(self, problem_location, grader_id):
         response = self.get(self.get_next_submission_url,
                 {'location': problem_location, 'grader_id': grader_id})
-        return self._render_rubric(response)
+        return json.dumps(self._render_rubric(response))
 
     def save_grade(self, location, grader_id, submission_id, score, feedback, submission_key, rubric_scores):
         data = {'grader_id' : grader_id,
@@ -106,7 +106,7 @@ class PeerGradingService(GradingService):
     def show_calibration_essay(self, problem_location, grader_id):
         params = {'problem_id' : problem_location, 'student_id': grader_id}
         response = self.get(self.show_calibration_essay_url, params)
-        return self._render_rubric(response)
+        return json.dumps(self._render_rubric(response))
 
     def save_calibration_essay(self, problem_location, grader_id, calibration_essay_id, submission_key, 
             score, feedback, rubric_scores):

lms/djangoapps/open_ended_grading/staff_grading_service.py

@@ -112,7 +112,7 @@ class StaffGradingService(GradingService):
         response = self.get(self.get_next_url,
                                       params={'location': location,
                                               'grader_id': grader_id})
-        return self._render_rubric(response)
+        return json.dumps(self._render_rubric(response))
 
 
     def save_grade(self, course_id, grader_id, submission_id, score, feedback, skipped, rubric_scores):
@@ -148,7 +148,6 @@ class StaffGradingService(GradingService):
 # importing this file doesn't create objects that may not have the right config
 _service = None
 
-module_system = ModuleSystem("", None, None, render_to_string, None)
 
 def staff_grading_service():
     """