From 51e148a7a7413176afd8661d2a8defa25bd729c4 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 11:42:24 -0500 Subject: [PATCH 01/10] Add in support for rendering feedback within lms --- common/lib/capa/capa/responsetypes.py | 46 ++++++++++++++++++++++++-- lms/templates/open_ended_error.html | 12 +++++++ lms/templates/open_ended_feedback.html | 16 +++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 lms/templates/open_ended_error.html create mode 100644 lms/templates/open_ended_feedback.html diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index ba9f03549e..8bdfb4f550 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -24,6 +24,7 @@ import os import subprocess import xml.sax.saxutils as saxutils from shapely.geometry import Point, MultiPoint +from django.template.loader import render_to_string # specific library imports from calc import evaluator, UndefinedVariable @@ -1965,6 +1966,8 @@ class OpenEndedResponse(LoncapaResponse): 'max_score' : self.max_score }) + log.debug(xheader) + log.debug(contents) # Submit request. When successful, 'msg' is the prior length of the queue (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) @@ -2027,6 +2030,40 @@ class OpenEndedResponse(LoncapaResponse): def get_initial_display(self): return {self.answer_id: self.initial_display} + def _format_feedback(self, response_items): + """ + Input: + Dictionary called feedback. Must contain keys seen below. + Output: + Return success/fail, error message or feedback template + """ + tags=['feedback_items','score','grader_type'] + + for tag in tags: + if tag not in response_items: + return False, "Grader response missing required feedback key!" + + if 'errors' in response_items['feedback_items']: + return True, render_to_string("open_ended_error.html", {'errors' : response_items['feedback_items']['errors']}) + + + feedback_item_start='
' + feedback_item_end='
' + feedback_long="" + for k,v in response_items: + feedback_long+=feedback_item_start.format(feedback_key=k) + feedback_long+=v + feedback_long+=feedback_item_end + + feedback_template=render_to_string("open_ended_feedback.html",{ + 'grader_type' : response_items['grader_type'], + 'score' : response_items['score'], + 'feedback_long' : feedback_long, + }) + + return True, feedback_template + + def _parse_score_msg(self, score_msg): """ Grader reply is a JSON-dump of the following dict @@ -2052,7 +2089,7 @@ class OpenEndedResponse(LoncapaResponse): log.error("External grader message should be a JSON-serialized dict." " Received score_result = %s" % score_result) return fail - for tag in ['score','feedback']: + for tag in ['score','feedback', 'grader_type']: if tag not in score_result: log.error("External grader message is missing one or more required" " tags: 'score', 'feedback") @@ -2062,7 +2099,12 @@ class OpenEndedResponse(LoncapaResponse): # is safe for the LMS. # 1) Make sure that the message is valid XML (proper opening/closing tags) # 2) TODO: Is the message actually HTML? - feedback = score_result['feedback'] + + feedback = self._format_feedback({ + 'score' : score_result['score'], + 'feedback_items' : score_result['feedback'], + 'grader_type' : score_result['grader_type'], + }) score_ratio=int(score_result['score'])/self.max_score diff --git a/lms/templates/open_ended_error.html b/lms/templates/open_ended_error.html new file mode 100644 index 0000000000..c91beb88b0 --- /dev/null +++ b/lms/templates/open_ended_error.html @@ -0,0 +1,12 @@ +
+
+
+ There was an error with your submission. Please contact course staff. +
+
+
+
+ {errors} +
+
+
\ No newline at end of file diff --git a/lms/templates/open_ended_feedback.html b/lms/templates/open_ended_feedback.html new file mode 100644 index 0000000000..39de3ad1c3 --- /dev/null +++ b/lms/templates/open_ended_feedback.html @@ -0,0 +1,16 @@ +
+
Feedback
+
+
+

Score: ${score}

+ % if grader_type=="ML" +

Number of potential problem areas identified: ${problem_areas}

