From 6430e7e98c3481b8aedc333ae24475e779fd998a Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 27 Mar 2014 14:09:58 -0400 Subject: [PATCH] Make GradingServices always return dictionaries, rather than sometimes returning strings --- .../grading_service_module.py | 17 ++-- .../peer_grading_service.py | 23 ++---- .../xmodule/xmodule/peer_grading_module.py | 3 +- .../staff_grading_service.py | 78 +++++++++---------- lms/djangoapps/open_ended_grading/tests.py | 2 +- 5 files changed, 53 insertions(+), 70 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py index bc97516b70..c6915ded21 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py @@ -132,30 +132,25 @@ class GradingService(object): def _render_rubric(self, response, view_only=False): """ - Given an HTTP Response with the key 'rubric', render out the html + Given an HTTP Response json with the key 'rubric', render out the html 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) - except: - response_json = response - - try: - if 'rubric' in response_json: - rubric = response_json['rubric'] + if 'rubric' in response: + rubric = response['rubric'] rubric_renderer = CombinedOpenEndedRubric(self.system, view_only) rubric_dict = rubric_renderer.render_rubric(rubric) success = rubric_dict['success'] rubric_html = rubric_dict['html'] - response_json['rubric'] = rubric_html - return response_json + response['rubric'] = rubric_html + return response # if we can't parse the rubric into HTML, except (etree.XMLSyntaxError, RubricParsingError): #This is a dev_facing_error - log.exception("Cannot parse rubric string. Raw string: {0}".format(rubric)) + log.exception("Cannot parse rubric string. Raw string: {0}".format(response['rubric'])) return {'success': False, 'error': 'Error displaying submission'} except ValueError: diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py index 6dc2e15e9a..b21dcdd639 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py @@ -29,7 +29,7 @@ class PeerGradingService(GradingService): def get_data_for_location(self, problem_location, student_id): params = {'location': problem_location, 'student_id': student_id} response = self.get(self.get_data_for_location_url, params) - return self.try_to_decode(response) + return response.json() def get_next_submission(self, problem_location, grader_id): response = self.get( @@ -39,43 +39,36 @@ class PeerGradingService(GradingService): 'grader_id': grader_id } ) - return self.try_to_decode(self._render_rubric(response)) + return self._render_rubric(response.json()) def save_grade(self, **kwargs): data = kwargs data.update({'rubric_scores_complete': True}) - return self.try_to_decode(self.post(self.save_grade_url, data)) + return self.post(self.save_grade_url, data).json() def is_student_calibrated(self, problem_location, grader_id): params = {'problem_id': problem_location, 'student_id': grader_id} - return self.try_to_decode(self.get(self.is_student_calibrated_url, params)) + return self.get(self.is_student_calibrated_url, params).json() 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.try_to_decode(self._render_rubric(response)) + return self._render_rubric(response.json()) def save_calibration_essay(self, **kwargs): data = kwargs data.update({'rubric_scores_complete': True}) - return self.try_to_decode(self.post(self.save_calibration_essay_url, data)) + return self.post(self.save_calibration_essay_url, data).json() def get_problem_list(self, course_id, grader_id): params = {'course_id': course_id, 'student_id': grader_id} response = self.get(self.get_problem_list_url, params) - return self.try_to_decode(response) + return response.json() def get_notifications(self, course_id, grader_id): params = {'course_id': course_id, 'student_id': grader_id} response = self.get(self.get_notifications_url, params) - return self.try_to_decode(response) - - def try_to_decode(self, text): - try: - text = json.loads(text) - except: - pass - return text + return response.json() """ diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index e6a30e4450..191c05b46f 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -539,8 +539,7 @@ class PeerGradingModule(PeerGradingFields, XModule): error_text = "" problem_list = [] try: - problem_list_json = self.peer_gs.get_problem_list(self.course_id, self.system.anonymous_student_id) - problem_list_dict = problem_list_json + problem_list_dict = self.peer_gs.get_problem_list(self.course_id, self.system.anonymous_student_id) success = problem_list_dict['success'] if 'error' in problem_list_dict: error_text = problem_list_dict['error'] diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index db8672aee3..e48dc56f1d 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -44,28 +44,28 @@ class MockStaffGradingService(object): def get_next(self, course_id, location, grader_id): self.cnt += 1 - return json.dumps({'success': True, - 'submission_id': self.cnt, - 'submission': 'Test submission {cnt}'.format(cnt=self.cnt), - 'num_graded': 3, - 'min_for_ml': 5, - 'num_pending': 4, - 'prompt': 'This is a fake prompt', - 'ml_error_info': 'ML info', - 'max_score': 2 + self.cnt % 3, - 'rubric': 'A rubric'}) + return {'success': True, + 'submission_id': self.cnt, + 'submission': 'Test submission {cnt}'.format(cnt=self.cnt), + 'num_graded': 3, + 'min_for_ml': 5, + 'num_pending': 4, + 'prompt': 'This is a fake prompt', + 'ml_error_info': 'ML info', + 'max_score': 2 + self.cnt % 3, + 'rubric': 'A rubric'} def get_problem_list(self, course_id, grader_id): self.cnt += 1 - return json.dumps({'success': True, - 'problem_list': [ - json.dumps({'location': 'i4x://MITx/3.091x/problem/open_ended_demo1', - 'problem_name': "Problem 1", 'num_graded': 3, 'num_pending': 5, - 'min_for_ml': 10}), - json.dumps({'location': 'i4x://MITx/3.091x/problem/open_ended_demo2', - 'problem_name': "Problem 2", 'num_graded': 1, 'num_pending': 5, - 'min_for_ml': 10}) - ]}) + return {'success': True, + 'problem_list': [ + json.dumps({'location': 'i4x://MITx/3.091x/problem/open_ended_demo1', + 'problem_name': "Problem 1", 'num_graded': 3, 'num_pending': 5, + 'min_for_ml': 10}), + json.dumps({'location': 'i4x://MITx/3.091x/problem/open_ended_demo2', + 'problem_name': "Problem 2", 'num_graded': 1, 'num_pending': 5, + 'min_for_ml': 10}) + ]} def save_grade(self, course_id, grader_id, submission_id, score, feedback, skipped, rubric_scores, submission_flagged): @@ -106,7 +106,7 @@ class StaffGradingService(GradingService): grader_id: who is grading this? The anonymous user_id of the grader. Returns: - json string with the response from the service. (Deliberately not + dict with the response from the service. (Deliberately not writing out the fields here--see the docs on the staff_grading view in the grading_controller repo) @@ -114,7 +114,7 @@ class StaffGradingService(GradingService): GradingServiceError: something went wrong with the connection. """ params = {'course_id': course_id, 'grader_id': grader_id} - return self.get(self.get_problem_list_url, params) + return self.get(self.get_problem_list_url, params).json() def get_next(self, course_id, location, grader_id): """ @@ -127,7 +127,7 @@ class StaffGradingService(GradingService): grader_id: who is grading this? The anonymous user_id of the grader. Returns: - json string with the response from the service. (Deliberately not + dict with the response from the service. (Deliberately not writing out the fields here--see the docs on the staff_grading view in the grading_controller repo) @@ -137,7 +137,7 @@ class StaffGradingService(GradingService): response = self.get(self.get_next_url, params={'location': location, 'grader_id': grader_id}) - return json.dumps(self._render_rubric(response)) + return self._render_rubric(response.json()) def save_grade(self, course_id, grader_id, submission_id, score, feedback, skipped, rubric_scores, submission_flagged): @@ -145,7 +145,7 @@ class StaffGradingService(GradingService): Save a score and feedback for a submission. Returns: - json dict with keys + dict with keys 'success': bool 'error': error msg, if something went wrong. @@ -162,12 +162,12 @@ class StaffGradingService(GradingService): 'rubric_scores_complete': True, 'submission_flagged': submission_flagged} - return self.post(self.save_grade_url, data=data) + response = self.post(self.save_grade_url, data=data) + return self._render_rubric(response.json()) def get_notifications(self, course_id): params = {'course_id': course_id} - response = self.get(self.get_notifications_url, params) - return response + return self.get(self.get_notifications_url, params).json() # don't initialize until staff_grading_service() is called--means that just @@ -249,7 +249,7 @@ def get_next(request, course_id): p = request.POST location = p['location'] - return HttpResponse(_get_next(course_id, grader_id, location), + return HttpResponse(json.dumps(_get_next(course_id, grader_id, location)), mimetype="application/json") def get_problem_list(request, course_id): @@ -278,7 +278,6 @@ def get_problem_list(request, course_id): _check_access(request.user, course_id) try: response = staff_grading_service().get_problem_list(course_id, unique_id_for_user(request.user)) - response = json.loads(response) # If 'problem_list' is in the response, then we got a list of problems from the ORA server. # If it is not, then ORA could not find any problems. @@ -373,14 +372,14 @@ def save_grade(request, course_id): location = p['location'] try: - result_json = staff_grading_service().save_grade(course_id, - grader_id, - p['submission_id'], - p['score'], - p['feedback'], - skipped, - p.getlist('rubric_scores[]'), - p['submission_flagged']) + result = staff_grading_service().save_grade(course_id, + grader_id, + p['submission_id'], + p['score'], + p['feedback'], + skipped, + p.getlist('rubric_scores[]'), + p['submission_flagged']) except GradingServiceError: #This is a dev_facing_error log.exception( @@ -388,9 +387,6 @@ def save_grade(request, course_id): request, course_id)) #This is a staff_facing_error return _err_response(STAFF_ERROR_MESSAGE) - - try: - result = json.loads(result_json) except ValueError: #This is a dev_facing_error log.exception( @@ -406,7 +402,7 @@ def save_grade(request, course_id): return _err_response(STAFF_ERROR_MESSAGE) # Ok, save_grade seemed to work. Get the next submission to grade. - return HttpResponse(_get_next(course_id, grader_id, location), + return HttpResponse(json.dumps(_get_next(course_id, grader_id, location)), mimetype="application/json") diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index a7aac6130a..75020ef024 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -45,7 +45,7 @@ class EmptyStaffGradingService(object): """ Return a staff grading response that is missing a problem list key. """ - return json.dumps({'success': True, 'error': 'No problems found.'}) + return {'success': True, 'error': 'No problems found.'} def make_instructor(course, user_email):