From f5600c2b8abb36583c612c184cf69a9f2a15106c Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 14 Jan 2013 15:42:38 -0500 Subject: [PATCH 1/9] Make things more robust, ensure that a rubric is specified properly before combined open ended module will render --- .../xmodule/xmodule/combined_open_ended_module.py | 7 +++++++ .../xmodule/xmodule/combined_open_ended_rubric.py | 14 ++++++++++++-- common/lib/xmodule/xmodule/open_ended_module.py | 2 +- .../lib/xmodule/xmodule/self_assessment_module.py | 2 +- .../open_ended_grading/staff_grading_service.py | 7 ++++++- 5 files changed, 27 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index a88acc6ffd..67c7d15910 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -21,6 +21,7 @@ from .xml_module import XmlDescriptor from xmodule.modulestore import Location import self_assessment_module import open_ended_module +from combined_open_ended_rubric import CombinedOpenEndedRubric from mitxmako.shortcuts import render_to_string @@ -140,6 +141,12 @@ class CombinedOpenEndedModule(XModule): # completion (doesn't matter if you self-assessed correct/incorrect). self._max_score = int(self.metadata.get('max_score', MAX_SCORE)) + rubric_renderer = CombinedOpenEndedRubric(True) + success, rubric_feedback = rubric_renderer.render_rubric(True, definition['rubric']) + if not success: + error_message="Could not parse rubric : {0}".format(definition['rubric']) + log.exception(error_message) + raise Exception(error_message) #Static data is passed to the child modules to render self.static_data = { 'max_score': self._max_score, diff --git a/common/lib/xmodule/xmodule/combined_open_ended_rubric.py b/common/lib/xmodule/xmodule/combined_open_ended_rubric.py index 4861a15dbc..2146942296 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_rubric.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_rubric.py @@ -20,16 +20,26 @@ class CombinedOpenEndedRubric: html: the html that corresponds to the xml given ''' def render_rubric(self, rubric_xml): + success = False try: + rubric_xml = rubric_xml.encode('ascii', 'ignore') rubric_categories = self.extract_categories(rubric_xml) html = render_to_string('open_ended_rubric.html', {'categories' : rubric_categories, 'has_score': self.has_score, 'view_only': self.view_only}) + success = True except: log.exception("Could not parse the rubric.") - html = etree.tostring(rubric_xml, pretty_print=True) - return html + try: + html = etree.tostring(rubric_xml, pretty_print=True) + except: + log.exception("Rubric XML is a string, not an XML object : {0}".format(rubric_xml)) + if isinstance(rubric_xml, basestring): + html = rubric_xml + else: + html = "Invalid rubric. Please contact course staff." + return success, html def extract_categories(self, element): ''' diff --git a/common/lib/xmodule/xmodule/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_module.py index 4033d140dd..824eac3639 100644 --- a/common/lib/xmodule/xmodule/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_module.py @@ -383,7 +383,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): feedback = self._convert_longform_feedback_to_html(response_items) if response_items['rubric_scores_complete']==True: rubric_renderer = CombinedOpenEndedRubric(True) - rubric_feedback = rubric_renderer.render_rubric(response_items['rubric_xml']) + success, rubric_feedback = rubric_renderer.render_rubric(response_items['rubric_xml']) if not response_items['success']: return system.render_template("open_ended_error.html", diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py index 4a7e86e134..de842091c9 100644 --- a/common/lib/xmodule/xmodule/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/self_assessment_module.py @@ -124,7 +124,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): rubric_renderer = CombinedOpenEndedRubric(True) - rubric_html = rubric_renderer.render_rubric(self.rubric) + success, rubric_html = rubric_renderer.render_rubric(self.rubric) # we'll render it context = {'rubric': rubric_html, diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index c684fa1d95..9f7daeb786 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -264,7 +264,12 @@ def _get_next(course_id, grader_id, location): response_json = json.loads(response) rubric = response_json['rubric'] rubric_renderer = CombinedOpenEndedRubric(False) - rubric_html = rubric_renderer.render_rubric(rubric) + success, rubric_html = rubric_renderer.render_rubric(rubric) + if not success: + error_message = "Could not render rubric: {0}".format(rubric) + log.exception(error_message) + return json.dumps({'success': False, + 'error': error_message}) response_json['rubric'] = rubric_html return json.dumps(response_json) except GradingServiceError: From 8e213f53d892aa3e28ea0e5edbef91cb2c5f5245 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 14 Jan 2013 15:52:57 -0500 Subject: [PATCH 2/9] Fix rubric parsing errors --- common/lib/xmodule/xmodule/combined_open_ended_module.py | 5 +++-- common/lib/xmodule/xmodule/combined_open_ended_rubric.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 67c7d15910..a35573032a 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -22,6 +22,7 @@ from xmodule.modulestore import Location import self_assessment_module import open_ended_module from combined_open_ended_rubric import CombinedOpenEndedRubric +from .stringify import stringify_children from mitxmako.shortcuts import render_to_string @@ -142,11 +143,11 @@ class CombinedOpenEndedModule(XModule): self._max_score = int(self.metadata.get('max_score', MAX_SCORE)) rubric_renderer = CombinedOpenEndedRubric(True) - success, rubric_feedback = rubric_renderer.render_rubric(True, definition['rubric']) + success, rubric_feedback = rubric_renderer.render_rubric(stringify_children(definition['rubric'])) if not success: error_message="Could not parse rubric : {0}".format(definition['rubric']) log.exception(error_message) - raise Exception(error_message) + raise Exception #Static data is passed to the child modules to render self.static_data = { 'max_score': self._max_score, diff --git a/common/lib/xmodule/xmodule/combined_open_ended_rubric.py b/common/lib/xmodule/xmodule/combined_open_ended_rubric.py index 2146942296..37ce18e4f1 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_rubric.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_rubric.py @@ -22,7 +22,6 @@ class CombinedOpenEndedRubric: def render_rubric(self, rubric_xml): success = False try: - rubric_xml = rubric_xml.encode('ascii', 'ignore') rubric_categories = self.extract_categories(rubric_xml) html = render_to_string('open_ended_rubric.html', {'categories' : rubric_categories, @@ -53,7 +52,8 @@ class CombinedOpenEndedRubric: {text: "Option 3 Name", points: 2]}] ''' - element = etree.fromstring(element) + if isinstance(element, basestring): + element = etree.fromstring(element) categories = [] for category in element: if category.tag != 'category': From 6427a7dac548cf2e747b0285139c47a5b1372243 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 14 Jan 2013 16:03:53 -0500 Subject: [PATCH 3/9] Rubric html change --- lms/djangoapps/open_ended_grading/staff_grading_service.py | 1 + lms/templates/instructor/staff_grading.html | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index 9f7daeb786..67f2775ad4 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -271,6 +271,7 @@ def _get_next(course_id, grader_id, location): return json.dumps({'success': False, 'error': error_message}) response_json['rubric'] = rubric_html + log.debug(rubric_html) return json.dumps(response_json) except GradingServiceError: log.exception("Error from grading service. server url: {0}" diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index 3749a63e73..f518f1e924 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -57,6 +57,12 @@ +
+
+ +
+
+
From 50f951d9df049781751d8d92f6cf4565dd1c144b Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 14 Jan 2013 17:14:55 -0500 Subject: [PATCH 4/9] Bug fixes --- .../staff_grading_service.py | 20 +++++++++---------- lms/templates/instructor/staff_grading.html | 6 ------ 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index 67f2775ad4..f417ca026e 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -262,16 +262,16 @@ def _get_next(course_id, grader_id, location): try: response = staff_grading_service().get_next(course_id, location, grader_id) response_json = json.loads(response) - rubric = response_json['rubric'] - rubric_renderer = CombinedOpenEndedRubric(False) - success, rubric_html = rubric_renderer.render_rubric(rubric) - if not success: - error_message = "Could not render rubric: {0}".format(rubric) - log.exception(error_message) - return json.dumps({'success': False, - 'error': error_message}) - response_json['rubric'] = rubric_html - log.debug(rubric_html) + if response_json.has_key('rubric'): + rubric = response_json['rubric'] + rubric_renderer = CombinedOpenEndedRubric(False) + success, rubric_html = rubric_renderer.render_rubric(rubric) + if not success: + error_message = "Could not render rubric: {0}".format(rubric) + log.exception(error_message) + return json.dumps({'success': False, + 'error': error_message}) + response_json['rubric'] = rubric_html return json.dumps(response_json) except GradingServiceError: log.exception("Error from grading service. server url: {0}" diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index f518f1e924..3749a63e73 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -57,12 +57,6 @@ -
-
- -
-
-
From e5c6414d39daef349bc16c136cb3c2b41e06a64b Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 14 Jan 2013 17:35:41 -0500 Subject: [PATCH 5/9] Adding score selection back into staff grading view --- .../src/staff_grading/staff_grading.coffee | 27 +++++++++++++++---- lms/templates/instructor/staff_grading.html | 2 ++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index d8fcb087a6..8e1fe738bf 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -174,7 +174,8 @@ class StaffGrading @grading_wrapper = $('.grading-wrapper') @feedback_area = $('.feedback-area') - @score_selection_container = $('.score-selection-container') + @score_selection_container = $('.score-selection-container') + @grade_selection_container = $('.grade-selection-container') @submit_button = $('.submit-button') @action_button = $('.action-button') @@ -202,6 +203,7 @@ class StaffGrading @num_graded = 0 @num_pending = 0 @score_lst = [] + @score = null @problems = null @@ -216,10 +218,23 @@ class StaffGrading setup_score_selection: => + # first, get rid of all the old inputs, if any. + @grade_selection_container.html('Choose score: ') + # Now create new labels and inputs for each possible score. + for score in [0..@max_score] + id = 'score-' + score + label = """""" + input = """ + + """ # " fix broken parsing in emacs + @grade_selection_container.append(input + label) + @score_selection_container.html(@rubric) $('.score-selection').click => @graded_callback() graded_callback: () => + @grade = $("input[name='grade-selection']:selected").val() + # check to see whether or not any categories have not been scored num_categories = $('table.rubric tr').length for i in [0..(num_categories-1)] @@ -229,8 +244,6 @@ class StaffGrading # show button if we have scores for all categories @state = state_graded @submit_button.show() - - set_button_text: (text) => @action_button.attr('value', text) @@ -272,6 +285,7 @@ class StaffGrading skip_and_get_next: () => data = + score: @grade rubric_scores: @get_score_list() feedback: @feedback_area.val() submission_id: @submission_id @@ -285,8 +299,8 @@ class StaffGrading submit_and_get_next: () -> data = + score: @grade rubric_scores: @get_score_list() - score: 0 feedback: @feedback_area.val() submission_id: @submission_id location: @location @@ -303,6 +317,8 @@ class StaffGrading @rubric = response.rubric @submission_id = response.submission_id @feedback_area.val('') + @grade = null + @max_score = response.max_score @ml_error_info=response.ml_error_info @prompt_name = response.problem_name @num_graded = response.num_graded @@ -322,9 +338,10 @@ class StaffGrading @ml_error_info = null @submission_id = null @message = message + @grade = null + @max_score = 0 @state = state_no_data - render_view: () -> # clear the problem list and breadcrumbs @problem_list.html('') diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index 3749a63e73..7364e3beb8 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -71,6 +71,8 @@
+

+

From 6f9c0a7d72f4d9c84b281d4e5baf946a863de23e Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 14 Jan 2013 17:47:44 -0500 Subject: [PATCH 7/9] Minor fix for when grade is ready to submit --- lms/static/coffee/src/staff_grading/staff_grading.coffee | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 65cf85fd8d..5bbf72529e 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -234,7 +234,8 @@ class StaffGrading graded_callback: () => @grade = $("input[name='grade-selection']:checked").val() - + if score == undefined + return # check to see whether or not any categories have not been scored num_categories = $('table.rubric tr').length for i in [0..(num_categories-1)] From 78a94a4e206c47d74efd6bd5897f7153c42fff3f Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 14 Jan 2013 17:50:20 -0500 Subject: [PATCH 8/9] Fix naming bug --- lms/static/coffee/src/staff_grading/staff_grading.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 5bbf72529e..aeb39993e8 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -234,7 +234,7 @@ class StaffGrading graded_callback: () => @grade = $("input[name='grade-selection']:checked").val() - if score == undefined + if @grade == undefined return # check to see whether or not any categories have not been scored num_categories = $('table.rubric tr').length From be3d8cbb861fd14cc6ba2edd123f100b34079ffe Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 14 Jan 2013 18:00:53 -0500 Subject: [PATCH 9/9] Fix small issue with graded callback --- lms/static/coffee/src/staff_grading/staff_grading.coffee | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index aeb39993e8..3274f2048c 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -219,19 +219,24 @@ class StaffGrading setup_score_selection: => # first, get rid of all the old inputs, if any. - @grade_selection_container.html('Choose score: ') + @grade_selection_container.html(""" +

Overall Score

+

Choose an overall score for this submission.

+ """) # Now create new labels and inputs for each possible score. for score in [0..@max_score] id = 'score-' + score label = """""" input = """ - + """ # " fix broken parsing in emacs @grade_selection_container.append(input + label) + $('.grade-selection').click => @graded_callback() @score_selection_container.html(@rubric) $('.score-selection').click => @graded_callback() + graded_callback: () => @grade = $("input[name='grade-selection']:checked").val() if @grade == undefined