+ % endif +
+
+
+
+ ${feedback_long} +
+
+
\ No newline at end of file From 15c5072984b5e1bceed2ff1eace82380c433e00f Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 11:47:12 -0500 Subject: [PATCH 02/10] Work on template render code --- common/lib/capa/capa/responsetypes.py | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 8bdfb4f550..c587034931 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -2037,20 +2037,15 @@ class OpenEndedResponse(LoncapaResponse): Output: Return success/fail, error message or feedback template """ - tags=['feedback_items','score','grader_type'] - for tag in tags: - if tag not in response_items: - return False, "Grader response missing required feedback key!" - - if 'errors' in response_items['feedback_items']: - return True, render_to_string("open_ended_error.html", {'errors' : response_items['feedback_items']['errors']}) + if not response_items['success']: + return True, render_to_string("open_ended_error.html", {'errors' : response_items['feedback']['errors']}) feedback_item_start='
' feedback_item_end='
' feedback_long="" - for k,v in response_items: + for k,v in response_items['feedback']: feedback_long+=feedback_item_start.format(feedback_key=k) feedback_long+=v feedback_long+=feedback_item_end @@ -2089,7 +2084,7 @@ class OpenEndedResponse(LoncapaResponse): log.error("External grader message should be a JSON-serialized dict." " Received score_result = %s" % score_result) return fail - for tag in ['score','feedback', 'grader_type']: + for tag in ['score','feedback', 'grader_type', 'success', 'errors']: if tag not in score_result: log.error("External grader message is missing one or more required" " tags: 'score', 'feedback") @@ -2100,11 +2095,7 @@ class OpenEndedResponse(LoncapaResponse): # 1) Make sure that the message is valid XML (proper opening/closing tags) # 2) TODO: Is the message actually HTML? - feedback = self._format_feedback({ - 'score' : score_result['score'], - 'feedback_items' : score_result['feedback'], - 'grader_type' : score_result['grader_type'], - }) + feedback = self._format_feedback(score_result) score_ratio=int(score_result['score'])/self.max_score From 7330bef8e6e727708525afa133b93777cb55e02a Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 12:11:58 -0500 Subject: [PATCH 03/10] Changes to fix compatibility with controller feedback --- common/lib/capa/capa/responsetypes.py | 19 +++++++------------ lms/templates/open_ended_feedback.html | 2 +- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index c587034931..42f3091e6f 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -2039,21 +2039,17 @@ class OpenEndedResponse(LoncapaResponse): """ if not response_items['success']: - return True, render_to_string("open_ended_error.html", {'errors' : response_items['feedback']['errors']}) + return True, render_to_string("open_ended_error.html", {'errors' : response_items['feedback']}) + problem_areas=response_items['feedback'].count("") - feedback_item_start='
' - feedback_item_end='
' - feedback_long="" - for k,v in response_items['feedback']: - feedback_long+=feedback_item_start.format(feedback_key=k) - feedback_long+=v - feedback_long+=feedback_item_end + feedback=response_items['feedback'] feedback_template=render_to_string("open_ended_feedback.html",{ 'grader_type' : response_items['grader_type'], 'score' : response_items['score'], - 'feedback_long' : feedback_long, + 'feedback' : feedback, + 'problem_areas' : problem_areas, }) return True, feedback_template @@ -2084,10 +2080,9 @@ class OpenEndedResponse(LoncapaResponse): log.error("External grader message should be a JSON-serialized dict." " Received score_result = %s" % score_result) return fail - for tag in ['score','feedback', 'grader_type', 'success', 'errors']: + for tag in ['score', 'feedback', 'grader_type', 'success', 'errors']: if tag not in score_result: - log.error("External grader message is missing one or more required" - " tags: 'score', 'feedback") + log.error("External grader message is missing required tag: {0}".format(tag)) return fail # Next, we need to check that the contents of the external grader message diff --git a/lms/templates/open_ended_feedback.html b/lms/templates/open_ended_feedback.html index 39de3ad1c3..91efc20958 100644 --- a/lms/templates/open_ended_feedback.html +++ b/lms/templates/open_ended_feedback.html @@ -10,7 +10,7 @@
- ${feedback_long} + ${feedback}
\ No newline at end of file From 2a0c9d0c705091403661c74db79476964c7d2b47 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 12:48:13 -0500 Subject: [PATCH 04/10] Handle templating within lms --- common/lib/capa/capa/responsetypes.py | 11 +++++------ lms/templates/open_ended_error.html | 2 +- lms/templates/open_ended_feedback.html | 10 +++++----- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 42f3091e6f..7eff89fbb4 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -2035,13 +2035,12 @@ class OpenEndedResponse(LoncapaResponse): Input: Dictionary called feedback. Must contain keys seen below. Output: - Return success/fail, error message or feedback template + Return error message or feedback template """ if not response_items['success']: - return True, render_to_string("open_ended_error.html", {'errors' : response_items['feedback']}) + return render_to_string("open_ended_error.html", {'errors' : response_items['feedback']}) - problem_areas=response_items['feedback'].count("") feedback=response_items['feedback'] @@ -2049,10 +2048,9 @@ class OpenEndedResponse(LoncapaResponse): 'grader_type' : response_items['grader_type'], 'score' : response_items['score'], 'feedback' : feedback, - 'problem_areas' : problem_areas, }) - return True, feedback_template + return feedback_template def _parse_score_msg(self, score_msg): @@ -2080,7 +2078,7 @@ class OpenEndedResponse(LoncapaResponse): log.error("External grader message should be a JSON-serialized dict." " Received score_result = %s" % score_result) return fail - for tag in ['score', 'feedback', 'grader_type', 'success', 'errors']: + for tag in ['score', 'feedback', 'grader_type', 'success']: if tag not in score_result: log.error("External grader message is missing required tag: {0}".format(tag)) return fail @@ -2098,6 +2096,7 @@ class OpenEndedResponse(LoncapaResponse): if score_ratio>=.66: correct=True + log.debug(feedback) try: etree.fromstring(feedback) except etree.XMLSyntaxError as err: diff --git a/lms/templates/open_ended_error.html b/lms/templates/open_ended_error.html index c91beb88b0..fb3698ccd3 100644 --- a/lms/templates/open_ended_error.html +++ b/lms/templates/open_ended_error.html @@ -6,7 +6,7 @@
- {errors} + {{errors}}
\ No newline at end of file diff --git a/lms/templates/open_ended_feedback.html b/lms/templates/open_ended_feedback.html index 91efc20958..1e59652d33 100644 --- a/lms/templates/open_ended_feedback.html +++ b/lms/templates/open_ended_feedback.html @@ -2,15 +2,15 @@
Feedback
-

Score: ${score}

- % if grader_type=="ML" -

Number of potential problem areas identified: ${problem_areas}

- % endif +

Score: {{score}}

+ {% if grader_type == "ML" %} +

Check below for full feedback:

+ {% endif %}
- ${feedback} + {% autoescape off %} {{feedback |safe }} {% endautoescape %}
\ No newline at end of file From 9fb6c6f9891006f5372c3bd46e83add9b67f0c09 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 13:18:32 -0500 Subject: [PATCH 05/10] Debug template issues --- lms/templates/open_ended_feedback.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/templates/open_ended_feedback.html b/lms/templates/open_ended_feedback.html index 1e59652d33..2dc8f555f6 100644 --- a/lms/templates/open_ended_feedback.html +++ b/lms/templates/open_ended_feedback.html @@ -10,7 +10,7 @@
- {% autoescape off %} {{feedback |safe }} {% endautoescape %} + {% autoescape off %} {{feedback}} {% endautoescape %}
\ No newline at end of file From 2621d07ced5904b0c52882c5d0d7af0329c7c9ca Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 13:48:17 -0500 Subject: [PATCH 06/10] Remove unneeded debug statements --- common/lib/capa/capa/responsetypes.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 7eff89fbb4..aec90627f2 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1966,8 +1966,6 @@ class OpenEndedResponse(LoncapaResponse): 'max_score' : self.max_score }) - log.debug(xheader) - log.debug(contents) # Submit request. When successful, 'msg' is the prior length of the queue (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) From a20a6c8fb59ec755521857458dc2096cae9d911e Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 14:11:43 -0500 Subject: [PATCH 07/10] Do all html rendering and generation in lms --- common/lib/capa/capa/responsetypes.py | 47 ++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index aec90627f2..ec3f4763d5 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -2028,6 +2028,46 @@ class OpenEndedResponse(LoncapaResponse): def get_initial_display(self): return {self.answer_id: self.initial_display} + def _convert_longform_feedback_to_html(response_items): + """ + Take in a dictionary, and return html formatted strings appropriate for sending via xqueue. + Input: + Dictionary with keys success, feedback, and errors + Output: + String + """ + + feedback_item_start='
' + feedback_item_end='
' + + for tag in ['status', 'feedback']: + if tag not in response_items: + feedback_long=feedback_item_start.format(feedback_key="errors") + "Error getting feedback." + feedback_item_end + + feedback_items=response_items['feedback'] + try: + feedback_items=json.loads(feedback_items) + except: + pass + + success=response_items['success'] + + if success: + feedback_long="" + for k,v in feedback_items.items(): + feedback_long+=feedback_item_start.format(feedback_key=k) + feedback_long+=str(v) + feedback_long+=feedback_item_end + + if len(feedback_items)==0: + feedback_long=feedback_item_start.format(feedback_key="feedback") + "No feedback available." + feedback_item_end + + else: + feedback_long=feedback_item_start.format(feedback_key="errors") + response_items['feedback'] + feedback_item_end + + return feedback_long + + def _format_feedback(self, response_items): """ Input: @@ -2036,11 +2076,10 @@ class OpenEndedResponse(LoncapaResponse): Return error message or feedback template """ + feedback=self._convert_longform_feedback_to_html(response_items) + if not response_items['success']: - return render_to_string("open_ended_error.html", {'errors' : response_items['feedback']}) - - - feedback=response_items['feedback'] + return render_to_string("open_ended_error.html", {'errors' : feedback}) feedback_template=render_to_string("open_ended_feedback.html",{ 'grader_type' : response_items['grader_type'], From 6e75584d06f7954cf21abf4f9ef7edf9f6d497ee Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 14:14:29 -0500 Subject: [PATCH 08/10] Convert templates to mako --- common/lib/capa/capa/responsetypes.py | 5 ++--- lms/templates/open_ended_error.html | 2 +- lms/templates/open_ended_feedback.html | 8 ++++---- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index ec3f4763d5..56be2ae584 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -24,7 +24,6 @@ import os import subprocess import xml.sax.saxutils as saxutils from shapely.geometry import Point, MultiPoint -from django.template.loader import render_to_string # specific library imports from calc import evaluator, UndefinedVariable @@ -2079,9 +2078,9 @@ class OpenEndedResponse(LoncapaResponse): feedback=self._convert_longform_feedback_to_html(response_items) if not response_items['success']: - return render_to_string("open_ended_error.html", {'errors' : feedback}) + return self.system.render_template("open_ended_error.html", {'errors' : feedback}) - feedback_template=render_to_string("open_ended_feedback.html",{ + feedback_template=self.system.render_template("open_ended_feedback.html",{ 'grader_type' : response_items['grader_type'], 'score' : response_items['score'], 'feedback' : feedback, diff --git a/lms/templates/open_ended_error.html b/lms/templates/open_ended_error.html index fb3698ccd3..58a90f86ef 100644 --- a/lms/templates/open_ended_error.html +++ b/lms/templates/open_ended_error.html @@ -6,7 +6,7 @@
- {{errors}} + ${errors}
\ No newline at end of file diff --git a/lms/templates/open_ended_feedback.html b/lms/templates/open_ended_feedback.html index 2dc8f555f6..cb90006456 100644 --- a/lms/templates/open_ended_feedback.html +++ b/lms/templates/open_ended_feedback.html @@ -2,15 +2,15 @@
Feedback
-

Score: {{score}}

- {% if grader_type == "ML" %} +

Score: ${score}

+ % if grader_type == "ML":

Check below for full feedback:

- {% endif %} + % endif
- {% autoescape off %} {{feedback}} {% endautoescape %} + ${ feedback | n}
\ No newline at end of file From 3fa1cfd010b5e2f5573d074da92c2b5de60449d2 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 14:16:30 -0500 Subject: [PATCH 09/10] Minor bugfix --- common/lib/capa/capa/responsetypes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 56be2ae584..451a863573 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -2027,7 +2027,7 @@ class OpenEndedResponse(LoncapaResponse): def get_initial_display(self): return {self.answer_id: self.initial_display} - def _convert_longform_feedback_to_html(response_items): + def _convert_longform_feedback_to_html(self,response_items): """ Take in a dictionary, and return html formatted strings appropriate for sending via xqueue. Input: From bcd30223208177f6cfdd3ae2b41f6ef75e3a39fc Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 15:26:52 -0500 Subject: [PATCH 10/10] Add in import for reverse in staff grading (wouldn't work without it) --- common/lib/capa/capa/responsetypes.py | 2 -- lms/djangoapps/instructor/views.py | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 451a863573..f387a20bb5 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1921,8 +1921,6 @@ class OpenEndedResponse(LoncapaResponse): else: self.max_score = 1 - log.debug(self.max_score) - def get_score(self, student_answers): try: diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 389a64721a..79cf0caaf3 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -12,6 +12,7 @@ from django.http import HttpResponse from django_future.csrf import ensure_csrf_cookie from django.views.decorators.cache import cache_control from mitxmako.shortcuts import render_to_response +from django.core.urlresolvers import reverse from courseware import grades from courseware.access import has_access, get_access_group_name