From 6c06b39b3f7c17966c04f032cf2c1d3185759bec Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 26 Mar 2014 12:04:57 -0400 Subject: [PATCH] Centralize when json parsing of responses from controller_query_service happens --- .../controller_query_service.py | 51 +++++++++++++++---- .../openendedchild.py | 4 -- .../open_ended_notifications.py | 3 +- lms/djangoapps/open_ended_grading/tests.py | 7 +-- lms/djangoapps/open_ended_grading/utils.py | 4 +- lms/djangoapps/open_ended_grading/views.py | 5 +- 6 files changed, 47 insertions(+), 27 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py index acd8df4404..31e4962e29 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py @@ -25,7 +25,7 @@ class ControllerQueryService(GradingService): 'location': location, } response = self.get(self.check_eta_url, params) - return response + return response.json() def check_combined_notifications(self, course_id, student_id, user_is_staff, last_time_viewed): params = { @@ -36,7 +36,7 @@ class ControllerQueryService(GradingService): } log.debug(self.combined_notifications_url) response = self.get(self.combined_notifications_url, params) - return response + return response.json() def get_grading_status_list(self, course_id, student_id): params = { @@ -45,7 +45,7 @@ class ControllerQueryService(GradingService): } response = self.get(self.grading_status_list_url, params) - return response + return response.json() def get_flagged_problem_list(self, course_id): params = { @@ -53,7 +53,7 @@ class ControllerQueryService(GradingService): } response = self.get(self.flagged_problem_list_url, params) - return response + return response.json() def take_action_on_flags(self, course_id, student_id, submission_id, action_type): params = { @@ -64,7 +64,7 @@ class ControllerQueryService(GradingService): } response = self.post(self.take_action_on_flags_url, params) - return response + return response.json() class MockControllerQueryService(object): @@ -84,15 +84,47 @@ class MockControllerQueryService(object): pass def check_combined_notifications(self, *args, **kwargs): - combined_notifications = '{"flagged_submissions_exist": false, "version": 1, "new_student_grading_to_view": false, "success": true, "staff_needs_to_grade": false, "student_needs_to_peer_grade": true, "overall_need_to_check": true}' + combined_notifications = { + "flagged_submissions_exist": False, + "version": 1, + "new_student_grading_to_view": False, + "success": True, + "staff_needs_to_grade": False, + "student_needs_to_peer_grade": True, + "overall_need_to_check": True + } return combined_notifications def get_grading_status_list(self, *args, **kwargs): - grading_status_list = '{"version": 1, "problem_list": [{"problem_name": "Science Question -- Machine Assessed", "grader_type": "NA", "eta_available": true, "state": "Waiting to be Graded", "eta": 259200, "location": "i4x://MITx/oe101x/combinedopenended/Science_SA_ML"}, {"problem_name": "Humanities Question -- Peer Assessed", "grader_type": "NA", "eta_available": true, "state": "Waiting to be Graded", "eta": 259200, "location": "i4x://MITx/oe101x/combinedopenended/Humanities_SA_Peer"}], "success": true}' + grading_status_list = { + "version": 1, + "problem_list": [ + { + "problem_name": "Science Question -- Machine Assessed", + "grader_type": "NA", + "eta_available": True, + "state": "Waiting to be Graded", + "eta": 259200, + "location": "i4x://MITx/oe101x/combinedopenended/Science_SA_ML" + }, { + "problem_name": "Humanities Question -- Peer Assessed", + "grader_type": "NA", + "eta_available": True, + "state": "Waiting to be Graded", + "eta": 259200, + "location": "i4x://MITx/oe101x/combinedopenended/Humanities_SA_Peer" + } + ], + "success": True + } return grading_status_list def get_flagged_problem_list(self, *args, **kwargs): - flagged_problem_list = '{"version": 1, "success": false, "error": "No flagged submissions exist for course: MITx/oe101x/2012_Fall"}' + flagged_problem_list = { + "version": 1, + "success": False, + "error": "No flagged submissions exist for course: MITx/oe101x/2012_Fall" + } return flagged_problem_list def take_action_on_flags(self, *args, **kwargs): @@ -113,5 +145,4 @@ def convert_seconds_to_human_readable(seconds): else: human_string = "{0} days".format(round(seconds / (60 * 60 * 24), 1)) - eta_string = "{0}".format(human_string) - return eta_string + return human_string diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py index 47f6921994..b68d8ad72a 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py @@ -533,10 +533,6 @@ class OpenEndedChild(object): def get_eta(self): if self.controller_qs: response = self.controller_qs.check_for_eta(self.location_string) - try: - response = json.loads(response) - except: - pass else: return "" diff --git a/lms/djangoapps/open_ended_grading/open_ended_notifications.py b/lms/djangoapps/open_ended_grading/open_ended_notifications.py index 5826560ab4..426b93dd3c 100644 --- a/lms/djangoapps/open_ended_grading/open_ended_notifications.py +++ b/lms/djangoapps/open_ended_grading/open_ended_notifications.py @@ -165,9 +165,8 @@ def combined_notifications(course, user): try: #Get the notifications from the grading controller - controller_response = controller_qs.check_combined_notifications(course.id, student_id, user_is_staff, + notifications = controller_qs.check_combined_notifications(course.id, student_id, user_is_staff, last_time_viewed) - notifications = json.loads(controller_response) if notifications.get('success'): if (notifications.get('staff_needs_to_grade') or notifications.get('student_needs_to_peer_grade')): diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 8ac9bab50f..a7aac6130a 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -62,10 +62,9 @@ class StudentProblemListMockQuery(object): def get_grading_status_list(self, *args, **kwargs): """ Get a mock grading status list with locations from the open_ended test course. - @returns: json formatted grading status message. + @returns: grading status message dictionary. """ - grading_status_list = json.dumps( - { + return { "version": 1, "problem_list": [ { @@ -95,8 +94,6 @@ class StudentProblemListMockQuery(object): ], "success": True } - ) - return grading_status_list @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) diff --git a/lms/djangoapps/open_ended_grading/utils.py b/lms/djangoapps/open_ended_grading/utils.py index fb523df17e..4833d01fc1 100644 --- a/lms/djangoapps/open_ended_grading/utils.py +++ b/lms/djangoapps/open_ended_grading/utils.py @@ -118,12 +118,10 @@ class StudentProblemList(object): self.success = False try: #Get list of all open ended problems that the grading server knows about - problem_list_json = self.controller_qs.get_grading_status_list(self.course_id, self.user_id) + problem_list_dict = self.controller_qs.get_grading_status_list(self.course_id, self.user_id) except GradingServiceError: log.error("Problem contacting open ended grading service " + self.course_error_ending) return self.success - try: - problem_list_dict = json.loads(problem_list_json) except ValueError: log.error("Problem with results from external grading service for open ended" + self.course_error_ending) return self.success diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index 50f34f6395..c045d6e56f 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -197,8 +197,7 @@ def flagged_problem_list(request, course_id): # Make a service that can query edX ORA. controller_qs = create_controller_query_service() try: - problem_list_json = controller_qs.get_flagged_problem_list(course_id) - problem_list_dict = json.loads(problem_list_json) + problem_list_dict = controller_qs.get_flagged_problem_list(course_id) success = problem_list_dict['success'] if 'error' in problem_list_dict: error_text = problem_list_dict['error'] @@ -326,7 +325,7 @@ def take_action_on_flags(request, course_id): controller_qs = create_controller_query_service() try: response = controller_qs.take_action_on_flags(course_id, student_id, submission_id, action_type) - return HttpResponse(response, mimetype="application/json") + return HttpResponse(json.dumps(response), mimetype="application/json") except GradingServiceError: log.exception( u"Error taking action on flagged peer grading submissions, "