From f0b4ac3ab8183ff2af39595bffa87e29c2dc61cd Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 5 Nov 2012 14:11:29 -0500 Subject: [PATCH 01/89] add open ended input type and html template --- .idea/.name | 1 + .idea/encodings.xml | 5 + .../inspectionProfiles/profiles_settings.xml | 7 + .idea/misc.xml | 8 + .idea/mitx.iml | 9 + .idea/modules.xml | 9 + .idea/scopes/scope_settings.xml | 5 + .idea/vcs.xml | 7 + .idea/workspace.xml | 516 ++++++++++++++++++ common/lib/capa/capa/inputtypes.py | 49 ++ .../capa/capa/templates/openendedinput.html | 32 ++ 11 files changed, 648 insertions(+) create mode 100644 .idea/.name create mode 100644 .idea/encodings.xml create mode 100644 .idea/inspectionProfiles/profiles_settings.xml create mode 100644 .idea/misc.xml create mode 100644 .idea/mitx.iml create mode 100644 .idea/modules.xml create mode 100644 .idea/scopes/scope_settings.xml create mode 100644 .idea/vcs.xml create mode 100644 .idea/workspace.xml create mode 100644 common/lib/capa/capa/templates/openendedinput.html diff --git a/.idea/.name b/.idea/.name new file mode 100644 index 0000000000..36d4bf6d51 --- /dev/null +++ b/.idea/.name @@ -0,0 +1 @@ +mitx \ No newline at end of file diff --git a/.idea/encodings.xml b/.idea/encodings.xml new file mode 100644 index 0000000000..e206d70d85 --- /dev/null +++ b/.idea/encodings.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/.idea/inspectionProfiles/profiles_settings.xml b/.idea/inspectionProfiles/profiles_settings.xml new file mode 100644 index 0000000000..c60c33bb47 --- /dev/null +++ b/.idea/inspectionProfiles/profiles_settings.xml @@ -0,0 +1,7 @@ + + + + \ No newline at end of file diff --git a/.idea/misc.xml b/.idea/misc.xml new file mode 100644 index 0000000000..e746fb7776 --- /dev/null +++ b/.idea/misc.xml @@ -0,0 +1,8 @@ + + + + $APPLICATION_HOME_DIR$/lib/pycharm.jar!/resources/html5-schema/html5.rnc + + + + diff --git a/.idea/mitx.iml b/.idea/mitx.iml new file mode 100644 index 0000000000..2d50e7b1fc --- /dev/null +++ b/.idea/mitx.iml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/.idea/modules.xml b/.idea/modules.xml new file mode 100644 index 0000000000..7eaf1301cf --- /dev/null +++ b/.idea/modules.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/.idea/scopes/scope_settings.xml b/.idea/scopes/scope_settings.xml new file mode 100644 index 0000000000..922003b843 --- /dev/null +++ b/.idea/scopes/scope_settings.xml @@ -0,0 +1,5 @@ + + + + \ No newline at end of file diff --git a/.idea/vcs.xml b/.idea/vcs.xml new file mode 100644 index 0000000000..c80f2198b5 --- /dev/null +++ b/.idea/vcs.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/.idea/workspace.xml b/.idea/workspace.xml new file mode 100644 index 0000000000..ef9844844f --- /dev/null +++ b/.idea/workspace.xml @@ -0,0 +1,516 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 1351640399572 + 1351640399572 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index ec1cda83c7..70fe5dd6c8 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -733,3 +733,52 @@ class ChemicalEquationInput(InputTypeBase): return {'previewer': '/static/js/capa/chemical_equation_preview.js',} registry.register(ChemicalEquationInput) + +#----------------------------------------------------------------------------- + +class OpenEndedInput(InputTypeBase): + """ + A text area input for code--uses codemirror, does syntax highlighting, special tab handling, + etc. + """ + + template = "openendedinput.html" + tags = ['openendedinput'] + + # pulled out for testing + submitted_msg = ("Submitted. As soon as your submission is" + " graded, this message will be replaced with the grader's feedback.") + + @classmethod + def get_attributes(cls): + """ + Convert options to a convenient format. + """ + return [Attribute('rows', '30'), + Attribute('cols', '80'), + Attribute('hidden', ''), + ] + + def setup(self): + """ + Implement special logic: handle queueing state, and default input. + """ + # if no student input yet, then use the default input given by the problem + if not self.value: + self.value = self.xml.text + + # Check if problem has been queued + self.queue_len = 0 + # Flag indicating that the problem has been queued, 'msg' is length of queue + if self.status == 'incomplete': + self.status = 'queued' + self.queue_len = self.msg + self.msg = self.submitted_msg + + def _extra_context(self): + """Defined queue_len, add it """ + return {'queue_len': self.queue_len,} + +registry.register(OpenEndedInput) + +#----------------------------------------------------------------------------- \ No newline at end of file diff --git a/common/lib/capa/capa/templates/openendedinput.html b/common/lib/capa/capa/templates/openendedinput.html new file mode 100644 index 0000000000..697bff8082 --- /dev/null +++ b/common/lib/capa/capa/templates/openendedinput.html @@ -0,0 +1,32 @@ +
+ + +
+ % if status == 'unsubmitted': + Unanswered + % elif status == 'correct': + Correct + % elif status == 'incorrect': + Incorrect + % elif status == 'queued': + Queued + + % endif + + % if hidden: +
+ % endif + +

${status}

+
+ + + +
+ ${msg|n} +
+
From 329ea7ab7200db8662767758b95cfd9aa3378a6a Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 5 Nov 2012 14:12:22 -0500 Subject: [PATCH 02/89] removed .idea files --- .gitmodules | 0 .idea/.name | 1 - .idea/encodings.xml | 5 - .../inspectionProfiles/profiles_settings.xml | 7 - .idea/misc.xml | 8 - .idea/mitx.iml | 9 - .idea/modules.xml | 9 - .idea/scopes/scope_settings.xml | 5 - .idea/vcs.xml | 7 - .idea/workspace.xml | 516 ------------------ lms/static/admin/css/ie.css | 63 --- 11 files changed, 630 deletions(-) delete mode 100644 .gitmodules delete mode 100644 .idea/.name delete mode 100644 .idea/encodings.xml delete mode 100644 .idea/inspectionProfiles/profiles_settings.xml delete mode 100644 .idea/misc.xml delete mode 100644 .idea/mitx.iml delete mode 100644 .idea/modules.xml delete mode 100644 .idea/scopes/scope_settings.xml delete mode 100644 .idea/vcs.xml delete mode 100644 .idea/workspace.xml delete mode 100644 lms/static/admin/css/ie.css diff --git a/.gitmodules b/.gitmodules deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/.idea/.name b/.idea/.name deleted file mode 100644 index 36d4bf6d51..0000000000 --- a/.idea/.name +++ /dev/null @@ -1 +0,0 @@ -mitx \ No newline at end of file diff --git a/.idea/encodings.xml b/.idea/encodings.xml deleted file mode 100644 index e206d70d85..0000000000 --- a/.idea/encodings.xml +++ /dev/null @@ -1,5 +0,0 @@ - - - - - diff --git a/.idea/inspectionProfiles/profiles_settings.xml b/.idea/inspectionProfiles/profiles_settings.xml deleted file mode 100644 index c60c33bb47..0000000000 --- a/.idea/inspectionProfiles/profiles_settings.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - \ No newline at end of file diff --git a/.idea/misc.xml b/.idea/misc.xml deleted file mode 100644 index e746fb7776..0000000000 --- a/.idea/misc.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - - $APPLICATION_HOME_DIR$/lib/pycharm.jar!/resources/html5-schema/html5.rnc - - - - diff --git a/.idea/mitx.iml b/.idea/mitx.iml deleted file mode 100644 index 2d50e7b1fc..0000000000 --- a/.idea/mitx.iml +++ /dev/null @@ -1,9 +0,0 @@ - - - - - - - - - diff --git a/.idea/modules.xml b/.idea/modules.xml deleted file mode 100644 index 7eaf1301cf..0000000000 --- a/.idea/modules.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - - - - - - diff --git a/.idea/scopes/scope_settings.xml b/.idea/scopes/scope_settings.xml deleted file mode 100644 index 922003b843..0000000000 --- a/.idea/scopes/scope_settings.xml +++ /dev/null @@ -1,5 +0,0 @@ - - - - \ No newline at end of file diff --git a/.idea/vcs.xml b/.idea/vcs.xml deleted file mode 100644 index c80f2198b5..0000000000 --- a/.idea/vcs.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - - - - diff --git a/.idea/workspace.xml b/.idea/workspace.xml deleted file mode 100644 index ef9844844f..0000000000 --- a/.idea/workspace.xml +++ /dev/null @@ -1,516 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 1351640399572 - 1351640399572 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/lms/static/admin/css/ie.css b/lms/static/admin/css/ie.css deleted file mode 100644 index fd00f7f204..0000000000 --- a/lms/static/admin/css/ie.css +++ /dev/null @@ -1,63 +0,0 @@ -/* IE 6 & 7 */ - -/* Proper fixed width for dashboard in IE6 */ - -.dashboard #content { - *width: 768px; -} - -.dashboard #content-main { - *width: 535px; -} - -/* IE 6 ONLY */ - -/* Keep header from flowing off the page */ - -#container { - _position: static; -} - -/* Put the right sidebars back on the page */ - -.colMS #content-related { - _margin-right: 0; - _margin-left: 10px; - _position: static; -} - -/* Put the left sidebars back on the page */ - -.colSM #content-related { - _margin-right: 10px; - _margin-left: -115px; - _position: static; -} - -.form-row { - _height: 1%; -} - -/* Fix right margin for changelist filters in IE6 */ - -#changelist-filter ul { - _margin-right: -10px; -} - -/* IE ignores min-height, but treats height as if it were min-height */ - -.change-list .filtered { - _height: 400px; -} - -/* IE doesn't know alpha transparency in PNGs */ - -.inline-deletelink { - background: transparent url(../img/inline-delete-8bit.png) no-repeat; -} - -/* IE7 doesn't support inline-block */ -.change-list ul.toplinks li { - zoom: 1; - *display: inline; -} \ No newline at end of file From de6e913d983f2f1e7cccf5694f318352b55dc7b1 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 5 Nov 2012 14:51:51 -0500 Subject: [PATCH 03/89] copy code response input type and modify a bit --- common/lib/capa/capa/responsetypes.py | 204 +++++++++++++++++++++++++- 1 file changed, 203 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 20e7c43577..53c9637373 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1814,6 +1814,207 @@ class ImageResponse(LoncapaResponse): return (dict([(ie.get('id'), ie.get('rectangle')) for ie in self.ielements]), dict([(ie.get('id'), ie.get('regions')) for ie in self.ielements])) #----------------------------------------------------------------------------- + +class OpenEndedResponse(LoncapaResponse): + """ + Grade student open ended responses using an external queueing server, called 'xqueue' + + Expects 'xqueue' dict in ModuleSystem with the following keys that are needed by OpenEndedResponse: + system.xqueue = { 'interface': XqueueInterface object, + 'callback_url': Per-StudentModule callback URL + where results are posted (string), + 'default_queuename': Default queuename to submit request (string) + } + + External requests are only submitted for student submission grading + (i.e. and not for getting reference answers) + """ + + response_tag = 'openendedresponse' + allowed_inputfields = ['openendedinput'] + max_inputfields = 1 + + def setup_response(self): + ''' + Configure OpenEndedResponse from XML. + ''' + xml = self.xml + # TODO: XML can override external resource (grader/queue) URL + self.url = xml.get('url', None) + self.queue_name = xml.get('queuename', self.system.xqueue['default_queuename']) + + # VS[compat]: + # Check if XML uses the ExternalResponse format or the generic OpenEndedResponse format + oeparam = self.xml.find('openendedparam') + self._parse_openendedresponse_xml(oeparam) + + def _parse_openendedresponse_xml(self,oeparam): + ''' + Parse OpenEndedResponse XML: + self.initial_display + self.answer (an answer to display to the student in the LMS) + self.payload + ''' + # Note that OpenEndedResponse is agnostic to the specific contents of grader_payload + grader_payload = oeparam.find('grader_payload') + grader_payload = grader_payload.text if grader_payload is not None else '' + self.payload = {'grader_payload': grader_payload} + + answer_display = oeparam.find('answer_display') + if answer_display is not None: + self.answer = answer_display.text + else: + self.answer = 'No answer provided.' + + initial_display = oeparam.find('initial_display') + if initial_display is not None: + self.initial_display = initial_display.text + else: + self.initial_display = '' + + def get_score(self, student_answers): + try: + # Note that submission can be a file + submission = student_answers[self.answer_id] + except Exception as err: + log.error('Error in OpenEndedResponse %s: cannot get student answer for %s;' + ' student_answers=%s' % + (err, self.answer_id, convert_files_to_filenames(student_answers))) + raise Exception(err) + + # Prepare xqueue request + #------------------------------------------------------------ + + qinterface = self.system.xqueue['interface'] + qtime = datetime.strftime(datetime.now(), xqueue_interface.dateformat) + + anonymous_student_id = self.system.anonymous_student_id + + # Generate header + queuekey = xqueue_interface.make_hashkey(str(self.system.seed) + qtime + + anonymous_student_id + + self.answer_id) + xheader = xqueue_interface.make_xheader(lms_callback_url=self.system.xqueue['callback_url'], + lms_key=queuekey, + queue_name=self.queue_name) + + self.context.update({'submission': submission}) + + contents = self.payload.copy() + + # Metadata related to the student submission revealed to the external grader + student_info = {'anonymous_student_id': anonymous_student_id, + 'submission_time': qtime, + } + contents.update({'student_info': json.dumps(student_info)}) + + # Submit request. When successful, 'msg' is the prior length of the queue + contents.update({'student_response': submission}) + (error, msg) = qinterface.send_to_queue(header=xheader, + body=json.dumps(contents)) + + # State associated with the queueing request + queuestate = {'key': queuekey, + 'time': qtime,} + + cmap = CorrectMap() + if error: + cmap.set(self.answer_id, queuestate=None, + msg='Unable to deliver your submission to grader. (Reason: %s.)' + ' Please try again later.' % msg) + else: + # Queueing mechanism flags: + # 1) Backend: Non-null CorrectMap['queuestate'] indicates that + # the problem has been queued + # 2) Frontend: correctness='incomplete' eventually trickles down + # through inputtypes.textbox and .filesubmission to inform the + # browser to poll the LMS + cmap.set(self.answer_id, queuestate=queuestate, correctness='incomplete', msg=msg) + + return cmap + + def update_score(self, score_msg, oldcmap, queuekey): + + (valid_score_msg, correct, points, msg) = self._parse_score_msg(score_msg) + if not valid_score_msg: + oldcmap.set(self.answer_id, + msg='Invalid grader reply. Please contact the course staff.') + return oldcmap + + correctness = 'correct' if correct else 'incorrect' + + # TODO: Find out how this is used elsewhere, if any + self.context['correct'] = correctness + + # Replace 'oldcmap' with new grading results if queuekey matches. If queuekey + # does not match, we keep waiting for the score_msg whose key actually matches + if oldcmap.is_right_queuekey(self.answer_id, queuekey): + # Sanity check on returned points + if points < 0: + points = 0 + elif points > self.maxpoints[self.answer_id]: + points = self.maxpoints[self.answer_id] + # Queuestate is consumed + oldcmap.set(self.answer_id, npoints=points, correctness=correctness, + msg=msg.replace(' ', ' '), queuestate=None) + else: + log.debug('OpenEndedResponse: queuekey %s does not match for answer_id=%s.' % + (queuekey, self.answer_id)) + + return oldcmap + + def get_answers(self): + anshtml = '
%s
' % self.answer + return {self.answer_id: anshtml} + + def get_initial_display(self): + return {self.answer_id: self.initial_display} + + def _parse_score_msg(self, score_msg): + """ + Grader reply is a JSON-dump of the following dict + { 'correct': True/False, + 'score': Numeric value (floating point is okay) to assign to answer + 'msg': grader_msg } + + Returns (valid_score_msg, correct, score, msg): + valid_score_msg: Flag indicating valid score_msg format (Boolean) + correct: Correctness of submission (Boolean) + score: Points to be assigned (numeric, can be float) + msg: Message from grader to display to student (string) + """ + fail = (False, False, 0, '') + try: + score_result = json.loads(score_msg) + except (TypeError, ValueError): + log.error("External grader message should be a JSON-serialized dict." + " Received score_msg = %s" % score_msg) + return fail + if not isinstance(score_result, dict): + log.error("External grader message should be a JSON-serialized dict." + " Received score_result = %s" % score_result) + return fail + for tag in ['correct', 'score', 'msg']: + if tag not in score_result: + log.error("External grader message is missing one or more required" + " tags: 'correct', 'score', 'msg'") + return fail + + # Next, we need to check that the contents of the external grader message + # 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? + msg = score_result['msg'] + try: + etree.fromstring(msg) + except etree.XMLSyntaxError as err: + log.error("Unable to parse external grader message as valid" + " XML: score_msg['msg']=%s" % msg) + return fail + + return (True, score_result['correct'], score_result['score'], msg) + +#----------------------------------------------------------------------------- # TEMPORARY: List of all response subclasses # FIXME: To be replaced by auto-registration @@ -1830,4 +2031,5 @@ __all__ = [CodeResponse, ChoiceResponse, MultipleChoiceResponse, TrueFalseResponse, - JavascriptResponse] + JavascriptResponse, + OpenEndedResponse] From 8e87d49228c20514f83ddb661034eebb4da240f6 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 5 Nov 2012 15:30:34 -0500 Subject: [PATCH 04/89] setup grader type parameter to be paarsed from xml --- common/lib/capa/capa/responsetypes.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 53c9637373..d5b0c86ec1 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1852,26 +1852,27 @@ class OpenEndedResponse(LoncapaResponse): ''' Parse OpenEndedResponse XML: self.initial_display - self.answer (an answer to display to the student in the LMS) self.payload + self.grader_type - type of grader to use. One of 'peer','ml','turk' + ''' # Note that OpenEndedResponse is agnostic to the specific contents of grader_payload grader_payload = oeparam.find('grader_payload') grader_payload = grader_payload.text if grader_payload is not None else '' self.payload = {'grader_payload': grader_payload} - answer_display = oeparam.find('answer_display') - if answer_display is not None: - self.answer = answer_display.text - else: - self.answer = 'No answer provided.' - initial_display = oeparam.find('initial_display') if initial_display is not None: self.initial_display = initial_display.text else: self.initial_display = '' + grader_type = oeparam.find('initial_display') + if grader_type is not None: + self.grader_type = grader_type.text + else: + self.grader_type='ml' + def get_score(self, student_answers): try: # Note that submission can be a file @@ -1910,6 +1911,7 @@ class OpenEndedResponse(LoncapaResponse): # Submit request. When successful, 'msg' is the prior length of the queue contents.update({'student_response': submission}) + contents.update({'grader_type'} : self.grader_type) (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) From 79941271d7341a0adde11f8c708c2987bdda61b8 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 5 Nov 2012 15:50:18 -0500 Subject: [PATCH 05/89] fix bracket in wrong spot --- 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 d5b0c86ec1..e18c894452 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1911,7 +1911,7 @@ class OpenEndedResponse(LoncapaResponse): # Submit request. When successful, 'msg' is the prior length of the queue contents.update({'student_response': submission}) - contents.update({'grader_type'} : self.grader_type) + contents.update({'grader_type' : self.grader_type}) (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) From 5c7e6415be911a3007d527a952fe354f66847777 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 5 Nov 2012 18:10:29 -0500 Subject: [PATCH 06/89] Modify capa types --- common/lib/capa/capa/responsetypes.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index e18c894452..6fd975aa45 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1867,12 +1867,14 @@ class OpenEndedResponse(LoncapaResponse): else: self.initial_display = '' - grader_type = oeparam.find('initial_display') + grader_type = oeparam.find('grader_type') if grader_type is not None: self.grader_type = grader_type.text else: self.grader_type='ml' + self.answer="None available." + def get_score(self, student_answers): try: # Note that submission can be a file @@ -1912,6 +1914,7 @@ class OpenEndedResponse(LoncapaResponse): # Submit request. When successful, 'msg' is the prior length of the queue contents.update({'student_response': submission}) contents.update({'grader_type' : self.grader_type}) + log.debug(contents) (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) From bf361af3b37c351933142a71d496912e76f99920 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 6 Nov 2012 20:43:38 -0500 Subject: [PATCH 07/89] Added debug code, then removed it --- common/lib/capa/capa/responsetypes.py | 5 +---- common/lib/capa/capa/xqueue_interface.py | 1 + 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 6fd975aa45..d12e451b7a 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1843,8 +1843,6 @@ class OpenEndedResponse(LoncapaResponse): self.url = xml.get('url', None) self.queue_name = xml.get('queuename', self.system.xqueue['default_queuename']) - # VS[compat]: - # Check if XML uses the ExternalResponse format or the generic OpenEndedResponse format oeparam = self.xml.find('openendedparam') self._parse_openendedresponse_xml(oeparam) @@ -1914,7 +1912,7 @@ class OpenEndedResponse(LoncapaResponse): # Submit request. When successful, 'msg' is the prior length of the queue contents.update({'student_response': submission}) contents.update({'grader_type' : self.grader_type}) - log.debug(contents) + (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) @@ -1939,7 +1937,6 @@ class OpenEndedResponse(LoncapaResponse): return cmap def update_score(self, score_msg, oldcmap, queuekey): - (valid_score_msg, correct, points, msg) = self._parse_score_msg(score_msg) if not valid_score_msg: oldcmap.set(self.answer_id, diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index 0214488cce..f145cad23c 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -49,6 +49,7 @@ def parse_xreply(xreply): return_code = xreply['return_code'] content = xreply['content'] + return (return_code, content) From e2d19d1a4083f57e587d51889635b95fdb82e02e Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 7 Nov 2012 10:21:56 -0500 Subject: [PATCH 08/89] Add openendedparam to list of tags to remove from output html --- common/lib/capa/capa/capa_problem.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index db42fb698a..53ddab00f3 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -53,7 +53,7 @@ response_tag_dict = dict([(x.response_tag, x) for x in responsetypes.__all__]) solution_tags = ['solution'] # these get captured as student responses -response_properties = ["codeparam", "responseparam", "answer"] +response_properties = ["codeparam", "responseparam", "answer", "openendedparam"] # special problem tags which should be turned into innocuous HTML html_transforms = {'problem': {'tag': 'div'}, @@ -72,7 +72,7 @@ global_context = {'random': random, 'miller': chem.miller} # These should be removed from HTML output, including all subelements -html_problem_semantics = ["codeparam", "responseparam", "answer", "script", "hintgroup"] +html_problem_semantics = ["codeparam", "responseparam", "answer", "script", "hintgroup", "openendedparam"] log = logging.getLogger('mitx.' + __name__) From ce7444780bf5e9528fa3244957ed80b9cbfe7b00 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 7 Nov 2012 11:53:01 -0500 Subject: [PATCH 09/89] Add in feedback, work on css to display open ended feedback --- common/lib/capa/capa/responsetypes.py | 10 +++++++--- common/lib/xmodule/xmodule/css/capa/display.scss | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index d12e451b7a..4eaaa1607f 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1937,6 +1937,7 @@ class OpenEndedResponse(LoncapaResponse): return cmap def update_score(self, score_msg, oldcmap, queuekey): + log.debug(score_msg) (valid_score_msg, correct, points, msg) = self._parse_score_msg(score_msg) if not valid_score_msg: oldcmap.set(self.answer_id, @@ -1996,17 +1997,20 @@ 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 ['correct', 'score', 'msg']: + for tag in ['correct', 'score', 'msg', 'feedback']: if tag not in score_result: log.error("External grader message is missing one or more required" - " tags: 'correct', 'score', 'msg'") + " tags: 'correct', 'score', 'msg', 'feedback") return fail + #Extract feedback from score_result + feedback = score_result['feedback'] # Next, we need to check that the contents of the external grader message # 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? msg = score_result['msg'] + try: etree.fromstring(msg) except etree.XMLSyntaxError as err: @@ -2014,7 +2018,7 @@ class OpenEndedResponse(LoncapaResponse): " XML: score_msg['msg']=%s" % msg) return fail - return (True, score_result['correct'], score_result['score'], msg) + return (True, score_result['correct'], score_result['score'], feedback) #----------------------------------------------------------------------------- # TEMPORARY: List of all response subclasses diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index fd67a3804e..098100f297 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -685,6 +685,21 @@ section.problem { color: #B00; } } + + .markup-text{ + margin: 5px; + padding: 20px 0px 15px 50px; + border-top: 1px solid #DDD; + border-left: 20px solid #FAFAFA; + + .badspelling { + color: #BB0000; + } + + .badgrammar { + color: #BDA046; + } + } } } } From d59681e3aa4ddfd584648020ff6ef1c66aba4ddc Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 7 Nov 2012 13:52:04 -0500 Subject: [PATCH 10/89] Alter css tag names, change documentation and answer display settings. --- common/lib/capa/capa/responsetypes.py | 19 ++++++++++--------- .../lib/xmodule/xmodule/css/capa/display.scss | 4 ++-- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 4eaaa1607f..bdeea036f6 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1850,28 +1850,29 @@ class OpenEndedResponse(LoncapaResponse): ''' Parse OpenEndedResponse XML: self.initial_display - self.payload - self.grader_type - type of grader to use. One of 'peer','ml','turk' - + self.payload - dict containing keys -- + 'grader' : path to grader settings file, 'problem_id' : id of the problem + + self.answer - What to display when show answer is clicked ''' # Note that OpenEndedResponse is agnostic to the specific contents of grader_payload grader_payload = oeparam.find('grader_payload') grader_payload = grader_payload.text if grader_payload is not None else '' self.payload = {'grader_payload': grader_payload} + #Parse initial display initial_display = oeparam.find('initial_display') if initial_display is not None: self.initial_display = initial_display.text else: self.initial_display = '' - grader_type = oeparam.find('grader_type') - if grader_type is not None: - self.grader_type = grader_type.text + #Parse answer display + answer_display = oeparam.find('answer_display') + if answer_display is not None: + self.answer= answer_display.text else: - self.grader_type='ml' - - self.answer="None available." + self.answer = "No answer available." def get_score(self, student_answers): try: diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index 098100f297..58ba7b00ed 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -692,11 +692,11 @@ section.problem { border-top: 1px solid #DDD; border-left: 20px solid #FAFAFA; - .badspelling { + bs { color: #BB0000; } - .badgrammar { + bg { color: #BDA046; } } From a901757dd741dc82fcce39c7bf2ac3338390cec7 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 7 Nov 2012 13:53:45 -0500 Subject: [PATCH 11/89] Fix minor bug with removed grader type tag --- common/lib/capa/capa/responsetypes.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index bdeea036f6..7101dccfd9 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1852,7 +1852,7 @@ class OpenEndedResponse(LoncapaResponse): self.initial_display self.payload - dict containing keys -- 'grader' : path to grader settings file, 'problem_id' : id of the problem - + self.answer - What to display when show answer is clicked ''' # Note that OpenEndedResponse is agnostic to the specific contents of grader_payload @@ -1912,7 +1912,6 @@ class OpenEndedResponse(LoncapaResponse): # Submit request. When successful, 'msg' is the prior length of the queue contents.update({'student_response': submission}) - contents.update({'grader_type' : self.grader_type}) (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) From dba8a7a767a477336d06589386a5d22e007c1d63 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 7 Nov 2012 14:16:56 -0500 Subject: [PATCH 12/89] Add anonymous student id to grader payload --- common/lib/capa/capa/responsetypes.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 7101dccfd9..bef79e3a4b 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1843,6 +1843,7 @@ class OpenEndedResponse(LoncapaResponse): self.url = xml.get('url', None) self.queue_name = xml.get('queuename', self.system.xqueue['default_queuename']) + #Look for tag named openendedparam that encapsulates all grader settings oeparam = self.xml.find('openendedparam') self._parse_openendedresponse_xml(oeparam) @@ -1858,8 +1859,17 @@ class OpenEndedResponse(LoncapaResponse): # Note that OpenEndedResponse is agnostic to the specific contents of grader_payload grader_payload = oeparam.find('grader_payload') grader_payload = grader_payload.text if grader_payload is not None else '' + + #Update grader payload with student id. If grader payload not json, error. + try: + grader_payload=json.loads(grader_payload) + grader_payload.update({'student_id' : self.system.anonymous_student_id}) + grader_payload=json.dumps(grader_payload) + except Exception as err: + log.error("Grader payload is not a json object!") self.payload = {'grader_payload': grader_payload} + #Parse initial display initial_display = oeparam.find('initial_display') if initial_display is not None: From 941ef528638315b2316e654e961b0882df921749 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 7 Nov 2012 15:54:49 -0500 Subject: [PATCH 13/89] Add docs, fix some logic --- common/lib/capa/capa/responsetypes.py | 28 +++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index bef79e3a4b..daaea90562 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1867,8 +1867,8 @@ class OpenEndedResponse(LoncapaResponse): grader_payload=json.dumps(grader_payload) except Exception as err: log.error("Grader payload is not a json object!") - self.payload = {'grader_payload': grader_payload} + self.payload = {'grader_payload': grader_payload} #Parse initial display initial_display = oeparam.find('initial_display') @@ -1882,16 +1882,15 @@ class OpenEndedResponse(LoncapaResponse): if answer_display is not None: self.answer= answer_display.text else: - self.answer = "No answer available." + self.answer = "No answer given." def get_score(self, student_answers): + try: - # Note that submission can be a file submission = student_answers[self.answer_id] except Exception as err: log.error('Error in OpenEndedResponse %s: cannot get student answer for %s;' - ' student_answers=%s' % - (err, self.answer_id, convert_files_to_filenames(student_answers))) + ' student_answers=%s', err, self.answer_id) raise Exception(err) # Prepare xqueue request @@ -1906,6 +1905,7 @@ class OpenEndedResponse(LoncapaResponse): queuekey = xqueue_interface.make_hashkey(str(self.system.seed) + qtime + anonymous_student_id + self.answer_id) + xheader = xqueue_interface.make_xheader(lms_callback_url=self.system.xqueue['callback_url'], lms_key=queuekey, queue_name=self.queue_name) @@ -1918,11 +1918,11 @@ class OpenEndedResponse(LoncapaResponse): student_info = {'anonymous_student_id': anonymous_student_id, 'submission_time': qtime, } - contents.update({'student_info': json.dumps(student_info)}) + + #Update contents with student response and student info + contents.update({'student_info': json.dumps(student_info), 'student_response': submission}) # Submit request. When successful, 'msg' is the prior length of the queue - contents.update({'student_response': submission}) - (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) @@ -1988,7 +1988,9 @@ class OpenEndedResponse(LoncapaResponse): Grader reply is a JSON-dump of the following dict { 'correct': True/False, 'score': Numeric value (floating point is okay) to assign to answer - 'msg': grader_msg } + 'msg': grader_msg + 'feedback' : feedback from grader + } Returns (valid_score_msg, correct, score, msg): valid_score_msg: Flag indicating valid score_msg format (Boolean) @@ -2012,22 +2014,24 @@ class OpenEndedResponse(LoncapaResponse): log.error("External grader message is missing one or more required" " tags: 'correct', 'score', 'msg', 'feedback") return fail - #Extract feedback from score_result - feedback = score_result['feedback'] # Next, we need to check that the contents of the external grader message # 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? msg = score_result['msg'] + feedback = score_result['feedback'] try: etree.fromstring(msg) + etree.fromstring(feedback) except etree.XMLSyntaxError as err: log.error("Unable to parse external grader message as valid" - " XML: score_msg['msg']=%s" % msg) + " Msg: score_msg['msg']=%r " + "\n Feedback : score_result['feedback'] = %r", msg, feedback) return fail + #Currently ignore msg and only return feedback (which takes the place of msg) return (True, score_result['correct'], score_result['score'], feedback) #----------------------------------------------------------------------------- From 06a00a982ae301ff9dde498100c5344c2378546d Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 13 Nov 2012 11:56:14 -0500 Subject: [PATCH 14/89] Send location to external grader --- common/lib/capa/capa/responsetypes.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index daaea90562..3193643318 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1863,7 +1863,8 @@ class OpenEndedResponse(LoncapaResponse): #Update grader payload with student id. If grader payload not json, error. try: grader_payload=json.loads(grader_payload) - grader_payload.update({'student_id' : self.system.anonymous_student_id}) + location=self.system.ajax_url.split("://")[1] + grader_payload.update({'location' : location}) grader_payload=json.dumps(grader_payload) except Exception as err: log.error("Grader payload is not a json object!") From 9abd80203fba176c654876bceb2be991e9042571 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 14 Nov 2012 10:24:19 -0500 Subject: [PATCH 15/89] Pass course id in payload --- common/lib/capa/capa/responsetypes.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 3193643318..dc120f7f47 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1864,7 +1864,11 @@ class OpenEndedResponse(LoncapaResponse): try: grader_payload=json.loads(grader_payload) location=self.system.ajax_url.split("://")[1] - grader_payload.update({'location' : location}) + org,course,type,name=location.split("/") + grader_payload.update({ + 'location' : location, + 'course_id' : "{0}/{1}".format(org,course) + }) grader_payload=json.dumps(grader_payload) except Exception as err: log.error("Grader payload is not a json object!") From 14403103a440cca4635b51b31e6d0bf7cf3c33e6 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 15 Nov 2012 14:00:45 -0500 Subject: [PATCH 16/89] Remove msg key from xqueue passback dict --- common/lib/capa/capa/responsetypes.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index dc120f7f47..0a5471d47f 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -2001,7 +2001,6 @@ class OpenEndedResponse(LoncapaResponse): valid_score_msg: Flag indicating valid score_msg format (Boolean) correct: Correctness of submission (Boolean) score: Points to be assigned (numeric, can be float) - msg: Message from grader to display to student (string) """ fail = (False, False, 0, '') try: @@ -2014,26 +2013,23 @@ 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 ['correct', 'score', 'msg', 'feedback']: + for tag in ['correct', 'score','feedback']: if tag not in score_result: log.error("External grader message is missing one or more required" - " tags: 'correct', 'score', 'msg', 'feedback") + " tags: 'correct', 'score', 'feedback") return fail # Next, we need to check that the contents of the external grader message # 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? - msg = score_result['msg'] feedback = score_result['feedback'] try: - etree.fromstring(msg) etree.fromstring(feedback) except etree.XMLSyntaxError as err: log.error("Unable to parse external grader message as valid" - " Msg: score_msg['msg']=%r " - "\n Feedback : score_result['feedback'] = %r", msg, feedback) + "\n Feedback : score_result['feedback'] = %r",feedback) return fail #Currently ignore msg and only return feedback (which takes the place of msg) From 0c69c466583f17429d0b2cc4243cf00df15b6f2c Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 15 Nov 2012 15:55:00 -0500 Subject: [PATCH 17/89] Remove need for msg --- common/lib/capa/capa/responsetypes.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 0a5471d47f..6cf04458b6 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1959,6 +1959,7 @@ class OpenEndedResponse(LoncapaResponse): msg='Invalid grader reply. Please contact the course staff.') return oldcmap + correctness = 'correct' if correct else 'incorrect' # TODO: Find out how this is used elsewhere, if any @@ -2025,6 +2026,10 @@ class OpenEndedResponse(LoncapaResponse): # 2) TODO: Is the message actually HTML? feedback = score_result['feedback'] + correct=False + if score_result['correct']=="True": + correct=True + try: etree.fromstring(feedback) except etree.XMLSyntaxError as err: @@ -2033,7 +2038,7 @@ class OpenEndedResponse(LoncapaResponse): return fail #Currently ignore msg and only return feedback (which takes the place of msg) - return (True, score_result['correct'], score_result['score'], feedback) + return (True, correct, score_result['score'], feedback) #----------------------------------------------------------------------------- # TEMPORARY: List of all response subclasses From e17db85d34b5db91b5f3fc34c93754e3b0bbaa6a Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Thu, 15 Nov 2012 16:11:44 -0500 Subject: [PATCH 18/89] Add in max score attribute for proper instructor scoring --- common/lib/capa/capa/responsetypes.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 6cf04458b6..887aaa9752 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1889,6 +1889,18 @@ class OpenEndedResponse(LoncapaResponse): else: self.answer = "No answer given." + #Parse max_score + top_score = oeparam.find('max_score') + if top_score is not None: + try: + self.max_score= int(top_score.text) + except: + self.top_score=1 + else: + self.max_score = 1 + + log.debug(self.max_score) + def get_score(self, student_answers): try: @@ -1925,7 +1937,11 @@ class OpenEndedResponse(LoncapaResponse): } #Update contents with student response and student info - contents.update({'student_info': json.dumps(student_info), 'student_response': submission}) + contents.update({ + 'student_info': json.dumps(student_info), + 'student_response': submission, + 'max_score' : self.max_score + }) # Submit request. When successful, 'msg' is the prior length of the queue (error, msg) = qinterface.send_to_queue(header=xheader, From 565f502cb17707d64c1c5c86eed3218020464db4 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 20 Nov 2012 09:28:10 -0500 Subject: [PATCH 19/89] Add in prompt tag to openended response and parse/pass along properly --- common/lib/capa/capa/responsetypes.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 887aaa9752..1fa93eac62 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1845,9 +1845,10 @@ class OpenEndedResponse(LoncapaResponse): #Look for tag named openendedparam that encapsulates all grader settings oeparam = self.xml.find('openendedparam') - self._parse_openendedresponse_xml(oeparam) + prompt=self.xml.find('prompt') + self._parse_openendedresponse_xml(oeparam,prompt) - def _parse_openendedresponse_xml(self,oeparam): + def _parse_openendedresponse_xml(self,oeparam,prompt): ''' Parse OpenEndedResponse XML: self.initial_display @@ -1857,6 +1858,16 @@ class OpenEndedResponse(LoncapaResponse): self.answer - What to display when show answer is clicked ''' # Note that OpenEndedResponse is agnostic to the specific contents of grader_payload + + #Modify code from stringify_children in xmodule. Didn't import directly in order to avoid capa depending + #on xmodule (seems to be avoided in code) + prompt_parts=[prompt.text] + [prompt_parts.append((etree.tostring(p, with_tail=True))) for p in prompt.getchildren()] + prompt_string=' '.join(prompt_parts) + + #Strip html tags from prompt. This may need to be removed in order to display prompt to instructors properly. + prompt_string=re.sub('<[^<]+?>', '', prompt_string) + grader_payload = oeparam.find('grader_payload') grader_payload = grader_payload.text if grader_payload is not None else '' @@ -1867,7 +1878,8 @@ class OpenEndedResponse(LoncapaResponse): org,course,type,name=location.split("/") grader_payload.update({ 'location' : location, - 'course_id' : "{0}/{1}".format(org,course) + 'course_id' : "{0}/{1}".format(org,course), + 'prompt' : prompt_string }) grader_payload=json.dumps(grader_payload) except Exception as err: From d71445f9d89caf9f3f84396b02a82b8cfebdd8b1 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 20 Nov 2012 13:52:58 -0500 Subject: [PATCH 20/89] Add rubric field to open ended response --- common/lib/capa/capa/responsetypes.py | 32 ++++++++++++++++++--------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 1fa93eac62..14b3c97886 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1846,9 +1846,25 @@ class OpenEndedResponse(LoncapaResponse): #Look for tag named openendedparam that encapsulates all grader settings oeparam = self.xml.find('openendedparam') prompt=self.xml.find('prompt') + rubric=self.xml.find('rubric') self._parse_openendedresponse_xml(oeparam,prompt) - def _parse_openendedresponse_xml(self,oeparam,prompt): + def stringify_children(self,node,strip_tags=True): + """ + Modify code from stringify_children in xmodule. Didn't import directly in order to avoid capa depending + on xmodule (seems to be avoided in code) + """ + parts=[node.text] + [parts.append((etree.tostring(p, with_tail=True))) for p in node.getchildren()] + node_string=' '.join(parts) + + #Strip html tags from prompt. This may need to be removed in order to display prompt to instructors properly. + if strip_tags: + node_string=re.sub('<[^<]+?>', '', node_string) + + return node_string + + def _parse_openendedresponse_xml(self,oeparam,prompt,rubric): ''' Parse OpenEndedResponse XML: self.initial_display @@ -1858,15 +1874,8 @@ class OpenEndedResponse(LoncapaResponse): self.answer - What to display when show answer is clicked ''' # Note that OpenEndedResponse is agnostic to the specific contents of grader_payload - - #Modify code from stringify_children in xmodule. Didn't import directly in order to avoid capa depending - #on xmodule (seems to be avoided in code) - prompt_parts=[prompt.text] - [prompt_parts.append((etree.tostring(p, with_tail=True))) for p in prompt.getchildren()] - prompt_string=' '.join(prompt_parts) - - #Strip html tags from prompt. This may need to be removed in order to display prompt to instructors properly. - prompt_string=re.sub('<[^<]+?>', '', prompt_string) + prompt_string=self.stringify_children(prompt) + rubric_string=self.stringify_children(rubric) grader_payload = oeparam.find('grader_payload') grader_payload = grader_payload.text if grader_payload is not None else '' @@ -1879,7 +1888,8 @@ class OpenEndedResponse(LoncapaResponse): grader_payload.update({ 'location' : location, 'course_id' : "{0}/{1}".format(org,course), - 'prompt' : prompt_string + 'prompt' : prompt_string, + 'rubric' : rubric_string, }) grader_payload=json.dumps(grader_payload) except Exception as err: From 1e194f0e1aa0cb159eccafedee7b8a638caef4c0 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 20 Nov 2012 13:57:46 -0500 Subject: [PATCH 21/89] Fix rubric code, make sure it is removed properly. --- common/lib/capa/capa/capa_problem.py | 2 +- common/lib/capa/capa/responsetypes.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 53ddab00f3..2eaa0e4286 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -72,7 +72,7 @@ global_context = {'random': random, 'miller': chem.miller} # These should be removed from HTML output, including all subelements -html_problem_semantics = ["codeparam", "responseparam", "answer", "script", "hintgroup", "openendedparam"] +html_problem_semantics = ["codeparam", "responseparam", "answer", "script", "hintgroup", "openendedparam","openendedrubric"] log = logging.getLogger('mitx.' + __name__) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 14b3c97886..82ed4d2ff7 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1846,8 +1846,8 @@ class OpenEndedResponse(LoncapaResponse): #Look for tag named openendedparam that encapsulates all grader settings oeparam = self.xml.find('openendedparam') prompt=self.xml.find('prompt') - rubric=self.xml.find('rubric') - self._parse_openendedresponse_xml(oeparam,prompt) + rubric=self.xml.find('openendedrubric') + self._parse_openendedresponse_xml(oeparam,prompt,rubric) def stringify_children(self,node,strip_tags=True): """ From 422ecb5b9d1e22b4e0ae7ff95255a8fb62041904 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 20 Nov 2012 18:51:56 -0500 Subject: [PATCH 22/89] define correctness in the response type --- common/lib/capa/capa/responsetypes.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 82ed4d2ff7..6a9ac98171 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -2052,7 +2052,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 ['correct', 'score','feedback']: + for tag in ['score','feedback']: if tag not in score_result: log.error("External grader message is missing one or more required" " tags: 'correct', 'score', 'feedback") @@ -2064,8 +2064,10 @@ class OpenEndedResponse(LoncapaResponse): # 2) TODO: Is the message actually HTML? feedback = score_result['feedback'] - correct=False - if score_result['correct']=="True": + score_ratio=int(score_result['score'])/self.max_score + + correct=FALSE + if score_ratio>=.66: correct=True try: From 1a22d3a15fb942b3dd9c0675b91c98f41d1087ef Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 20 Nov 2012 19:04:46 -0500 Subject: [PATCH 23/89] Lowercase False --- 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 6a9ac98171..31fb7b0f0f 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -2066,7 +2066,7 @@ class OpenEndedResponse(LoncapaResponse): score_ratio=int(score_result['score'])/self.max_score - correct=FALSE + correct=False if score_ratio>=.66: correct=True From c2bf0689b658f6a87a1b68b1ea82e64a6ce76e22 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 20 Nov 2012 19:24:39 -0500 Subject: [PATCH 24/89] Remove correct from expected tags --- 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 31fb7b0f0f..ba9f03549e 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -2055,7 +2055,7 @@ class OpenEndedResponse(LoncapaResponse): for tag in ['score','feedback']: if tag not in score_result: log.error("External grader message is missing one or more required" - " tags: 'correct', 'score', 'feedback") + " tags: 'score', 'feedback") return fail # Next, we need to check that the contents of the external grader message From 6b5125c4dfa1d143d2b66dfd54e94b474c003773 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 13 Nov 2012 18:07:59 -0500 Subject: [PATCH 25/89] fix typos, logger configs --- common/lib/xmodule/xmodule/seq_module.py | 2 +- lms/djangoapps/courseware/tabs.py | 2 +- lms/djangoapps/instructor/views.py | 2 +- lms/urls.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index b625646e66..817ed9ab2e 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -10,7 +10,7 @@ from xmodule.progress import Progress from xmodule.exceptions import NotFoundError from pkg_resources import resource_string -log = logging.getLogger("mitx.common.lib.seq_module") +log = logging.getLogger(__name__) # HACK: This shouldn't be hard-coded to two types # OBSOLETE: This obsoletes 'type' diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 980fedb947..4eaef20089 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -36,7 +36,7 @@ CourseTab = namedtuple('CourseTab', 'name link is_active') # wrong. (e.g. "is there a 'name' field?). Validators can assume # that the type field is valid. # -# - a function that takes a config, a user, and a course, and active_page and +# - a function that takes a config, a user, and a course, an active_page and # return a list of CourseTabs. (e.g. "return a CourseTab with specified # name, link to courseware, and is_active=True/False"). The function can # assume that it is only called with configs of the appropriate type that diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index f985cc43a0..fec7536151 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -27,7 +27,7 @@ from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundErr from xmodule.modulestore.search import path_to_location import track.views -log = logging.getLogger("mitx.courseware") +log = logging.getLogger(__name__) template_imports = {'urllib': urllib} diff --git a/lms/urls.py b/lms/urls.py index 529396c20e..62842ec5fe 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -161,7 +161,7 @@ if settings.COURSEWARE_ENABLED: # input types system so that previews can be context-specific. # Unfortunately, we don't have time to think through the right way to do # that (and implement it), and it's not a terrible thing to provide a - # generic chemican-equation rendering service. + # generic chemical-equation rendering service. url(r'^preview/chemcalc', 'courseware.module_render.preview_chemcalc', name='preview_chemcalc'), From f5d9e963cc897e19ceead213e7a6dedc0d9dc9af Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 13 Nov 2012 18:46:21 -0500 Subject: [PATCH 26/89] Initial wiring of a staff grading tab. - no actual functionality, but have a tab that renders static html via a view --- lms/djangoapps/courseware/tabs.py | 9 ++++++++ lms/djangoapps/instructor/grading.py | 25 +++++++++++++++++++++ lms/djangoapps/instructor/views.py | 21 +++++++++++++++++ lms/templates/instructor/staff_grading.html | 20 +++++++++++++++++ lms/urls.py | 2 ++ 5 files changed, 77 insertions(+) create mode 100644 lms/djangoapps/instructor/grading.py create mode 100644 lms/templates/instructor/staff_grading.html diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 4eaef20089..45b4e1821c 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -97,6 +97,14 @@ def _textbooks(tab, user, course, active_page): for index, textbook in enumerate(course.textbooks)] return [] + +def _staff_grading(tab, user, course, active_page): + if has_access(user, course, 'staff'): + link = reverse('staff_grading', args=[course.id]) + return [CourseTab('Staff grading', link, active_page == "staff_grading")] + return [] + + #### Validators @@ -132,6 +140,7 @@ VALID_TAB_TYPES = { 'textbooks': TabImpl(null_validator, _textbooks), 'progress': TabImpl(need_name, _progress), 'static_tab': TabImpl(key_checker(['name', 'url_slug']), _static_tab), + 'staff_grading': TabImpl(null_validator, _staff_grading), } diff --git a/lms/djangoapps/instructor/grading.py b/lms/djangoapps/instructor/grading.py new file mode 100644 index 0000000000..7a48b25a49 --- /dev/null +++ b/lms/djangoapps/instructor/grading.py @@ -0,0 +1,25 @@ +""" +LMS part of instructor grading: + +- views + ajax handling +- calls the instructor grading service +""" + +import json +import logging + +log = logging.getLogger(__name__) + + +class StaffGrading(object): + """ + Wrap up functionality for staff grading of submissions--interface exposes get_html, ajax views. + """ + def __init__(self, course): + self.course = course + + def get_html(self): + return "Instructor grading!" + # context = {} + # return render_to_string('courseware/instructor_grading_view.html', context) + diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index fec7536151..8a33f1d60b 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -27,6 +27,9 @@ from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundErr from xmodule.modulestore.search import path_to_location import track.views +from .grading import StaffGrading + + log = logging.getLogger(__name__) template_imports = {'urllib': urllib} @@ -409,6 +412,24 @@ def get_student_grade_summary_data(request, course, course_id, get_grades=True, return datatable + +@cache_control(no_cache=True, no_store=True, must_revalidate=True) +def staff_grading(request, course_id): + """ + Show the instructor grading interface. + """ + course = get_course_with_access(request.user, course_id, 'staff') + + grading = StaffGrading(course) + + return render_to_response('instructor/staff_grading.html', { + 'view_html': grading.get_html(), + 'course': course, + 'course_id': course_id, + # Checked above + 'staff_access': True, }) + + @cache_control(no_cache=True, no_store=True, must_revalidate=True) def gradebook(request, course_id): """ diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html new file mode 100644 index 0000000000..b00fd935aa --- /dev/null +++ b/lms/templates/instructor/staff_grading.html @@ -0,0 +1,20 @@ +<%inherit file="/main.html" /> +<%block name="bodyclass">${course.css_class} +<%namespace name='static' file='/static_content.html'/> + +<%block name="headextra"> + <%static:css group='course'/> + + +<%block name="title">${course.number} Staff Grading + +<%include file="/courseware/course_navigation.html" args="active_page='staff_grading'" /> + +<%block name="js_extra"> + + +
+
+ ${view_html} +
+
diff --git a/lms/urls.py b/lms/urls.py index 62842ec5fe..7c1ccb6bde 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -230,6 +230,8 @@ if settings.COURSEWARE_ENABLED: 'instructor.views.grade_summary', name='grade_summary'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/enroll_students$', 'instructor.views.enroll_students', name='enroll_students'), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/staff_grading$', + 'instructor.views.staff_grading', name='staff_grading'), ) # discussion forums live within courseware, so courseware must be enabled first From 7d1d135c1638d752ad5a79db2a1fa4a2ba9ed32e Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 13 Nov 2012 18:46:38 -0500 Subject: [PATCH 27/89] Wire in some initial js. --- lms/envs/common.py | 15 +++++++++++++-- .../coffee/src/staff_grading/staff_grading.coffee | 5 +++++ lms/templates/instructor/staff_grading.html | 1 + 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 lms/static/coffee/src/staff_grading/staff_grading.coffee diff --git a/lms/envs/common.py b/lms/envs/common.py index dd9013bcb3..008de7ac84 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -406,6 +406,9 @@ main_vendor_js = [ discussion_js = sorted(glob2.glob(PROJECT_ROOT / 'static/coffee/src/discussion/**/*.coffee')) +staff_grading_js = sorted(glob2.glob(PROJECT_ROOT / 'static/coffee/src/staff_grading/**/*.coffee')) + + # Load javascript from all of the available xmodules, and # prep it for use in pipeline js from xmodule.x_module import XModuleDescriptor @@ -468,7 +471,8 @@ with open(module_styles_path, 'w') as module_styles: PIPELINE_JS = { 'application': { - # Application will contain all paths not in courseware_only_js + # Application will contain all paths not in courseware_only_js or + # discussion_js or staff_grading_js 'source_filenames': [ pth.replace(COMMON_ROOT / 'static/', '') for pth @@ -476,7 +480,9 @@ PIPELINE_JS = { ] + [ pth.replace(PROJECT_ROOT / 'static/', '') for pth in sorted(glob2.glob(PROJECT_ROOT / 'static/coffee/src/**/*.coffee'))\ - if pth not in courseware_only_js and pth not in discussion_js + if (pth not in courseware_only_js and + pth not in discussion_js and + pth not in staff_grading_js) ] + [ 'js/form.ext.js', 'js/my_courses_dropdown.js', @@ -505,7 +511,12 @@ PIPELINE_JS = { 'discussion' : { 'source_filenames': [pth.replace(PROJECT_ROOT / 'static/', '') for pth in discussion_js], 'output_filename': 'js/discussion.js' + }, + 'staff_grading' : { + 'source_filenames': [pth.replace(PROJECT_ROOT / 'static/', '') for pth in staff_grading_js], + 'output_filename': 'js/staff_grading.js' } + } PIPELINE_DISABLE_WRAPPER = True diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee new file mode 100644 index 0000000000..06c84a3867 --- /dev/null +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -0,0 +1,5 @@ +class @StaffGrading + constructor: -> + alert('hi!') + + \ No newline at end of file diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index b00fd935aa..d9d183c161 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -11,6 +11,7 @@ <%include file="/courseware/course_navigation.html" args="active_page='staff_grading'" /> <%block name="js_extra"> + <%static:js group='staff_grading'/>
From c7ab37bdabd0ea961837e20f158a3574f0d64bde Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 14 Nov 2012 14:32:43 -0500 Subject: [PATCH 28/89] initial js--called at page load --- .../coffee/src/staff_grading/staff_grading.coffee | 10 ++++++++-- 1 file changed, 8 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 06c84a3867..445b32b444 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -1,5 +1,11 @@ -class @StaffGrading +# wrap everything in a class in case we want to use inside xmodules later +class StaffGrading constructor: -> alert('hi!') - \ No newline at end of file + load: -> + alert('loading') + +# for now, just create an instance and load it... +grading = new StaffGrading +$(document).ready(grading.load) From aa786d138d8976057a005d4683139780c87aa42e Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 14 Nov 2012 20:46:02 -0500 Subject: [PATCH 29/89] move computers--adding test file --- .../src/staff_grading/staff_grading.coffee | 11 ++++++--- .../src/staff_grading/test_grading.html | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 lms/static/coffee/src/staff_grading/test_grading.html diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 445b32b444..fbaf1cbfa8 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -1,11 +1,16 @@ # wrap everything in a class in case we want to use inside xmodules later class StaffGrading constructor: -> - alert('hi!') + @submission_container = $('.submission-container') + @rubric_container = $('.rubric-container') + @submit_button = $('.submit-button') + @mock_backend = true + @load() + + load: -> alert('loading') # for now, just create an instance and load it... -grading = new StaffGrading -$(document).ready(grading.load) +$(document).ready(() -> new StaffGrading) diff --git a/lms/static/coffee/src/staff_grading/test_grading.html b/lms/static/coffee/src/staff_grading/test_grading.html new file mode 100644 index 0000000000..dd960b03ac --- /dev/null +++ b/lms/static/coffee/src/staff_grading/test_grading.html @@ -0,0 +1,24 @@ + + + + + + + + +

Staff grading

+ +
+ +
+ +
+
+ +
+ +
+ + + + From 3d0ef7580710aeceb3b86aeaef2040c0bbc53986 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 14 Nov 2012 21:57:11 -0500 Subject: [PATCH 30/89] Basic js implementation, with mocked backend --- .../src/staff_grading/staff_grading.coffee | 121 ++++++++++++++++-- .../src/staff_grading/test_grading.html | 10 +- 2 files changed, 118 insertions(+), 13 deletions(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index fbaf1cbfa8..4d754adfab 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -1,16 +1,121 @@ # wrap everything in a class in case we want to use inside xmodules later + +get_random_int: (min, max) -> + return Math.floor(Math.random() * (max - min + 1)) + min + +# states +state_grading = "have_data" +state_no_data = "no_data" +state_error = "error" + class StaffGrading - constructor: -> + constructor: (mock_backend) -> + @el = $('.staff-grading') + @ajax_url = @el.data('ajax_url') + + @error_container = $('.error-container') @submission_container = $('.submission-container') @rubric_container = $('.rubric-container') - @submit_button = $('.submit-button') - @mock_backend = true + @button = $('.submit-button') + @button.click @clicked + @state = state_no_data + + @mock_backend = mock_backend + if @mock_backend + @mock_cnt = 0 + + @get_next_submission() + + mock: (cmd, data) -> + # Return a mock response to cmd and data + @mock_cnt++ + if cmd == 'get_next' + response = + 'success': true + 'submission': 'submission! ' + @mock_cnt + 'rubric': 'A rubric!' + @mock_cnt + else if cmd == 'save_grade' + response = + 'success': true + 'submission': 'another submission! ' + @mock_cnt + 'rubric': 'A rubric!' + @mock_cnt + else + response = + 'success': false + 'error': 'Unknown command ' + cmd + return response + + + set_button_text: (text) -> + @button.prop('value', text) + + _post: (cmd, data, callback) -> + if @mock_backend + callback(@mock(cmd, data)) + else + # TODO: replace with postWithPrefix when that's loaded + $.post(@ajax_url + cmd, data, callback) + + + ajax_callback: (response) => + if response.success + if response.submission + @data_loaded(response.submission, response.rubric) + else + @no_more() + else + @error(response.error) + + get_next_submission: () -> + @_post('get_next', {}, @ajax_callback) + + submit_and_get_next: () -> + data = {eval: '123'} + @_post('save_grade', data, @ajax_callback) + + error: (msg) -> + @error_container.html(msg) + @state = state_error + @update() + + data_loaded: (submission, rubric) -> + @submission_container.html(submission) + @rubric_container.html(rubric) + @state = state_grading + @update() + + no_more: () -> + @submission_container.html(submission) + @rubric_container.html(rubric) + @state = state_no_data + @update() + + update: () -> + # make button state and actions right + if @state == state_error + @set_button_text('Try loading again') + else if @state == state_grading + @set_button_text('Submit') + else if @state == state_no_data + @set_button_text('Re-check for submissions') + else + @error('System got into invalid state ' + @state) + + clicked: (event) => + event.preventDefault() + if @state == state_error + @error_container.html('') + @get_next_submission() + else if @state == state_grading + @submit_and_get_next() + else if @state == state_no_data + @get_next_submission() + else + @error('System got into invalid state ' + @state) + - @load() - - load: -> - alert('loading') # for now, just create an instance and load it... -$(document).ready(() -> new StaffGrading) +mock_backend = true +$(document).ready(() -> new StaffGrading(mock_backend)) diff --git a/lms/static/coffee/src/staff_grading/test_grading.html b/lms/static/coffee/src/staff_grading/test_grading.html index dd960b03ac..07891e1c33 100644 --- a/lms/static/coffee/src/staff_grading/test_grading.html +++ b/lms/static/coffee/src/staff_grading/test_grading.html @@ -6,19 +6,19 @@ +

Staff grading

-
+
-
+
-
-
+
- +
From 546096e8a0a7fa22af779fd007b9a9829d380bac Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 15 Nov 2012 10:23:24 -0500 Subject: [PATCH 31/89] Split out mock backend, fix out-of-data bug --- .../src/staff_grading/staff_grading.coffee | 98 +++++++++++-------- 1 file changed, 59 insertions(+), 39 deletions(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 4d754adfab..22416a5276 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -8,10 +8,57 @@ state_grading = "have_data" state_no_data = "no_data" state_error = "error" +class StaffGradingBackend + constructor: (ajax_url, mock_backend) -> + @ajax_url = ajax_url + @mock_backend = mock_backend + if @mock_backend + @mock_cnt = 0 + + mock: (cmd, data) -> + # Return a mock response to cmd and data + @mock_cnt++ + if cmd == 'get_next' + response = + success: true + submission: 'submission! ' + @mock_cnt + rubric: 'A rubric!' + @mock_cnt + + else if cmd == 'save_grade' + response = + success: true + submission: 'another submission! ' + @mock_cnt + rubric: 'A rubric!' + @mock_cnt + else + response = + success: false + error: 'Unknown command ' + cmd + + if @mock_cnt % 5 == 0 + response = + success: true + message: 'No more submissions' + + + if @mock_cnt % 7 == 0 + response = + success: false + error: 'An error for testing' + + return response + + + post: (cmd, data, callback) -> + if @mock_backend + callback(@mock(cmd, data)) + else + # TODO: replace with postWithPrefix when that's loaded + $.post(@ajax_url + cmd, data, callback) + + class StaffGrading - constructor: (mock_backend) -> - @el = $('.staff-grading') - @ajax_url = @el.data('ajax_url') + constructor: (backend) -> + @backend = backend @error_container = $('.error-container') @submission_container = $('.submission-container') @@ -20,42 +67,12 @@ class StaffGrading @button.click @clicked @state = state_no_data - @mock_backend = mock_backend - if @mock_backend - @mock_cnt = 0 - @get_next_submission() - mock: (cmd, data) -> - # Return a mock response to cmd and data - @mock_cnt++ - if cmd == 'get_next' - response = - 'success': true - 'submission': 'submission! ' + @mock_cnt - 'rubric': 'A rubric!' + @mock_cnt - else if cmd == 'save_grade' - response = - 'success': true - 'submission': 'another submission! ' + @mock_cnt - 'rubric': 'A rubric!' + @mock_cnt - else - response = - 'success': false - 'error': 'Unknown command ' + cmd - return response - set_button_text: (text) -> @button.prop('value', text) - _post: (cmd, data, callback) -> - if @mock_backend - callback(@mock(cmd, data)) - else - # TODO: replace with postWithPrefix when that's loaded - $.post(@ajax_url + cmd, data, callback) - ajax_callback: (response) => if response.success @@ -67,11 +84,11 @@ class StaffGrading @error(response.error) get_next_submission: () -> - @_post('get_next', {}, @ajax_callback) + @backend.post('get_next', {}, @ajax_callback) submit_and_get_next: () -> data = {eval: '123'} - @_post('save_grade', data, @ajax_callback) + @backend.post('save_grade', data, @ajax_callback) error: (msg) -> @error_container.html(msg) @@ -85,8 +102,8 @@ class StaffGrading @update() no_more: () -> - @submission_container.html(submission) - @rubric_container.html(rubric) + @submission_container.html('') + @rubric_container.html('') @state = state_no_data @update() @@ -117,5 +134,8 @@ class StaffGrading # for now, just create an instance and load it... -mock_backend = true -$(document).ready(() -> new StaffGrading(mock_backend)) +mock_backend = true +ajax_url = $('.staff-grading').data('ajax_url') +backend = new StaffGradingBackend(ajax_url, mock_backend) + +$(document).ready(() -> new StaffGrading(backend)) From b1d5273a2a10b828a137714d2e03a4e0fe4f9aea Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 15 Nov 2012 10:45:55 -0500 Subject: [PATCH 32/89] add messages, headers for sections --- .../src/staff_grading/staff_grading.coffee | 25 ++++++++++++++----- .../src/staff_grading/test_grading.html | 22 ++++++++++++---- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 22416a5276..e36e70151f 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -61,12 +61,18 @@ class StaffGrading @backend = backend @error_container = $('.error-container') + @message_container = $('.message-container') @submission_container = $('.submission-container') @rubric_container = $('.rubric-container') + @submission_wrapper = $('.submission-wrapper') + @rubric_wrapper = $('.rubric-wrapper') @button = $('.submit-button') @button.click @clicked @state = state_no_data + @submission_wrapper.hide() + @rubric_wrapper.hide() + @get_next_submission() @@ -102,26 +108,35 @@ class StaffGrading @update() no_more: () -> - @submission_container.html('') - @rubric_container.html('') @state = state_no_data @update() update: () -> - # make button state and actions right + # make button and div state match the state. Idempotent. if @state == state_error @set_button_text('Try loading again') + else if @state == state_grading + @submission_wrapper.show() + @rubric_wrapper.show() @set_button_text('Submit') + else if @state == state_no_data + @submission_wrapper.hide() + @rubric_wrapper.hide() + @message_container.html('Nothing to grade') @set_button_text('Re-check for submissions') + else @error('System got into invalid state ' + @state) clicked: (event) => event.preventDefault() + # always clear out errors and messages on transition... + @message_container.html('') + @error_container.html('') + if @state == state_error - @error_container.html('') @get_next_submission() else if @state == state_grading @submit_and_get_next() @@ -131,8 +146,6 @@ class StaffGrading @error('System got into invalid state ' + @state) - - # for now, just create an instance and load it... mock_backend = true ajax_url = $('.staff-grading').data('ajax_url') diff --git a/lms/static/coffee/src/staff_grading/test_grading.html b/lms/static/coffee/src/staff_grading/test_grading.html index 07891e1c33..780515a752 100644 --- a/lms/static/coffee/src/staff_grading/test_grading.html +++ b/lms/static/coffee/src/staff_grading/test_grading.html @@ -1,7 +1,7 @@ - + @@ -9,11 +9,23 @@

Staff grading

-
- -
+
+
-
+
+
+ +
+

Submission

+
+
+
+ +
+

Rubric

+
+
+
From e1fd6d73b3ebd80fb695d0d2d775506fdacf96e3 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 15 Nov 2012 13:35:53 -0500 Subject: [PATCH 33/89] refactor to be more clearly model-view --- .../src/staff_grading/staff_grading.coffee | 83 ++++++++++++------- .../src/staff_grading/test_grading.html | 1 + 2 files changed, 55 insertions(+), 29 deletions(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index e36e70151f..827f9fe56e 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -25,6 +25,7 @@ class StaffGradingBackend rubric: 'A rubric!' + @mock_cnt else if cmd == 'save_grade' + console.log("eval: #{data.score} pts, Feedback: #{data.feedback}") response = success: true submission: 'another submission! ' + @mock_cnt @@ -60,6 +61,7 @@ class StaffGrading constructor: (backend) -> @backend = backend + # all the jquery selectors @error_container = $('.error-container') @message_container = $('.message-container') @submission_container = $('.submission-container') @@ -67,75 +69,98 @@ class StaffGrading @submission_wrapper = $('.submission-wrapper') @rubric_wrapper = $('.rubric-wrapper') @button = $('.submit-button') - @button.click @clicked + + # model state @state = state_no_data + @submission = '' + @rubric = '' + @error_msg = '' + @message = '' - @submission_wrapper.hide() - @rubric_wrapper.hide() + @feedback = null + @score = null + # action handlers + @button.click @clicked + + # render intial state + @render_view() + + # send initial request automatically @get_next_submission() set_button_text: (text) -> @button.prop('value', text) - ajax_callback: (response) => - if response.success - if response.submission - @data_loaded(response.submission, response.rubric) - else - @no_more() + # always clear out errors and messages on transition. + @error_msg = '' + @message = '' + + if response.success + if response.submission + @data_loaded(response.submission, response.rubric) else - @error(response.error) - + @no_more() + else + @error(response.error) + + @render_view() + get_next_submission: () -> @backend.post('get_next', {}, @ajax_callback) submit_and_get_next: () -> - data = {eval: '123'} + data = {score: '1', feedback: 'Great!'} + @backend.post('save_grade', data, @ajax_callback) error: (msg) -> - @error_container.html(msg) + @error_msg = msg @state = state_error - @update() data_loaded: (submission, rubric) -> - @submission_container.html(submission) - @rubric_container.html(rubric) + @submission = submission + @rubric = rubric @state = state_grading - @update() no_more: () -> + @submission = null + @rubric = null + @message = 'Nothing to grade' @state = state_no_data - @update() - update: () -> - # make button and div state match the state. Idempotent. + render_view: () -> + # make the view elements match the state. Idempotent. + show_grading_elements = false + + @message_container.html(@message) + @error_container.html(@error_msg) + if @state == state_error @set_button_text('Try loading again') else if @state == state_grading - @submission_wrapper.show() - @rubric_wrapper.show() + @submission_container.html(@submission) + @rubric_container.html(@rubric) + show_grading_elements = true @set_button_text('Submit') else if @state == state_no_data - @submission_wrapper.hide() - @rubric_wrapper.hide() - @message_container.html('Nothing to grade') + @message_container.html(@message) @set_button_text('Re-check for submissions') else @error('System got into invalid state ' + @state) + @submission_wrapper.toggle(show_grading_elements) + @rubric_wrapper.toggle(show_grading_elements) + + clicked: (event) => event.preventDefault() - # always clear out errors and messages on transition... - @message_container.html('') - @error_container.html('') - + if @state == state_error @get_next_submission() else if @state == state_grading diff --git a/lms/static/coffee/src/staff_grading/test_grading.html b/lms/static/coffee/src/staff_grading/test_grading.html index 780515a752..8e69c90866 100644 --- a/lms/static/coffee/src/staff_grading/test_grading.html +++ b/lms/static/coffee/src/staff_grading/test_grading.html @@ -25,6 +25,7 @@

Rubric

+
From e910f60e08b2418fa118680ff785e5795f66e036 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 15 Nov 2012 14:27:46 -0500 Subject: [PATCH 34/89] wip --- .../src/staff_grading/staff_grading.coffee | 18 +++++++++++------- .../coffee/src/staff_grading/test_grading.html | 12 ++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 827f9fe56e..aded7464e7 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -22,7 +22,7 @@ class StaffGradingBackend response = success: true submission: 'submission! ' + @mock_cnt - rubric: 'A rubric!' + @mock_cnt + rubric: 'A rubric! ' + @mock_cnt else if cmd == 'save_grade' console.log("eval: #{data.score} pts, Feedback: #{data.feedback}") @@ -68,7 +68,8 @@ class StaffGrading @rubric_container = $('.rubric-container') @submission_wrapper = $('.submission-wrapper') @rubric_wrapper = $('.rubric-wrapper') - @button = $('.submit-button') + @feedback_area = $('.feedback-area') + @submit_button = $('.submit-button') # model state @state = state_no_data @@ -77,11 +78,12 @@ class StaffGrading @error_msg = '' @message = '' - @feedback = null @score = null # action handlers - @button.click @clicked + @submit_button.click @submit + @correct_button.click () => @score = 1 + @incorrect_button.click () => @score = 0 # render intial state @render_view() @@ -91,7 +93,7 @@ class StaffGrading set_button_text: (text) -> - @button.prop('value', text) + @submit_button.prop('value', text) ajax_callback: (response) => # always clear out errors and messages on transition. @@ -112,7 +114,7 @@ class StaffGrading @backend.post('get_next', {}, @ajax_callback) submit_and_get_next: () -> - data = {score: '1', feedback: 'Great!'} + data = {score: @score, feedback: @feedback_area.val()} @backend.post('save_grade', data, @ajax_callback) @@ -123,6 +125,8 @@ class StaffGrading data_loaded: (submission, rubric) -> @submission = submission @rubric = rubric + @feedback_area.val('') + @score = null @state = state_grading no_more: () -> @@ -158,7 +162,7 @@ class StaffGrading @rubric_wrapper.toggle(show_grading_elements) - clicked: (event) => + submit: (event) => event.preventDefault() if @state == state_error diff --git a/lms/static/coffee/src/staff_grading/test_grading.html b/lms/static/coffee/src/staff_grading/test_grading.html index 8e69c90866..b417aa41d9 100644 --- a/lms/static/coffee/src/staff_grading/test_grading.html +++ b/lms/static/coffee/src/staff_grading/test_grading.html @@ -26,6 +26,18 @@
+
+ +

+ + + + + +

+
+
From 5836438454225cc8c2297d43e490001f7784ce57 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 15 Nov 2012 19:23:07 -0500 Subject: [PATCH 35/89] hook up radio buttons, hide submit button till after grading --- .../src/staff_grading/staff_grading.coffee | 34 +++++++++++++++---- .../src/staff_grading/test_grading.html | 6 ++-- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index aded7464e7..b252e7a365 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -4,7 +4,8 @@ get_random_int: (min, max) -> return Math.floor(Math.random() * (max - min + 1)) + min # states -state_grading = "have_data" +state_grading = "grading" +state_graded = "graded" state_no_data = "no_data" state_error = "error" @@ -82,8 +83,10 @@ class StaffGrading # action handlers @submit_button.click @submit - @correct_button.click () => @score = 1 - @incorrect_button.click () => @score = 0 + # TODO: hook up an event to the input changing, which updates + # @score (instead of the individual hacks) + $('#correct-radio').click @graded_callback + $('#incorrect-radio').click @graded_callback # render intial state @render_view() @@ -92,8 +95,13 @@ class StaffGrading @get_next_submission() - set_button_text: (text) -> - @submit_button.prop('value', text) + set_button_text: (text) => + @submit_button.attr('value', text) + + graded_callback: (event) => + @score = event.target.value + @state = state_graded + @render_view() ajax_callback: (response) => # always clear out errors and messages on transition. @@ -138,6 +146,7 @@ class StaffGrading render_view: () -> # make the view elements match the state. Idempotent. show_grading_elements = false + show_submit_button = true @message_container.html(@message) @error_container.html(@error_msg) @@ -149,6 +158,16 @@ class StaffGrading @submission_container.html(@submission) @rubric_container.html(@rubric) show_grading_elements = true + + # no submit button until user picks grade. + show_submit_button = false + + # TODO: clean up with proper input-related logic + $('#correct-radio')[0].checked = false + $('#incorrect-radio')[0].checked = false + + else if @state == state_graded + show_grading_elements = true @set_button_text('Submit') else if @state == state_no_data @@ -158,6 +177,7 @@ class StaffGrading else @error('System got into invalid state ' + @state) + @submit_button.toggle(show_submit_button) @submission_wrapper.toggle(show_grading_elements) @rubric_wrapper.toggle(show_grading_elements) @@ -167,12 +187,12 @@ class StaffGrading if @state == state_error @get_next_submission() - else if @state == state_grading + else if @state == state_graded @submit_and_get_next() else if @state == state_no_data @get_next_submission() else - @error('System got into invalid state ' + @state) + @error('System got into invalid state for submission: ' + @state) # for now, just create an instance and load it... diff --git a/lms/static/coffee/src/staff_grading/test_grading.html b/lms/static/coffee/src/staff_grading/test_grading.html index b417aa41d9..8a1cde1fd4 100644 --- a/lms/static/coffee/src/staff_grading/test_grading.html +++ b/lms/static/coffee/src/staff_grading/test_grading.html @@ -1,7 +1,8 @@ - + + @@ -32,8 +33,7 @@

- - +

From 9d8d6655c685597ada8885e28a889a92d4fe288e Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 19 Nov 2012 09:10:02 -0500 Subject: [PATCH 36/89] add docstring for expect_json decorator --- common/djangoapps/util/json_request.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/common/djangoapps/util/json_request.py b/common/djangoapps/util/json_request.py index 169a7e3fb4..9458bff858 100644 --- a/common/djangoapps/util/json_request.py +++ b/common/djangoapps/util/json_request.py @@ -4,6 +4,11 @@ import json def expect_json(view_function): + """ + View decorator for simplifying handing of requests that expect json. If the request's + CONTENT_TYPE is application/json, parses the json dict from request.body, and updates + request.POST with the contents. + """ @wraps(view_function) def expect_json_with_cloned_request(request, *args, **kwargs): # cdodge: fix postback errors in CMS. The POST 'content-type' header can include additional information From ed6a8f68ac7c422dfd198ea1a582d186d4a3da53 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 19 Nov 2012 10:04:56 -0500 Subject: [PATCH 37/89] starting to stub out the backend staff grading service [wip] --- .../instructor/staff_grading_service.py | 57 +++++++++++++++++++ lms/urls.py | 4 ++ 2 files changed, 61 insertions(+) create mode 100644 lms/djangoapps/instructor/staff_grading_service.py diff --git a/lms/djangoapps/instructor/staff_grading_service.py b/lms/djangoapps/instructor/staff_grading_service.py new file mode 100644 index 0000000000..d4a4d82e63 --- /dev/null +++ b/lms/djangoapps/instructor/staff_grading_service.py @@ -0,0 +1,57 @@ +""" +This module provides views that proxy to the staff grading backend service. +""" + +import json +import requests +import sys + +from django.http import Http404 +from django.http import HttpResponse + + +from util.json_request import expect_json + +class GradingServiceError(Exception): + pass + +class StaffGradingService(object): + """ + Interface to staff grading backend. + """ + def __init__(self, url): + self.url = url + # TODO: add auth + self.session = requests.session() + + def get_next(course_id): + """ + Get the next thing to grade. Returns json, or raises GradingServiceError + if there's a problem. + """ + try: + r = self.session.get(url + 'get_next') + except requests.exceptions.ConnectionError as err: + # reraise as promised GradingServiceError, but preserve stacktrace. + raise GradingServiceError, str(err), sys.exc_info()[2] + + return r.text + + +#@login_required +def get_next(request, course_id): + """ + """ + d = {'success': False} + return HttpResponse(json.dumps(d)) + + +#@login_required +@expect_json +def save_grade(request, course_id): + """ + + """ + d = {'success': False} + return HttpResponse(json.dumps(d)) + diff --git a/lms/urls.py b/lms/urls.py index 7c1ccb6bde..0b2def9135 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -232,6 +232,10 @@ if settings.COURSEWARE_ENABLED: 'instructor.views.enroll_students', name='enroll_students'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/staff_grading$', 'instructor.views.staff_grading', name='staff_grading'), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/staff_grading/get_next$', + 'instructor.staff_grading_service.get_next', name='staff_grading_get_next'), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/staff_grading/save_grade$', + 'instructor.staff_grading_service.save_grade', name='staff_grading_save_grade'), ) # discussion forums live within courseware, so courseware must be enabled first From 4f359ea59412c7f1aba2b25912075dbcc8f61779 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 19 Nov 2012 13:38:21 -0500 Subject: [PATCH 38/89] pass through message when no more submissions. --- lms/static/coffee/src/staff_grading/staff_grading.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index b252e7a365..e410a3c41d 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -112,7 +112,7 @@ class StaffGrading if response.submission @data_loaded(response.submission, response.rubric) else - @no_more() + @no_more(response.message) else @error(response.error) @@ -137,10 +137,10 @@ class StaffGrading @score = null @state = state_grading - no_more: () -> + no_more: (message) -> @submission = null @rubric = null - @message = 'Nothing to grade' + @message = message @state = state_no_data render_view: () -> From 5bf39fef962c1e94e1f616074d31cc57a710e784 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 19 Nov 2012 15:23:05 -0500 Subject: [PATCH 39/89] Cherry pick the test-cleanup parts of e2826cb. - look up test courses by id, not name --- .../xmodule/xmodule/modulestore/__init__.py | 13 ++++++++++ lms/djangoapps/courseware/tests/tests.py | 19 ++++---------- lms/djangoapps/instructor/tests.py | 25 ++++++++++--------- lms/djangoapps/instructor/views.py | 4 +-- 4 files changed, 33 insertions(+), 28 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index fa8cf8d3d7..5b94add68f 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -339,6 +339,12 @@ class ModuleStore(object): ''' raise NotImplementedError + def get_course(self, course_id): + ''' + Look for a specific course id. Returns the course descriptor, or None if not found. + ''' + raise NotImplementedError + def get_parent_locations(self, location): '''Find all locations that are the parents of this location. Needed for path_to_location(). @@ -399,3 +405,10 @@ class ModuleStoreBase(ModuleStore): errorlog = self._get_errorlog(location) return errorlog.errors + + def get_course(self, course_id): + """Default impl--linear search through course list""" + for c in self.get_courses(): + if c.id == course_id: + return c + return None diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 8239eadfd9..1eebf0f408 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -288,14 +288,10 @@ class TestNavigation(PageLoader): def setUp(self): xmodule.modulestore.django._MODULESTORES = {} - courses = modulestore().get_courses() - def find_course(course_id): - """Assumes the course is present""" - return [c for c in courses if c.id==course_id][0] - - self.full = find_course("edX/full/6.002_Spring_2012") - self.toy = find_course("edX/toy/2012_Fall") + # Assume courses are there + self.full = modulestore().get_course("edX/full/6.002_Spring_2012") + self.toy = modulestore().get_course("edX/toy/2012_Fall") # Create two accounts self.student = 'view@test.com' @@ -346,14 +342,9 @@ class TestViewAuth(PageLoader): def setUp(self): xmodule.modulestore.django._MODULESTORES = {} - courses = modulestore().get_courses() - def find_course(course_id): - """Assumes the course is present""" - return [c for c in courses if c.id==course_id][0] - - self.full = find_course("edX/full/6.002_Spring_2012") - self.toy = find_course("edX/toy/2012_Fall") + self.full = modulestore().get_course("edX/full/6.002_Spring_2012") + self.toy = modulestore().get_course("edX/toy/2012_Fall") # Create two accounts self.student = 'view@test.com' diff --git a/lms/djangoapps/instructor/tests.py b/lms/djangoapps/instructor/tests.py index 532c0c3f68..55e63c8dc2 100644 --- a/lms/djangoapps/instructor/tests.py +++ b/lms/djangoapps/instructor/tests.py @@ -33,12 +33,8 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader): xmodule.modulestore.django._MODULESTORES = {} courses = modulestore().get_courses() - def find_course(name): - """Assumes the course is present""" - return [c for c in courses if c.location.course==name][0] - - self.full = find_course("full") - self.toy = find_course("toy") + self.full = modulestore().get_course("edX/full/6.002_Spring_2012") + self.toy = modulestore().get_course("edX/toy/2012_Fall") # Create two accounts self.student = 'view@test.com' @@ -49,9 +45,12 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader): self.activate_user(self.student) self.activate_user(self.instructor) - group_name = _course_staff_group_name(self.toy.location) - g = Group.objects.create(name=group_name) - g.user_set.add(ct.user(self.instructor)) + def make_instructor(course): + group_name = _course_staff_group_name(course.location) + g = Group.objects.create(name=group_name) + g.user_set.add(ct.user(self.instructor)) + + make_instructor(self.toy) self.logout() self.login(self.instructor, self.password) @@ -67,9 +66,9 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader): self.assertEqual(response['Content-Type'],'text/csv',msg) - cdisp = response['Content-Disposition'].replace('TT_2012','2012') # jenkins course_id is TT_2012_Fall instead of 2012_Fall? - msg += "cdisp = '{0}'\n".format(cdisp) - self.assertEqual(cdisp,'attachment; filename=grades_edX/toy/2012_Fall.csv',msg) + cdisp = response['Content-Disposition'] + msg += "Content-Disposition = '%s'\n" % cdisp + self.assertEqual(cdisp, 'attachment; filename=grades_{0}.csv'.format(course.id), msg) body = response.content.replace('\r','') msg += "body = '{0}'\n".format(body) @@ -77,6 +76,8 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader): expected_body = '''"ID","Username","Full Name","edX email","External email","HW 01","HW 02","HW 03","HW 04","HW 05","HW 06","HW 07","HW 08","HW 09","HW 10","HW 11","HW 12","HW Avg","Lab 01","Lab 02","Lab 03","Lab 04","Lab 05","Lab 06","Lab 07","Lab 08","Lab 09","Lab 10","Lab 11","Lab 12","Lab Avg","Midterm","Final" "2","u2","Fred Weasley","view2@test.com","","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0.0","0.0" ''' + # All the not-actually-in-the-course hw and labs come from the + # default grading policy string in graders.py self.assertEqual(body, expected_body, msg) FORUM_ROLES = [ FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA ] diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 8a33f1d60b..b877a7236d 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -90,7 +90,7 @@ def instructor_dashboard(request, course_id): try: group = Group.objects.get(name=staffgrp) except Group.DoesNotExist: - group = Group(name=staffgrp) # create the group + group = Group(name=staffgrp) # create the group group.save() return group @@ -380,7 +380,7 @@ def get_student_grade_summary_data(request, course, course_id, get_grades=True, enrolled_students = User.objects.filter(courseenrollment__course_id=course_id).prefetch_related("groups").order_by('username') header = ['ID', 'Username', 'Full Name', 'edX email', 'External email'] - if get_grades: + if get_grades and enrolled_students.count() > 0: # just to construct the header gradeset = grades.grade(enrolled_students[0], request, course, keep_raw_scores=get_raw_scores) # log.debug('student {0} gradeset {1}'.format(enrolled_students[0], gradeset)) From 4b8708c2e4278bae5f005f212c79983f0a8f5a88 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 19 Nov 2012 15:24:11 -0500 Subject: [PATCH 40/89] move a comment --- lms/djangoapps/instructor/tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/instructor/tests.py b/lms/djangoapps/instructor/tests.py index 55e63c8dc2..4f8ac140b0 100644 --- a/lms/djangoapps/instructor/tests.py +++ b/lms/djangoapps/instructor/tests.py @@ -73,11 +73,11 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader): body = response.content.replace('\r','') msg += "body = '{0}'\n".format(body) + # All the not-actually-in-the-course hw and labs come from the + # default grading policy string in graders.py expected_body = '''"ID","Username","Full Name","edX email","External email","HW 01","HW 02","HW 03","HW 04","HW 05","HW 06","HW 07","HW 08","HW 09","HW 10","HW 11","HW 12","HW Avg","Lab 01","Lab 02","Lab 03","Lab 04","Lab 05","Lab 06","Lab 07","Lab 08","Lab 09","Lab 10","Lab 11","Lab 12","Lab Avg","Midterm","Final" "2","u2","Fred Weasley","view2@test.com","","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0.0","0.0" ''' - # All the not-actually-in-the-course hw and labs come from the - # default grading policy string in graders.py self.assertEqual(body, expected_body, msg) FORUM_ROLES = [ FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA ] From 055aeae0b6b75bf31c733a0f618a24f319f92c74 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 19 Nov 2012 15:47:49 -0500 Subject: [PATCH 41/89] fix comment in access.py --- lms/djangoapps/courseware/access.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 00b4c763b3..ba9b8a3bc0 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -34,7 +34,8 @@ def has_access(user, obj, action): user: a Django user object. May be anonymous. - obj: The object to check access for. For now, a module or descriptor. + obj: The object to check access for. A module, descriptor, location, or + certain special strings (e.g. 'global') action: A string specifying the action that the client is trying to perform. From ff1192657015e67f0f057309842fa094d867a2e7 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 19 Nov 2012 15:48:38 -0500 Subject: [PATCH 42/89] Initial impl and basic access tests for staff grading service --- lms/djangoapps/courseware/tests/tests.py | 14 +- .../instructor/staff_grading_service.py | 120 +++++++++++++++++- lms/djangoapps/instructor/tests.py | 54 ++++++-- lms/envs/common.py | 4 + 4 files changed, 175 insertions(+), 17 deletions(-) diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 1eebf0f408..eb026e9c6b 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -221,8 +221,7 @@ class PageLoader(ActivateLoginTestCase): def check_for_get_code(self, code, url): """ - Check that we got the expected code. Hacks around our broken 404 - handling. + Check that we got the expected code when accessing url via GET. """ resp = self.client.get(url) self.assertEqual(resp.status_code, code, @@ -230,6 +229,17 @@ class PageLoader(ActivateLoginTestCase): .format(resp.status_code, url, code)) + def check_for_post_code(self, code, url, data={}): + """ + Check that we got the expected code when accessing url via POST. + """ + resp = self.client.post(url, data) + self.assertEqual(resp.status_code, code, + "got code {0} for url '{1}'. Expected code {2}" + .format(resp.status_code, url, code)) + + + def check_pages_load(self, course_name, data_dir, modstore): """Make all locations in course load""" print "Checking course {0} in {1}".format(course_name, data_dir) diff --git a/lms/djangoapps/instructor/staff_grading_service.py b/lms/djangoapps/instructor/staff_grading_service.py index d4a4d82e63..e4f82cd5e0 100644 --- a/lms/djangoapps/instructor/staff_grading_service.py +++ b/lms/djangoapps/instructor/staff_grading_service.py @@ -3,14 +3,20 @@ This module provides views that proxy to the staff grading backend service. """ import json +import logging import requests import sys +from django.conf import settings from django.http import Http404 from django.http import HttpResponse - +from courseware.access import has_access from util.json_request import expect_json +from xmodule.course_module import CourseDescriptor + +log = logging.getLogger("mitx.courseware") + class GradingServiceError(Exception): pass @@ -37,21 +43,121 @@ class StaffGradingService(object): return r.text + def save_grade(course_id, submission_id, score, feedback): + """ + Save a grade. + + TODO: what is data? + + Returns json, or raises GradingServiceError if there's a problem. + """ + try: + r = self.session.get(url + 'save_grade') + except requests.exceptions.ConnectionError as err: + # reraise as promised GradingServiceError, but preserve stacktrace. + raise GradingServiceError, str(err), sys.exc_info()[2] + + return r.text + +_service = StaffGradingService(settings.STAFF_GRADING_BACKEND_URL) + + +def _err_response(msg): + """ + Return a HttpResponse with a json dump with success=False, and the given error message. + """ + return HttpResponse(json.dumps({'success': False, 'error': msg})) + + +def _check_access(user, course_id): + """ + Raise 404 if user doesn't have staff access to course_id + """ + course_location = CourseDescriptor.id_to_location(course_id) + if not has_access(user, course_location, 'staff'): + raise Http404 + + return + -#@login_required def get_next(request, course_id): """ + Get the next thing to grade for course_id. + + Returns a json dict with the following keys: + + 'success': bool + + 'submission_id': a unique identifier for the submission, to be passed back + with the grade. + + 'submission': the submission, rendered as read-only html for grading + + 'rubric': the rubric, also rendered as html. + + 'message': if there was no submission available, but nothing went wrong, + there will be a message field. + + 'error': if success is False, will have an error message with more info. """ - d = {'success': False} - return HttpResponse(json.dumps(d)) + _check_access(request.user, course_id) + + return HttpResponse(_get_next(course_id)) + + +def _get_next(course_id): + """ + Implementation of get_next (also called from save_grade) -- returns a json string + """ + + try: + return _service.get_next(course_id) + except GradingServiceError: + log.exception("Error from grading service") + return json.dumps({'success': False, 'error': 'Could not connect to grading service'}) -#@login_required @expect_json def save_grade(request, course_id): """ + Save the grade and feedback for a submission, and, if all goes well, return + the next thing to grade. + Expects the following POST parameters: + 'score': int + 'feedback': string + 'submission_id': int + + Returns the same thing as get_next, except that additional error messages + are possible if something goes wrong with saving the grade. """ - d = {'success': False} - return HttpResponse(json.dumps(d)) + _check_access(request.user, course_id) + + if request.method != 'POST': + raise Http404 + + required = ('score', 'feedback', 'submission_id') + for k in required: + if k not in request.POST.keys(): + return _err_response('Missing required key {0}'.format(k)) + + p = request.POST + + try: + result_json = _service.save_grade(course_id, p['submission_id'], p['score'], p['feedback']) + except GradingServiceError: + log.exception("Error saving grade") + return _err_response('Could not connect to grading service') + + try: + result = json.loads(result_json) + except ValueError: + return _err_response('Grading service returned mal-formatted data.') + + if not result.get('success', False): + log.warning('Got success=False from grading service. Response: %s', result_json) + return _err_response('Grading service failed') + + # Ok, save_grade seemed to work. Get the next submission to grade. + return HttpResponse(_get_next(course_id)) diff --git a/lms/djangoapps/instructor/tests.py b/lms/djangoapps/instructor/tests.py index 4f8ac140b0..5f740b8ff9 100644 --- a/lms/djangoapps/instructor/tests.py +++ b/lms/djangoapps/instructor/tests.py @@ -31,7 +31,6 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader): def setUp(self): xmodule.modulestore.django._MODULESTORES = {} - courses = modulestore().get_courses() self.full = modulestore().get_course("edX/full/6.002_Spring_2012") self.toy = modulestore().get_course("edX/toy/2012_Fall") @@ -79,6 +78,7 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader): "2","u2","Fred Weasley","view2@test.com","","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0.0","0.0" ''' self.assertEqual(body, expected_body, msg) + FORUM_ROLES = [ FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA ] FORUM_ADMIN_ACTION_SUFFIX = { FORUM_ROLE_ADMINISTRATOR : 'admin', FORUM_ROLE_MODERATOR : 'moderator', FORUM_ROLE_COMMUNITY_TA : 'community TA'} @@ -90,22 +90,19 @@ def action_name(operation, rolename): else: return '{0} forum {1}'.format(operation, FORUM_ADMIN_ACTION_SUFFIX[rolename]) + @override_settings(MODULESTORE=ct.TEST_DATA_XML_MODULESTORE) class TestInstructorDashboardForumAdmin(ct.PageLoader): ''' Check for change in forum admin role memberships ''' - + def setUp(self): xmodule.modulestore.django._MODULESTORES = {} courses = modulestore().get_courses() - def find_course(name): - """Assumes the course is present""" - return [c for c in courses if c.location.course==name][0] - - self.full = find_course("full") - self.toy = find_course("toy") + self.full = modulestore().get_course("edX/full/6.002_Spring_2012") + self.toy = modulestore().get_course("edX/toy/2012_Fall") # Create two accounts self.student = 'view@test.com' @@ -124,6 +121,8 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader): self.login(self.instructor, self.password) self.enroll(self.toy) + + def initialize_roles(self, course_id): self.admin_role = Role.objects.get_or_create(name=FORUM_ROLE_ADMINISTRATOR, course_id=course_id)[0] self.moderator_role = Role.objects.get_or_create(name=FORUM_ROLE_MODERATOR, course_id=course_id)[0] @@ -210,3 +209,42 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader): added_roles.sort() roles = ', '.join(added_roles) self.assertTrue(response.content.find('{0}'.format(roles))>=0, 'not finding roles "{0}"'.format(roles)) + + +@override_settings(MODULESTORE=ct.TEST_DATA_XML_MODULESTORE) +class TestStaffGradingService(ct.PageLoader): + ''' + Check that staff grading service proxy works. Basically just checking the + access control and error handling logic -- all the actual work is on the + backend. + ''' + + + + def setUp(self): + xmodule.modulestore.django._MODULESTORES = {} + + self.course_id = "edX/toy/2012_Fall" + self.toy = modulestore().get_course(self.course_id) + def make_instructor(course): + group_name = _course_staff_group_name(course.location) + g = Group.objects.create(name=group_name) + g.user_set.add(ct.user(self.instructor)) + + make_instructor(self.toy) + + self.logout() + + def test_access(self): + """ + Make sure only staff have access. + """ + self.login(self.student, self.password) + self.enroll(self.toy) + + # both get and post should return 404 + for view_name in ('staff_grading_get_next', 'staff_grading_save_grade'): + url = reverse(view_name, kwargs={'course_id': self.course_id}) + self.check_for_get_code(404, url) + self.check_for_post_code(404, url) + diff --git a/lms/envs/common.py b/lms/envs/common.py index 008de7ac84..7ddd2ffb9a 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -322,6 +322,10 @@ WIKI_USE_BOOTSTRAP_SELECT_WIDGET = False WIKI_LINK_LIVE_LOOKUPS = False WIKI_LINK_DEFAULT_LEVEL = 2 +################################# Staff grading config ##################### + +STAFF_GRADING_BACKEND_URL = None + ################################# Jasmine ################################### JASMINE_TEST_DIRECTORY = PROJECT_ROOT + '/static/coffee' From d0e2b85e3c0b876263d3e91da41364219f9a6a0b Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 19 Nov 2012 16:56:12 -0500 Subject: [PATCH 43/89] Refactor testing code, hook up frontend. - now getting requests from js to server and back, with mocked service. --- lms/djangoapps/courseware/tests/tests.py | 102 +++++++++--------- .../instructor/staff_grading_service.py | 31 ++++-- lms/djangoapps/instructor/tests.py | 58 ++++++++-- lms/djangoapps/instructor/views.py | 5 + .../src/staff_grading/staff_grading.coffee | 16 ++- lms/templates/instructor/staff_grading.html | 43 +++++++- 6 files changed, 187 insertions(+), 68 deletions(-) diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index eb026e9c6b..480a119b48 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -222,24 +222,28 @@ class PageLoader(ActivateLoginTestCase): def check_for_get_code(self, code, url): """ Check that we got the expected code when accessing url via GET. + Returns the response. """ resp = self.client.get(url) self.assertEqual(resp.status_code, code, "got code {0} for url '{1}'. Expected code {2}" .format(resp.status_code, url, code)) + return resp def check_for_post_code(self, code, url, data={}): """ Check that we got the expected code when accessing url via POST. + Returns the response. """ resp = self.client.post(url, data) self.assertEqual(resp.status_code, code, "got code {0} for url '{1}'. Expected code {2}" .format(resp.status_code, url, code)) + return resp + - def check_pages_load(self, course_name, data_dir, modstore): """Make all locations in course load""" print "Checking course {0} in {1}".format(course_name, data_dir) @@ -661,46 +665,46 @@ class TestCourseGrader(PageLoader): return [c for c in courses if c.id==course_id][0] self.graded_course = find_course("edX/graded/2012_Fall") - + # create a test student self.student = 'view@test.com' self.password = 'foo' self.create_account('u1', self.student, self.password) self.activate_user(self.student) self.enroll(self.graded_course) - + self.student_user = user(self.student) - + self.factory = RequestFactory() - + def get_grade_summary(self): student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( self.graded_course.id, self.student_user, self.graded_course) - - fake_request = self.factory.get(reverse('progress', - kwargs={'course_id': self.graded_course.id})) - - return grades.grade(self.student_user, fake_request, - self.graded_course, student_module_cache) - - def get_homework_scores(self): - return self.get_grade_summary()['totaled_scores']['Homework'] - - def get_progress_summary(self): - student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( - self.graded_course.id, self.student_user, self.graded_course) - + fake_request = self.factory.get(reverse('progress', kwargs={'course_id': self.graded_course.id})) - progress_summary = grades.progress_summary(self.student_user, fake_request, + return grades.grade(self.student_user, fake_request, + self.graded_course, student_module_cache) + + def get_homework_scores(self): + return self.get_grade_summary()['totaled_scores']['Homework'] + + def get_progress_summary(self): + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( + self.graded_course.id, self.student_user, self.graded_course) + + fake_request = self.factory.get(reverse('progress', + kwargs={'course_id': self.graded_course.id})) + + progress_summary = grades.progress_summary(self.student_user, fake_request, self.graded_course, student_module_cache) return progress_summary - + def check_grade_percent(self, percent): grade_summary = self.get_grade_summary() - self.assertEqual(percent, grade_summary['percent']) - + self.assertEqual(grade_summary['percent'], percent) + def submit_question_answer(self, problem_url_name, responses): """ The field names of a problem are hard to determine. This method only works @@ -710,96 +714,96 @@ class TestCourseGrader(PageLoader): input_i4x-edX-graded-problem-H1P3_2_2 """ problem_location = "i4x://edX/graded/problem/{0}".format(problem_url_name) - - modx_url = reverse('modx_dispatch', + + modx_url = reverse('modx_dispatch', kwargs={ 'course_id' : self.graded_course.id, 'location' : problem_location, 'dispatch' : 'problem_check', } ) - + resp = self.client.post(modx_url, { 'input_i4x-edX-graded-problem-{0}_2_1'.format(problem_url_name): responses[0], 'input_i4x-edX-graded-problem-{0}_2_2'.format(problem_url_name): responses[1], }) print "modx_url" , modx_url, "responses" , responses print "resp" , resp - + return resp - + def problem_location(self, problem_url_name): return "i4x://edX/graded/problem/{0}".format(problem_url_name) - + def reset_question_answer(self, problem_url_name): problem_location = self.problem_location(problem_url_name) - - modx_url = reverse('modx_dispatch', + + modx_url = reverse('modx_dispatch', kwargs={ 'course_id' : self.graded_course.id, 'location' : problem_location, 'dispatch' : 'problem_reset', } ) - + resp = self.client.post(modx_url) - return resp - + return resp + def test_get_graded(self): #### Check that the grader shows we have 0% in the course self.check_grade_percent(0) - + #### Submit the answers to a few problems as ajax calls def earned_hw_scores(): """Global scores, each Score is a Problem Set""" return [s.earned for s in self.get_homework_scores()] - + def score_for_hw(hw_url_name): hw_section = [section for section in self.get_progress_summary()[0]['sections'] if section.get('url_name') == hw_url_name][0] return [s.earned for s in hw_section['scores']] - + # Only get half of the first problem correct self.submit_question_answer('H1P1', ['Correct', 'Incorrect']) self.check_grade_percent(0.06) self.assertEqual(earned_hw_scores(), [1.0, 0, 0]) # Order matters self.assertEqual(score_for_hw('Homework1'), [1.0, 0.0]) - + # Get both parts of the first problem correct self.reset_question_answer('H1P1') self.submit_question_answer('H1P1', ['Correct', 'Correct']) self.check_grade_percent(0.13) self.assertEqual(earned_hw_scores(), [2.0, 0, 0]) self.assertEqual(score_for_hw('Homework1'), [2.0, 0.0]) - + # This problem is shown in an ABTest self.submit_question_answer('H1P2', ['Correct', 'Correct']) self.check_grade_percent(0.25) self.assertEqual(earned_hw_scores(), [4.0, 0.0, 0]) - self.assertEqual(score_for_hw('Homework1'), [2.0, 2.0]) - + self.assertEqual(score_for_hw('Homework1'), [2.0, 2.0]) + # This problem is hidden in an ABTest. Getting it correct doesn't change total grade self.submit_question_answer('H1P3', ['Correct', 'Correct']) self.check_grade_percent(0.25) self.assertEqual(score_for_hw('Homework1'), [2.0, 2.0]) - + # On the second homework, we only answer half of the questions. # Then it will be dropped when homework three becomes the higher percent # This problem is also weighted to be 4 points (instead of default of 2) - # If the problem was unweighted the percent would have been 0.38 so we + # If the problem was unweighted the percent would have been 0.38 so we # know it works. self.submit_question_answer('H2P1', ['Correct', 'Correct']) self.check_grade_percent(0.42) - self.assertEqual(earned_hw_scores(), [4.0, 4.0, 0]) - + self.assertEqual(earned_hw_scores(), [4.0, 4.0, 0]) + # Third homework self.submit_question_answer('H3P1', ['Correct', 'Correct']) self.check_grade_percent(0.42) # Score didn't change - self.assertEqual(earned_hw_scores(), [4.0, 4.0, 2.0]) - + self.assertEqual(earned_hw_scores(), [4.0, 4.0, 2.0]) + self.submit_question_answer('H3P2', ['Correct', 'Correct']) self.check_grade_percent(0.5) # Now homework2 dropped. Score changes - self.assertEqual(earned_hw_scores(), [4.0, 4.0, 4.0]) - + self.assertEqual(earned_hw_scores(), [4.0, 4.0, 4.0]) + # Now we answer the final question (worth half of the grade) self.submit_question_answer('FinalQuestion', ['Correct', 'Correct']) self.check_grade_percent(1.0) # Hooray! We got 100% diff --git a/lms/djangoapps/instructor/staff_grading_service.py b/lms/djangoapps/instructor/staff_grading_service.py index e4f82cd5e0..2a2a7a3552 100644 --- a/lms/djangoapps/instructor/staff_grading_service.py +++ b/lms/djangoapps/instructor/staff_grading_service.py @@ -21,6 +21,25 @@ log = logging.getLogger("mitx.courseware") class GradingServiceError(Exception): pass + +class MockStaffGradingService(object): + """ + A simple mockup of a staff grading service, testing. + """ + def __init__(self): + self.cnt = 0 + + def get_next(self, course_id): + self.cnt += 1 + return json.dumps({'success': True, + 'submission_id': self.cnt, + 'submission': 'Test submission {cnt}'.format(cnt=self.cnt), + 'rubric': 'A rubric'}) + + def save_grade(self, course_id, submission_id, score, feedback): + return self.get_next(course_id) + + class StaffGradingService(object): """ Interface to staff grading backend. @@ -30,20 +49,20 @@ class StaffGradingService(object): # TODO: add auth self.session = requests.session() - def get_next(course_id): + def get_next(self, course_id): """ Get the next thing to grade. Returns json, or raises GradingServiceError if there's a problem. """ try: - r = self.session.get(url + 'get_next') + r = self.session.get(self.url + 'get_next') except requests.exceptions.ConnectionError as err: # reraise as promised GradingServiceError, but preserve stacktrace. raise GradingServiceError, str(err), sys.exc_info()[2] return r.text - def save_grade(course_id, submission_id, score, feedback): + def save_grade(self, course_id, submission_id, score, feedback): """ Save a grade. @@ -52,15 +71,15 @@ class StaffGradingService(object): Returns json, or raises GradingServiceError if there's a problem. """ try: - r = self.session.get(url + 'save_grade') + r = self.session.get(self.url + 'save_grade') except requests.exceptions.ConnectionError as err: # reraise as promised GradingServiceError, but preserve stacktrace. raise GradingServiceError, str(err), sys.exc_info()[2] return r.text -_service = StaffGradingService(settings.STAFF_GRADING_BACKEND_URL) - +#_service = StaffGradingService(settings.STAFF_GRADING_BACKEND_URL) +_service = MockStaffGradingService() def _err_response(msg): """ diff --git a/lms/djangoapps/instructor/tests.py b/lms/djangoapps/instructor/tests.py index 5f740b8ff9..28a56ad9de 100644 --- a/lms/djangoapps/instructor/tests.py +++ b/lms/djangoapps/instructor/tests.py @@ -8,15 +8,24 @@ Notes for running by hand: django-admin.py test --settings=lms.envs.test --pythonpath=. lms/djangoapps/instructor """ +import courseware.tests.tests as ct + +import json + +from nose import SkipTest +from mock import patch, Mock + from override_settings import override_settings -from django.contrib.auth.models import \ - Group # Need access to internal func to put users in the right group +# Need access to internal func to put users in the right group +from django.contrib.auth.models import Group + from django.core.urlresolvers import reverse from django_comment_client.models import Role, FORUM_ROLE_ADMINISTRATOR, \ FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_STUDENT from django_comment_client.utils import has_forum_access +from instructor import staff_grading_service from courseware.access import _course_staff_group_name import courseware.tests.tests as ct from xmodule.modulestore.django import modulestore @@ -79,7 +88,7 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader): ''' self.assertEqual(body, expected_body, msg) - + FORUM_ROLES = [ FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA ] FORUM_ADMIN_ACTION_SUFFIX = { FORUM_ROLE_ADMINISTRATOR : 'admin', FORUM_ROLE_MODERATOR : 'moderator', FORUM_ROLE_COMMUNITY_TA : 'community TA'} FORUM_ADMIN_USER = { FORUM_ROLE_ADMINISTRATOR : 'forumadmin', FORUM_ROLE_MODERATOR : 'forummoderator', FORUM_ROLE_COMMUNITY_TA : 'forummoderator'} @@ -91,6 +100,8 @@ def action_name(operation, rolename): return '{0} forum {1}'.format(operation, FORUM_ADMIN_ACTION_SUFFIX[rolename]) +_mock_service = staff_grading_service.MockStaffGradingService() + @override_settings(MODULESTORE=ct.TEST_DATA_XML_MODULESTORE) class TestInstructorDashboardForumAdmin(ct.PageLoader): ''' @@ -101,8 +112,15 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader): xmodule.modulestore.django._MODULESTORES = {} courses = modulestore().get_courses() +<<<<<<< HEAD self.full = modulestore().get_course("edX/full/6.002_Spring_2012") self.toy = modulestore().get_course("edX/toy/2012_Fall") +======= + + + self.course_id = "edX/toy/2012_Fall" + self.toy = modulestore().get_course(self.course_id) +>>>>>>> Refactor testing code, hook up frontend. # Create two accounts self.student = 'view@test.com' @@ -122,7 +140,7 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader): self.enroll(self.toy) - + def initialize_roles(self, course_id): self.admin_role = Role.objects.get_or_create(name=FORUM_ROLE_ADMINISTRATOR, course_id=course_id)[0] self.moderator_role = Role.objects.get_or_create(name=FORUM_ROLE_MODERATOR, course_id=course_id)[0] @@ -220,7 +238,7 @@ class TestStaffGradingService(ct.PageLoader): ''' - + def setUp(self): xmodule.modulestore.django._MODULESTORES = {} @@ -240,7 +258,6 @@ class TestStaffGradingService(ct.PageLoader): Make sure only staff have access. """ self.login(self.student, self.password) - self.enroll(self.toy) # both get and post should return 404 for view_name in ('staff_grading_get_next', 'staff_grading_save_grade'): @@ -248,3 +265,32 @@ class TestStaffGradingService(ct.PageLoader): self.check_for_get_code(404, url) self.check_for_post_code(404, url) +<<<<<<< HEAD +======= + + @patch.object(staff_grading_service, '_service', _mock_service) + def test_get_next(self): + self.login(self.instructor, self.password) + + url = reverse('staff_grading_get_next', kwargs={'course_id': self.course_id}) + + r = self.check_for_get_code(200, url) + d = json.loads(r.content) + self.assertTrue(d['success']) + self.assertEquals(d['submission_id'], _mock_service.cnt) + + + @patch.object(staff_grading_service, '_service', _mock_service) + def test_save_grade(self): + self.login(self.instructor, self.password) + + url = reverse('staff_grading_save_grade', kwargs={'course_id': self.course_id}) + + data = {'score': '12', 'feedback': 'great!', 'submission_id': '123'} + r = self.check_for_post_code(200, url, data) + d = json.loads(r.content) + self.assertTrue(d['success'], str(d)) + self.assertEquals(d['submission_id'], _mock_service.cnt) + + +>>>>>>> Refactor testing code, hook up frontend. diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index b877a7236d..389a64721a 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -422,10 +422,15 @@ def staff_grading(request, course_id): grading = StaffGrading(course) + ajax_url = reverse('staff_grading', kwargs={'course_id': course_id}) + if not ajax_url.endswith('/'): + ajax_url += '/' + return render_to_response('instructor/staff_grading.html', { 'view_html': grading.get_html(), 'course': course, 'course_id': course_id, + 'ajax_url': ajax_url, # Checked above 'staff_access': True, }) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index e410a3c41d..7440277d98 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -24,6 +24,7 @@ class StaffGradingBackend success: true submission: 'submission! ' + @mock_cnt rubric: 'A rubric! ' + @mock_cnt + submission_id: @mock_cnt else if cmd == 'save_grade' console.log("eval: #{data.score} pts, Feedback: #{data.feedback}") @@ -31,6 +32,7 @@ class StaffGradingBackend success: true submission: 'another submission! ' + @mock_cnt rubric: 'A rubric!' + @mock_cnt + submission_id: @mock_cnt else response = success: false @@ -74,6 +76,7 @@ class StaffGrading # model state @state = state_no_data + @submission_id = null @submission = '' @rubric = '' @error_msg = '' @@ -110,7 +113,7 @@ class StaffGrading if response.success if response.submission - @data_loaded(response.submission, response.rubric) + @data_loaded(response.submission, response.rubric, response.submission_id) else @no_more(response.message) else @@ -122,7 +125,10 @@ class StaffGrading @backend.post('get_next', {}, @ajax_callback) submit_and_get_next: () -> - data = {score: @score, feedback: @feedback_area.val()} + data = + score: @score + feedback: @feedback_area.val() + submission_id: @submission_id @backend.post('save_grade', data, @ajax_callback) @@ -130,9 +136,10 @@ class StaffGrading @error_msg = msg @state = state_error - data_loaded: (submission, rubric) -> + data_loaded: (submission, rubric, submission_id) -> @submission = submission @rubric = rubric + @submission_id = submission_id @feedback_area.val('') @score = null @state = state_grading @@ -140,6 +147,7 @@ class StaffGrading no_more: (message) -> @submission = null @rubric = null + @submission_id = null @message = message @state = state_no_data @@ -196,7 +204,7 @@ class StaffGrading # for now, just create an instance and load it... -mock_backend = true +mock_backend = false ajax_url = $('.staff-grading').data('ajax_url') backend = new StaffGradingBackend(ajax_url, mock_backend) diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index d9d183c161..d8e834d4f6 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -15,7 +15,44 @@
-
- ${view_html} -
+ +
+

Staff grading

+ +
+
+ +
+
+ +
+

Submission

+
+
+
+ +
+

Rubric

+
+
+ +
+ +

+ + + + +

+
+ +
+ +
+ +
+ +
+
From a584f06bac1d03ca558491a40d9cadd9a5bf0ccf Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 19 Nov 2012 18:00:48 -0500 Subject: [PATCH 44/89] Add support for varying max_score. --- .../instructor/staff_grading_service.py | 26 +++++++--- lms/djangoapps/instructor/tests.py | 16 ++---- .../src/staff_grading/staff_grading.coffee | 50 +++++++++++++------ .../src/staff_grading/test_grading.html | 6 +-- 4 files changed, 57 insertions(+), 41 deletions(-) diff --git a/lms/djangoapps/instructor/staff_grading_service.py b/lms/djangoapps/instructor/staff_grading_service.py index 2a2a7a3552..11f6189547 100644 --- a/lms/djangoapps/instructor/staff_grading_service.py +++ b/lms/djangoapps/instructor/staff_grading_service.py @@ -34,9 +34,10 @@ class MockStaffGradingService(object): return json.dumps({'success': True, 'submission_id': self.cnt, 'submission': 'Test submission {cnt}'.format(cnt=self.cnt), + 'max_score': 2 + self.cnt % 3, 'rubric': 'A rubric'}) - def save_grade(self, course_id, submission_id, score, feedback): + def save_grade(self, course_id, grader_id, submission_id, score, feedback): return self.get_next(course_id) @@ -62,16 +63,23 @@ class StaffGradingService(object): return r.text - def save_grade(self, course_id, submission_id, score, feedback): + def save_grade(self, course_id, grader_id, submission_id, score, feedback): """ - Save a grade. + Save a score and feedback for a submission. - TODO: what is data? + Returns json dict with keys + 'success': bool + 'error': error msg, if something went wrong. - Returns json, or raises GradingServiceError if there's a problem. + Raises GradingServiceError if there's a problem connecting. """ try: - r = self.session.get(self.url + 'save_grade') + data = {'course_id': course_id, + 'submission_id': submission_id, + 'score': score, + 'feedback': feedback, + 'grader_id': grader} + r = self.session.post(self.url + 'save_grade') except requests.exceptions.ConnectionError as err: # reraise as promised GradingServiceError, but preserve stacktrace. raise GradingServiceError, str(err), sys.exc_info()[2] @@ -163,7 +171,11 @@ def save_grade(request, course_id): p = request.POST try: - result_json = _service.save_grade(course_id, p['submission_id'], p['score'], p['feedback']) + result_json = _service.save_grade(course_id, + request.user.id, + p['submission_id'], + p['score'], + p['feedback']) except GradingServiceError: log.exception("Error saving grade") return _err_response('Could not connect to grading service') diff --git a/lms/djangoapps/instructor/tests.py b/lms/djangoapps/instructor/tests.py index 28a56ad9de..87be93128c 100644 --- a/lms/djangoapps/instructor/tests.py +++ b/lms/djangoapps/instructor/tests.py @@ -112,15 +112,9 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader): xmodule.modulestore.django._MODULESTORES = {} courses = modulestore().get_courses() -<<<<<<< HEAD - self.full = modulestore().get_course("edX/full/6.002_Spring_2012") - self.toy = modulestore().get_course("edX/toy/2012_Fall") -======= - self.course_id = "edX/toy/2012_Fall" self.toy = modulestore().get_course(self.course_id) ->>>>>>> Refactor testing code, hook up frontend. # Create two accounts self.student = 'view@test.com' @@ -236,9 +230,6 @@ class TestStaffGradingService(ct.PageLoader): access control and error handling logic -- all the actual work is on the backend. ''' - - - def setUp(self): xmodule.modulestore.django._MODULESTORES = {} @@ -265,8 +256,6 @@ class TestStaffGradingService(ct.PageLoader): self.check_for_get_code(404, url) self.check_for_post_code(404, url) -<<<<<<< HEAD -======= @patch.object(staff_grading_service, '_service', _mock_service) def test_get_next(self): @@ -286,11 +275,12 @@ class TestStaffGradingService(ct.PageLoader): url = reverse('staff_grading_save_grade', kwargs={'course_id': self.course_id}) - data = {'score': '12', 'feedback': 'great!', 'submission_id': '123'} + data = {'score': '12', + 'feedback': 'great!', + 'submission_id': '123'} r = self.check_for_post_code(200, url, data) d = json.loads(r.content) self.assertTrue(d['success'], str(d)) self.assertEquals(d['submission_id'], _mock_service.cnt) ->>>>>>> Refactor testing code, hook up frontend. diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 7440277d98..b36b9e33e7 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -18,21 +18,19 @@ class StaffGradingBackend mock: (cmd, data) -> # Return a mock response to cmd and data - @mock_cnt++ if cmd == 'get_next' + @mock_cnt++ response = success: true submission: 'submission! ' + @mock_cnt rubric: 'A rubric! ' + @mock_cnt submission_id: @mock_cnt + max_score: 2 + @mock_cnt % 3 else if cmd == 'save_grade' console.log("eval: #{data.score} pts, Feedback: #{data.feedback}") response = - success: true - submission: 'another submission! ' + @mock_cnt - rubric: 'A rubric!' + @mock_cnt - submission_id: @mock_cnt + @mock('get_next', {}) else response = success: false @@ -72,6 +70,7 @@ class StaffGrading @submission_wrapper = $('.submission-wrapper') @rubric_wrapper = $('.rubric-wrapper') @feedback_area = $('.feedback-area') + @score_selection_container = $('.score-selection-container') @submit_button = $('.submit-button') # model state @@ -81,15 +80,12 @@ class StaffGrading @rubric = '' @error_msg = '' @message = '' + @max_score = 0 @score = null # action handlers @submit_button.click @submit - # TODO: hook up an event to the input changing, which updates - # @score (instead of the individual hacks) - $('#correct-radio').click @graded_callback - $('#incorrect-radio').click @graded_callback # render intial state @render_view() @@ -98,6 +94,24 @@ class StaffGrading @get_next_submission() + setup_score_selection: => + # first, get rid of all the old inputs, if any. + @score_selection_container.html('') + + # Now create new labels and inputs for each possible score. + for score in [1..@max_score] + id = 'score-' + score + label = """""" + + input = """ + + """ + @score_selection_container.append(label + input) + + # And now hook up an event handler again + $("input[name='score-selection']").change @graded_callback + + set_button_text: (text) => @submit_button.attr('value', text) @@ -113,7 +127,7 @@ class StaffGrading if response.success if response.submission - @data_loaded(response.submission, response.rubric, response.submission_id) + @data_loaded(response.submission, response.rubric, response.submission_id, response.max_score) else @no_more(response.message) else @@ -136,11 +150,12 @@ class StaffGrading @error_msg = msg @state = state_error - data_loaded: (submission, rubric, submission_id) -> + data_loaded: (submission, rubric, submission_id, max_score) -> @submission = submission @rubric = rubric @submission_id = submission_id @feedback_area.val('') + @max_score = max_score @score = null @state = state_grading @@ -149,6 +164,8 @@ class StaffGrading @rubric = null @submission_id = null @message = message + @score = null + @max_score = 0 @state = state_no_data render_view: () -> @@ -157,6 +174,9 @@ class StaffGrading show_submit_button = true @message_container.html(@message) + if @backend.mock_backend + @message_container.append("

NOTE: Mocking backend.

") + @error_container.html(@error_msg) if @state == state_error @@ -170,10 +190,8 @@ class StaffGrading # no submit button until user picks grade. show_submit_button = false - # TODO: clean up with proper input-related logic - $('#correct-radio')[0].checked = false - $('#incorrect-radio')[0].checked = false - + @setup_score_selection() + else if @state == state_graded show_grading_elements = true @set_button_text('Submit') @@ -204,7 +222,7 @@ class StaffGrading # for now, just create an instance and load it... -mock_backend = false +mock_backend = true ajax_url = $('.staff-grading').data('ajax_url') backend = new StaffGradingBackend(ajax_url, mock_backend) diff --git a/lms/static/coffee/src/staff_grading/test_grading.html b/lms/static/coffee/src/staff_grading/test_grading.html index 8a1cde1fd4..9b84d0703b 100644 --- a/lms/static/coffee/src/staff_grading/test_grading.html +++ b/lms/static/coffee/src/staff_grading/test_grading.html @@ -30,11 +30,7 @@
-

- - - - +

From d1b433dba57dfe60e2872e33c23ba5841abe7c99 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 19 Nov 2012 18:08:07 -0500 Subject: [PATCH 45/89] add adaptive-max-score html to lms view --- lms/static/coffee/src/staff_grading/staff_grading.coffee | 4 ++-- lms/templates/instructor/staff_grading.html | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index b36b9e33e7..a3d5cf939b 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -105,7 +105,7 @@ class StaffGrading input = """ - """ + """ # " fix broken parsing in emacs @score_selection_container.append(label + input) # And now hook up an event handler again @@ -222,7 +222,7 @@ class StaffGrading # for now, just create an instance and load it... -mock_backend = true +mock_backend = false ajax_url = $('.staff-grading').data('ajax_url') backend = new StaffGradingBackend(ajax_url, mock_backend) diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index d8e834d4f6..dbf2e97dde 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -37,13 +37,10 @@
+ -

- - - - +

From d2cc8696eda59d42eb6eb9014ac09631db87dd24 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 20 Nov 2012 11:11:18 -0500 Subject: [PATCH 46/89] Add some initial css. --- .../src/staff_grading/staff_grading.coffee | 4 +-- lms/static/sass/course.scss | 1 + lms/static/sass/course/_staff_grading.scss | 29 +++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 lms/static/sass/course/_staff_grading.scss diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index a3d5cf939b..ffdb28ccfd 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -96,7 +96,7 @@ class StaffGrading setup_score_selection: => # first, get rid of all the old inputs, if any. - @score_selection_container.html('') + @score_selection_container.html('Choose score: ') # Now create new labels and inputs for each possible score. for score in [1..@max_score] @@ -106,7 +106,7 @@ class StaffGrading input = """ """ # " fix broken parsing in emacs - @score_selection_container.append(label + input) + @score_selection_container.append(input + label) # And now hook up an event handler again $("input[name='score-selection']").change @graded_callback diff --git a/lms/static/sass/course.scss b/lms/static/sass/course.scss index acd735d25e..e900e589b2 100644 --- a/lms/static/sass/course.scss +++ b/lms/static/sass/course.scss @@ -43,6 +43,7 @@ @import "course/profile"; @import "course/gradebook"; @import "course/tabs"; +@import "course/staff_grading"; // instructor @import "course/instructor/instructor"; diff --git a/lms/static/sass/course/_staff_grading.scss b/lms/static/sass/course/_staff_grading.scss new file mode 100644 index 0000000000..4900f78bd0 --- /dev/null +++ b/lms/static/sass/course/_staff_grading.scss @@ -0,0 +1,29 @@ +div.staff-grading { + textarea.feedback-area { + height: 100px; + margin: 20px; + } + + div { + margin: 10px; + } + + label { + margin: 10px; + padding: 5px; + display: inline-block; + min-width: 50px; + background-color: #CCC; + text-size: 1.5em; + } + + /* Toggled State */ + input[type=radio]:checked + label { + background: #666; + color: white; + } + + input[name='score-selection'] { + display: none; + } +} From edd3277597644f7d7c509d2da18f135eb5ef9c5b Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 20 Nov 2012 18:25:16 -0500 Subject: [PATCH 47/89] Tweaks to actually work with the backend. - specify mime type - right urls --- .../instructor/staff_grading_service.py | 39 ++++++++++++------- lms/envs/dev.py | 3 ++ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/lms/djangoapps/instructor/staff_grading_service.py b/lms/djangoapps/instructor/staff_grading_service.py index 11f6189547..47f483e75f 100644 --- a/lms/djangoapps/instructor/staff_grading_service.py +++ b/lms/djangoapps/instructor/staff_grading_service.py @@ -8,8 +8,7 @@ import requests import sys from django.conf import settings -from django.http import Http404 -from django.http import HttpResponse +from django.http import HttpResponse, Http404 from courseware.access import has_access from util.json_request import expect_json @@ -29,7 +28,7 @@ class MockStaffGradingService(object): def __init__(self): self.cnt = 0 - def get_next(self, course_id): + def get_next(self, course_id, grader_id): self.cnt += 1 return json.dumps({'success': True, 'submission_id': self.cnt, @@ -47,16 +46,20 @@ class StaffGradingService(object): """ def __init__(self, url): self.url = url + self.get_next_url = url + '/get_next_submission/' + self.save_grade_url = url + '/save_grade/' # TODO: add auth self.session = requests.session() - def get_next(self, course_id): + def get_next(self, course_id, grader_id): """ Get the next thing to grade. Returns json, or raises GradingServiceError if there's a problem. """ try: - r = self.session.get(self.url + 'get_next') + r = self.session.get(self.get_next_url, + params={'course_id': course_id, + 'grader_id': grader_id}) except requests.exceptions.ConnectionError as err: # reraise as promised GradingServiceError, but preserve stacktrace. raise GradingServiceError, str(err), sys.exc_info()[2] @@ -78,22 +81,24 @@ class StaffGradingService(object): 'submission_id': submission_id, 'score': score, 'feedback': feedback, - 'grader_id': grader} - r = self.session.post(self.url + 'save_grade') + 'grader_id': grader_id} + + r = self.session.post(self.save_grade_url, data=data) except requests.exceptions.ConnectionError as err: # reraise as promised GradingServiceError, but preserve stacktrace. raise GradingServiceError, str(err), sys.exc_info()[2] return r.text -#_service = StaffGradingService(settings.STAFF_GRADING_BACKEND_URL) -_service = MockStaffGradingService() +_service = StaffGradingService(settings.STAFF_GRADING_BACKEND_URL) +#_service = MockStaffGradingService() def _err_response(msg): """ Return a HttpResponse with a json dump with success=False, and the given error message. """ - return HttpResponse(json.dumps({'success': False, 'error': msg})) + return HttpResponse(json.dumps({'success': False, 'error': msg}), + mimetype="application/json") def _check_access(user, course_id): @@ -129,16 +134,17 @@ def get_next(request, course_id): """ _check_access(request.user, course_id) - return HttpResponse(_get_next(course_id)) + return HttpResponse(_get_next(course_id, request.user.id), + mimetype="application/json") -def _get_next(course_id): +def _get_next(course_id, grader_id): """ Implementation of get_next (also called from save_grade) -- returns a json string """ try: - return _service.get_next(course_id) + return _service.get_next(course_id, grader_id) except GradingServiceError: log.exception("Error from grading service") return json.dumps({'success': False, 'error': 'Could not connect to grading service'}) @@ -168,11 +174,12 @@ def save_grade(request, course_id): if k not in request.POST.keys(): return _err_response('Missing required key {0}'.format(k)) + grader_id = request.user.id p = request.POST try: result_json = _service.save_grade(course_id, - request.user.id, + grader_id, p['submission_id'], p['score'], p['feedback']) @@ -183,6 +190,7 @@ def save_grade(request, course_id): try: result = json.loads(result_json) except ValueError: + log.exception("save_grade returned broken json: %s", result_json) return _err_response('Grading service returned mal-formatted data.') if not result.get('success', False): @@ -190,5 +198,6 @@ def save_grade(request, course_id): return _err_response('Grading service failed') # Ok, save_grade seemed to work. Get the next submission to grade. - return HttpResponse(_get_next(course_id)) + return HttpResponse(_get_next(course_id, grader_id), + mimetype="application/json") diff --git a/lms/envs/dev.py b/lms/envs/dev.py index bf72284425..57aeeb7bd9 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -102,6 +102,9 @@ SUBDOMAIN_BRANDING = { COMMENTS_SERVICE_KEY = "PUT_YOUR_API_KEY_HERE" +################################# Staff grading config ##################### + +STAFF_GRADING_BACKEND_URL = "http://127.0.0.1:3033/staff_grading" ################################ LMS Migration ################################# From 26f033b79ffe9cda25f870396f6cb645310fbab0 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 20 Nov 2012 18:25:25 -0500 Subject: [PATCH 48/89] allow score 0 --- 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 ffdb28ccfd..5edc5e5c35 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -99,7 +99,7 @@ class StaffGrading @score_selection_container.html('Choose score: ') # Now create new labels and inputs for each possible score. - for score in [1..@max_score] + for score in [0..@max_score] id = 'score-' + score label = """""" From b48b389a565e6dd938a0009be338bcba6d6f9632 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 26 Nov 2012 16:40:25 -0500 Subject: [PATCH 49/89] implement login into staff grading service. --- .../instructor/staff_grading_service.py | 71 ++++++++++++++++--- lms/envs/common.py | 2 + lms/envs/dev.py | 2 + 3 files changed, 66 insertions(+), 9 deletions(-) diff --git a/lms/djangoapps/instructor/staff_grading_service.py b/lms/djangoapps/instructor/staff_grading_service.py index 47f483e75f..5bff10bcda 100644 --- a/lms/djangoapps/instructor/staff_grading_service.py +++ b/lms/djangoapps/instructor/staff_grading_service.py @@ -5,6 +5,7 @@ This module provides views that proxy to the staff grading backend service. import json import logging import requests +from requests.exceptions import RequestException, ConnectionError, HTTPError import sys from django.conf import settings @@ -44,23 +45,70 @@ class StaffGradingService(object): """ Interface to staff grading backend. """ - def __init__(self, url): + def __init__(self, url, username, password): + self.username = username + self.password = password self.url = url + + self.login_url = url + '/login/' self.get_next_url = url + '/get_next_submission/' self.save_grade_url = url + '/save_grade/' + # TODO: add auth self.session = requests.session() + + def _login(self): + """ + Log into the staff grading service. + + Raises requests.exceptions.HTTPError if something goes wrong. + + Returns the decoded json dict of the response. + """ + response = self.session.post(self.login_url, + {'username': self.username, + 'password': self.password,}) + + response.raise_for_status() + + return response.json + + + def _try_with_login(self, operation): + """ + Call operation(), which should return a requests response object. If + the response status code is 302, call _login() and try the operation + again. NOTE: use requests.get(..., allow_redirects=False) to have + requests not auto-follow redirects. + + Returns the result of operation(). Does not catch exceptions. + """ + response = operation() + if response.status_code == 302: + # redirect means we aren't logged in + r = self._login() + if r and not r.get('success'): + log.warning("Couldn't log into staff_grading backend. Response: %s", + r) + # try again + return operation() + + return response + + def get_next(self, course_id, grader_id): """ Get the next thing to grade. Returns json, or raises GradingServiceError if there's a problem. """ + op = lambda: self.session.get(self.get_next_url, + allow_redirects=False, + params={'course_id': course_id, + 'grader_id': grader_id}) try: - r = self.session.get(self.get_next_url, - params={'course_id': course_id, - 'grader_id': grader_id}) - except requests.exceptions.ConnectionError as err: + r = self._try_with_login(op) + except (RequestException, ConnectionError, HTTPError) as err: # reraise as promised GradingServiceError, but preserve stacktrace. raise GradingServiceError, str(err), sys.exc_info()[2] @@ -82,15 +130,20 @@ class StaffGradingService(object): 'score': score, 'feedback': feedback, 'grader_id': grader_id} - - r = self.session.post(self.save_grade_url, data=data) - except requests.exceptions.ConnectionError as err: + + op = lambda: self.session.post(self.save_grade_url, data=data, + allow_redirects=False) + r = self._try_with_login(op) + except (RequestException, ConnectionError, HTTPError) as err: # reraise as promised GradingServiceError, but preserve stacktrace. raise GradingServiceError, str(err), sys.exc_info()[2] return r.text -_service = StaffGradingService(settings.STAFF_GRADING_BACKEND_URL) +_service = StaffGradingService(settings.STAFF_GRADING_BACKEND_URL, + settings.STAFF_GRADING_BACKEND_USERNAME, + settings.STAFF_GRADING_BACKEND_PASSWORD, + ) #_service = MockStaffGradingService() def _err_response(msg): diff --git a/lms/envs/common.py b/lms/envs/common.py index 7ddd2ffb9a..005d333e09 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -325,6 +325,8 @@ WIKI_LINK_DEFAULT_LEVEL = 2 ################################# Staff grading config ##################### STAFF_GRADING_BACKEND_URL = None +STAFF_GRADING_BACKEND_USERNAME = None +STAFF_GRADING_BACKEND_PASSWORD = None ################################# Jasmine ################################### JASMINE_TEST_DIRECTORY = PROJECT_ROOT + '/static/coffee' diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 57aeeb7bd9..f3cc3e4c63 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -105,6 +105,8 @@ COMMENTS_SERVICE_KEY = "PUT_YOUR_API_KEY_HERE" ################################# Staff grading config ##################### STAFF_GRADING_BACKEND_URL = "http://127.0.0.1:3033/staff_grading" +STAFF_GRADING_BACKEND_USERNAME = "lms" +STAFF_GRADING_BACKEND_PASSWORD = "abcd" ################################ LMS Migration ################################# From d28cd4f42910852e3242091901301b5f20bf9225 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 26 Nov 2012 17:09:52 -0500 Subject: [PATCH 50/89] use a dict for backend config. Load it on aws. --- .../instructor/staff_grading_service.py | 19 ++++++++----------- lms/envs/aws.py | 3 +++ lms/envs/common.py | 4 +--- lms/envs/dev.py | 11 ++++++----- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/lms/djangoapps/instructor/staff_grading_service.py b/lms/djangoapps/instructor/staff_grading_service.py index 5bff10bcda..05c131ed56 100644 --- a/lms/djangoapps/instructor/staff_grading_service.py +++ b/lms/djangoapps/instructor/staff_grading_service.py @@ -45,14 +45,14 @@ class StaffGradingService(object): """ Interface to staff grading backend. """ - def __init__(self, url, username, password): - self.username = username - self.password = password - self.url = url + def __init__(self, config): + self.username = config['username'] + self.password = config['password'] + self.url = config['url'] - self.login_url = url + '/login/' - self.get_next_url = url + '/get_next_submission/' - self.save_grade_url = url + '/save_grade/' + self.login_url = self.url + '/login/' + self.get_next_url = self.url + '/get_next_submission/' + self.save_grade_url = self.url + '/save_grade/' # TODO: add auth self.session = requests.session() @@ -140,10 +140,7 @@ class StaffGradingService(object): return r.text -_service = StaffGradingService(settings.STAFF_GRADING_BACKEND_URL, - settings.STAFF_GRADING_BACKEND_USERNAME, - settings.STAFF_GRADING_BACKEND_PASSWORD, - ) +_service = StaffGradingService(settings.STAFF_GRADING_INTERFACE) #_service = MockStaffGradingService() def _err_response(msg): diff --git a/lms/envs/aws.py b/lms/envs/aws.py index b58bc5602b..d1abce8a6d 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -76,5 +76,8 @@ DATABASES = AUTH_TOKENS['DATABASES'] XQUEUE_INTERFACE = AUTH_TOKENS['XQUEUE_INTERFACE'] +STAFF_GRADING_BACKEND = AUTH_TOKENS.get('STAFF_GRADING_INTERFACE') + + PEARSON_TEST_USER = "pearsontest" PEARSON_TEST_PASSWORD = AUTH_TOKENS.get("PEARSON_TEST_PASSWORD") diff --git a/lms/envs/common.py b/lms/envs/common.py index 005d333e09..3d26cb54c9 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -324,9 +324,7 @@ WIKI_LINK_DEFAULT_LEVEL = 2 ################################# Staff grading config ##################### -STAFF_GRADING_BACKEND_URL = None -STAFF_GRADING_BACKEND_USERNAME = None -STAFF_GRADING_BACKEND_PASSWORD = None +STAFF_GRADING_INTERFACE = None ################################# Jasmine ################################### JASMINE_TEST_DIRECTORY = PROJECT_ROOT + '/static/coffee' diff --git a/lms/envs/dev.py b/lms/envs/dev.py index f3cc3e4c63..0ad42f67d3 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -39,7 +39,7 @@ DATABASES = { } CACHES = { - # This is the cache used for most things. + # This is the cache used for most things. # In staging/prod envs, the sessions also live here. 'default': { 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', @@ -104,10 +104,11 @@ COMMENTS_SERVICE_KEY = "PUT_YOUR_API_KEY_HERE" ################################# Staff grading config ##################### -STAFF_GRADING_BACKEND_URL = "http://127.0.0.1:3033/staff_grading" -STAFF_GRADING_BACKEND_USERNAME = "lms" -STAFF_GRADING_BACKEND_PASSWORD = "abcd" - +STAFF_GRADING_INTERFACE = { + 'url': 'http://127.0.0.1:3033/staff_grading', + 'username': 'lms', + 'password': 'abcd', + } ################################ LMS Migration ################################# MITX_FEATURES['ENABLE_LMS_MIGRATION'] = True From 835f18795afe56f3fd4e6b96935f705d103f0ad2 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Fri, 30 Nov 2012 10:27:34 -0500 Subject: [PATCH 51/89] Make tests pass again --- .../instructor/staff_grading_service.py | 30 +++++++++++++++---- lms/djangoapps/instructor/tests.py | 16 +++++++--- lms/envs/common.py | 3 ++ lms/envs/test.py | 6 +++- 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/lms/djangoapps/instructor/staff_grading_service.py b/lms/djangoapps/instructor/staff_grading_service.py index 05c131ed56..c070bd6835 100644 --- a/lms/djangoapps/instructor/staff_grading_service.py +++ b/lms/djangoapps/instructor/staff_grading_service.py @@ -38,7 +38,7 @@ class MockStaffGradingService(object): 'rubric': 'A rubric'}) def save_grade(self, course_id, grader_id, submission_id, score, feedback): - return self.get_next(course_id) + return self.get_next(course_id, grader_id) class StaffGradingService(object): @@ -140,8 +140,28 @@ class StaffGradingService(object): return r.text -_service = StaffGradingService(settings.STAFF_GRADING_INTERFACE) -#_service = MockStaffGradingService() +# don't initialize until grading_service() is called--means that just +# importing this file doesn't create objects that may not have the right config +_service = None + +def grading_service(): + """ + Return a staff grading service instance--if settings.MOCK_STAFF_GRADING is True, + returns a mock one, otherwise a real one. + + Caches the result, so changing the setting after the first call to this + function will have no effect. + """ + global _service + if _service is not None: + return _service + + if settings.MOCK_STAFF_GRADING: + _service = MockStaffGradingService() + else: + _service = StaffGradingService(settings.STAFF_GRADING_INTERFACE) + + return _service def _err_response(msg): """ @@ -194,7 +214,7 @@ def _get_next(course_id, grader_id): """ try: - return _service.get_next(course_id, grader_id) + return grading_service().get_next(course_id, grader_id) except GradingServiceError: log.exception("Error from grading service") return json.dumps({'success': False, 'error': 'Could not connect to grading service'}) @@ -228,7 +248,7 @@ def save_grade(request, course_id): p = request.POST try: - result_json = _service.save_grade(course_id, + result_json = grading_service().save_grade(course_id, grader_id, p['submission_id'], p['score'], diff --git a/lms/djangoapps/instructor/tests.py b/lms/djangoapps/instructor/tests.py index 87be93128c..c47eb170fc 100644 --- a/lms/djangoapps/instructor/tests.py +++ b/lms/djangoapps/instructor/tests.py @@ -233,6 +233,14 @@ class TestStaffGradingService(ct.PageLoader): def setUp(self): xmodule.modulestore.django._MODULESTORES = {} + self.student = 'view@test.com' + self.instructor = 'view2@test.com' + self.password = 'foo' + self.create_account('u1', self.student, self.password) + self.create_account('u2', self.instructor, self.password) + self.activate_user(self.student) + self.activate_user(self.instructor) + self.course_id = "edX/toy/2012_Fall" self.toy = modulestore().get_course(self.course_id) def make_instructor(course): @@ -242,6 +250,8 @@ class TestStaffGradingService(ct.PageLoader): make_instructor(self.toy) + self.mock_service = staff_grading_service.grading_service() + self.logout() def test_access(self): @@ -257,7 +267,6 @@ class TestStaffGradingService(ct.PageLoader): self.check_for_post_code(404, url) - @patch.object(staff_grading_service, '_service', _mock_service) def test_get_next(self): self.login(self.instructor, self.password) @@ -266,10 +275,9 @@ class TestStaffGradingService(ct.PageLoader): r = self.check_for_get_code(200, url) d = json.loads(r.content) self.assertTrue(d['success']) - self.assertEquals(d['submission_id'], _mock_service.cnt) + self.assertEquals(d['submission_id'], self.mock_service.cnt) - @patch.object(staff_grading_service, '_service', _mock_service) def test_save_grade(self): self.login(self.instructor, self.password) @@ -281,6 +289,6 @@ class TestStaffGradingService(ct.PageLoader): r = self.check_for_post_code(200, url, data) d = json.loads(r.content) self.assertTrue(d['success'], str(d)) - self.assertEquals(d['submission_id'], _mock_service.cnt) + self.assertEquals(d['submission_id'], self.mock_service.cnt) diff --git a/lms/envs/common.py b/lms/envs/common.py index 3d26cb54c9..79d0bb78f9 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -325,6 +325,9 @@ WIKI_LINK_DEFAULT_LEVEL = 2 ################################# Staff grading config ##################### STAFF_GRADING_INTERFACE = None +# Used for testing, debugging +MOCK_STAFF_GRADING = False + ################################# Jasmine ################################### JASMINE_TEST_DIRECTORY = PROJECT_ROOT + '/static/coffee' diff --git a/lms/envs/test.py b/lms/envs/test.py index e815efdf4e..ef2a343db4 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -65,6 +65,10 @@ XQUEUE_INTERFACE = { } XQUEUE_WAITTIME_BETWEEN_REQUESTS = 5 # seconds + +# Don't rely on a real staff grading backend +MOCK_STAFF_GRADING = True + # TODO (cpennington): We need to figure out how envs/test.py can inject things # into common.py so that we don't have to repeat this sort of thing STATICFILES_DIRS = [ @@ -99,7 +103,7 @@ DATABASES = { } CACHES = { - # This is the cache used for most things. + # This is the cache used for most things. # In staging/prod envs, the sessions also live here. 'default': { 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', From 052807d7f60a877e6924d1240c8f481a23803ba5 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Fri, 30 Nov 2012 11:30:02 -0500 Subject: [PATCH 52/89] Change the unique student id calculation to match the id we send to xqueue - can't change the xqueue one because there's data that uses it - we checked, and no one has sent out survey links that use the old sha1() format doing this because we'll need the LMS to be able to send the anon student id to backend services e.g. for peer grading, and passing two different anon student ids to xmodule (one for xqueue, one for other uses) is just asking for confusion. --- common/djangoapps/student/models.py | 15 +++++++-------- lms/djangoapps/courseware/module_render.py | 10 ++-------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 5975853a21..2f5bc3ac04 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -36,7 +36,7 @@ file and check it in at the same time as your model changes. To do that, 3. Add the migration file created in mitx/common/djangoapps/student/migrations/ """ from datetime import datetime -from hashlib import sha1 +import hashlib import json import logging import uuid @@ -197,14 +197,13 @@ def unique_id_for_user(user): """ Return a unique id for a user, suitable for inserting into e.g. personalized survey links. - - Currently happens to be implemented as a sha1 hash of the username - (and thus assumes that usernames don't change). """ - # Using the user id as the salt because it's sort of random, and is already - # in the db. - salt = str(user.id) - return sha1(salt + user.username).hexdigest() + # include the secret key as a salt, and to make the ids unique accross + # different LMS installs. + h = hashlib.md5() + h.update(settings.SECRET_KEY) + h.update(str(user.id)) + return h.hexdigest() ## TODO: Should be renamed to generic UserGroup, and possibly diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 67927c0ee7..eb7b41b1e9 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -1,4 +1,3 @@ -import hashlib import json import logging import pyparsing @@ -20,6 +19,7 @@ from mitxmako.shortcuts import render_to_string from models import StudentModule, StudentModuleCache from psychometrics.psychoanalyze import make_psychometrics_data_update_handler from static_replace import replace_urls +from student.models import unique_id_for_user from xmodule.errortracker import exc_info_to_str from xmodule.exceptions import NotFoundError from xmodule.modulestore import Location @@ -152,12 +152,6 @@ def _get_module(user, request, location, student_module_cache, course_id, positi if not has_access(user, descriptor, 'load'): return None - # Anonymized student identifier - h = hashlib.md5() - h.update(settings.SECRET_KEY) - h.update(str(user.id)) - anonymous_student_id = h.hexdigest() - # Only check the cache if this module can possibly have state instance_module = None shared_module = None @@ -230,7 +224,7 @@ def _get_module(user, request, location, student_module_cache, course_id, positi # by the replace_static_urls code below replace_urls=replace_urls, node_path=settings.NODE_PATH, - anonymous_student_id=anonymous_student_id, + anonymous_student_id=unique_id_for_user(user), ) # pass position specified in URL to module through ModuleSystem system.set('position', position) From 51e148a7a7413176afd8661d2a8defa25bd729c4 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 11:42:24 -0500 Subject: [PATCH 53/89] 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 54/89] 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 55/89] 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 56/89] 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 57/89] 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 58/89] 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 32d8a6e92275dbbee7bbda26625b4beb46896a4a Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Fri, 30 Nov 2012 14:11:11 -0500 Subject: [PATCH 59/89] Pass through course_id, location to openendedresponse - set default queue in response type--don't use the default per-course one --- common/lib/capa/capa/responsetypes.py | 27 +++++++++++++--------- common/lib/xmodule/xmodule/capa_module.py | 5 ++++ common/lib/xmodule/xmodule/x_module.py | 6 ++++- lms/djangoapps/courseware/module_render.py | 1 + 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index ba9f03549e..6346b08a42 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1730,9 +1730,9 @@ class ImageResponse(LoncapaResponse): Regions is list of lists [region1, region2, region3, ...] where regionN is disordered list of points: [[1,1], [100,100], [50,50], [20, 70]]. - + If there is only one region in the list, simpler notation can be used: - regions="[[10,10], [30,30], [10, 30], [30, 10]]" (without explicitly + regions="[[10,10], [30,30], [10, 30], [30, 10]]" (without explicitly setting outer list) Returns: @@ -1817,19 +1817,24 @@ class ImageResponse(LoncapaResponse): class OpenEndedResponse(LoncapaResponse): """ - Grade student open ended responses using an external queueing server, called 'xqueue' + Grade student open ended responses using an external grading system, + accessed through the xqueue system. + + Expects 'xqueue' dict in ModuleSystem with the following keys that are + needed by OpenEndedResponse: - Expects 'xqueue' dict in ModuleSystem with the following keys that are needed by OpenEndedResponse: system.xqueue = { 'interface': XqueueInterface object, 'callback_url': Per-StudentModule callback URL where results are posted (string), - 'default_queuename': Default queuename to submit request (string) } External requests are only submitted for student submission grading (i.e. and not for getting reference answers) + + By default, uses the OpenEndedResponse.DEFAULT_QUEUE queue. """ + DEFAULT_QUEUE = 'open-ended' response_tag = 'openendedresponse' allowed_inputfields = ['openendedinput'] max_inputfields = 1 @@ -1841,7 +1846,7 @@ class OpenEndedResponse(LoncapaResponse): xml = self.xml # TODO: XML can override external resource (grader/queue) URL self.url = xml.get('url', None) - self.queue_name = xml.get('queuename', self.system.xqueue['default_queuename']) + self.queue_name = xml.get('queuename', self.DEFAULT_QUEUE) #Look for tag named openendedparam that encapsulates all grader settings oeparam = self.xml.find('openendedparam') @@ -1883,11 +1888,12 @@ class OpenEndedResponse(LoncapaResponse): #Update grader payload with student id. If grader payload not json, error. try: grader_payload=json.loads(grader_payload) - location=self.system.ajax_url.split("://")[1] - org,course,type,name=location.split("/") + # NOTE: self.system.location is valid because the capa_module + # __init__ adds it (easiest way to get problem location into + # response types) grader_payload.update({ - 'location' : location, - 'course_id' : "{0}/{1}".format(org,course), + 'location' : self.system.location, + 'course_id' : self.system.course_id, 'prompt' : prompt_string, 'rubric' : rubric_string, }) @@ -1997,7 +2003,6 @@ class OpenEndedResponse(LoncapaResponse): msg='Invalid grader reply. Please contact the course staff.') return oldcmap - correctness = 'correct' if correct else 'incorrect' # TODO: Find out how this is used elsewhere, if any diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 47d5d5c423..ead138a225 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -146,6 +146,11 @@ class CapaModule(XModule): else: self.seed = None + # Need the problem location in openendedresponse to send out. Adding + # it to the system here seems like the least clunky way to get it + # there. + self.system.set('location', self.location) + try: # TODO (vshnayder): move as much as possible of this work and error # checking to descriptor load time diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 6f3fb73356..19a592191e 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -809,7 +809,8 @@ class ModuleSystem(object): debug=False, xqueue=None, node_path="", - anonymous_student_id=''): + anonymous_student_id='', + course_id=None): ''' Create a closure around the system environment. @@ -844,6 +845,8 @@ class ModuleSystem(object): ajax results. anonymous_student_id - Used for tracking modules with student id + + course_id - the course_id containing this module ''' self.ajax_url = ajax_url self.xqueue = xqueue @@ -856,6 +859,7 @@ class ModuleSystem(object): self.replace_urls = replace_urls self.node_path = node_path self.anonymous_student_id = anonymous_student_id + self.course_id = course_id self.user_is_staff = user is not None and user.is_staff def get(self, attr): diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index eb7b41b1e9..bd919eeb15 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -225,6 +225,7 @@ def _get_module(user, request, location, student_module_cache, course_id, positi replace_urls=replace_urls, node_path=settings.NODE_PATH, anonymous_student_id=unique_id_for_user(user), + course_id=course_id, ) # pass position specified in URL to module through ModuleSystem system.set('position', position) From a20a6c8fb59ec755521857458dc2096cae9d911e Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 14:11:43 -0500 Subject: [PATCH 60/89] 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 61/89] 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 62/89] 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 63/89] 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 From 4e4bd4aba275873de94297a54f04a77be53591f0 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 30 Nov 2012 15:52:11 -0500 Subject: [PATCH 64/89] Altered staff grading to display message about ml grading performance. --- .../coffee/src/staff_grading/staff_grading.coffee | 13 ++++++++++--- lms/templates/instructor/staff_grading.html | 3 +++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 5edc5e5c35..c953b38c2e 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -26,6 +26,7 @@ class StaffGradingBackend rubric: 'A rubric! ' + @mock_cnt submission_id: @mock_cnt max_score: 2 + @mock_cnt % 3 + ml_error_info : 'ML error info!' + @mock_cnt else if cmd == 'save_grade' console.log("eval: #{data.score} pts, Feedback: #{data.feedback}") @@ -71,7 +72,8 @@ class StaffGrading @rubric_wrapper = $('.rubric-wrapper') @feedback_area = $('.feedback-area') @score_selection_container = $('.score-selection-container') - @submit_button = $('.submit-button') + @submit_button = $('.submit-button') + @ml_error_info_container = $('.ml-error-info-container') # model state @state = state_no_data @@ -81,6 +83,7 @@ class StaffGrading @error_msg = '' @message = '' @max_score = 0 + @ml_error_info= '' @score = null @@ -127,7 +130,7 @@ class StaffGrading if response.success if response.submission - @data_loaded(response.submission, response.rubric, response.submission_id, response.max_score) + @data_loaded(response.submission, response.rubric, response.submission_id, response.max_score, response.ml_error_info) else @no_more(response.message) else @@ -150,18 +153,20 @@ class StaffGrading @error_msg = msg @state = state_error - data_loaded: (submission, rubric, submission_id, max_score) -> + data_loaded: (submission, rubric, submission_id, max_score, ml_error_info) -> @submission = submission @rubric = rubric @submission_id = submission_id @feedback_area.val('') @max_score = max_score @score = null + @ml_error_info=ml_error_info @state = state_grading no_more: (message) -> @submission = null @rubric = null + @ml_error_info = null @submission_id = null @message = message @score = null @@ -183,6 +188,7 @@ class StaffGrading @set_button_text('Try loading again') else if @state == state_grading + @ml_error_info_container.html(@ml_error_info) @submission_container.html(@submission) @rubric_container.html(@rubric) show_grading_elements = true @@ -206,6 +212,7 @@ class StaffGrading @submit_button.toggle(show_submit_button) @submission_wrapper.toggle(show_grading_elements) @rubric_wrapper.toggle(show_grading_elements) + @ml_error_info_container.toggle(show_grading_elements) submit: (event) => diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index dbf2e97dde..ad704ae5a4 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -25,6 +25,9 @@
+
+
+

Submission

From 63c1a8ea10b3c315cdbe16434afd19fc3450eb55 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 3 Dec 2012 12:43:43 -0500 Subject: [PATCH 65/89] whitespace --- common/lib/capa/capa/responsetypes.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 508845f773..bda1f5f1b1 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1844,32 +1844,35 @@ class OpenEndedResponse(LoncapaResponse): Configure OpenEndedResponse from XML. ''' xml = self.xml - # TODO: XML can override external resource (grader/queue) URL self.url = xml.get('url', None) self.queue_name = xml.get('queuename', self.DEFAULT_QUEUE) - #Look for tag named openendedparam that encapsulates all grader settings + # The openendedparam tag encapsulates all grader settings oeparam = self.xml.find('openendedparam') - prompt=self.xml.find('prompt') - rubric=self.xml.find('openendedrubric') - self._parse_openendedresponse_xml(oeparam,prompt,rubric) + prompt = self.xml.find('prompt') + rubric = self.xml.find('openendedrubric') + self._parse(oeparam,prompt,rubric) - def stringify_children(self,node,strip_tags=True): + def stringify_children(self, node, strip_tags=True): """ - Modify code from stringify_children in xmodule. Didn't import directly in order to avoid capa depending - on xmodule (seems to be avoided in code) + Modify code from stringify_children in xmodule. Didn't import directly + in order to avoid capa depending on xmodule (seems to be avoided in + code) """ parts=[node.text] [parts.append((etree.tostring(p, with_tail=True))) for p in node.getchildren()] node_string=' '.join(parts) - #Strip html tags from prompt. This may need to be removed in order to display prompt to instructors properly. + # Strip html tags from result. This may need to be removed in order to + # display prompt to instructors properly. + # TODO: what breaks if this is removed? The ML code can strip tags + # as part of its input filtering. if strip_tags: node_string=re.sub('<[^<]+?>', '', node_string) return node_string - def _parse_openendedresponse_xml(self,oeparam,prompt,rubric): + def _parse(self, oeparam, prompt, rubric): ''' Parse OpenEndedResponse XML: self.initial_display @@ -1879,8 +1882,8 @@ class OpenEndedResponse(LoncapaResponse): self.answer - What to display when show answer is clicked ''' # Note that OpenEndedResponse is agnostic to the specific contents of grader_payload - prompt_string=self.stringify_children(prompt) - rubric_string=self.stringify_children(rubric) + prompt_string = self.stringify_children(prompt) + rubric_string = self.stringify_children(rubric) grader_payload = oeparam.find('grader_payload') grader_payload = grader_payload.text if grader_payload is not None else '' From 22ce306f6443966a72f872708b119eae8e8d5c90 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 3 Dec 2012 12:49:58 -0500 Subject: [PATCH 66/89] whitespace and comments --- common/lib/capa/capa/responsetypes.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index bda1f5f1b1..98421ce0b3 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1851,7 +1851,7 @@ class OpenEndedResponse(LoncapaResponse): oeparam = self.xml.find('openendedparam') prompt = self.xml.find('prompt') rubric = self.xml.find('openendedrubric') - self._parse(oeparam,prompt,rubric) + self._parse(oeparam, prompt, rubric) def stringify_children(self, node, strip_tags=True): """ @@ -1890,7 +1890,7 @@ class OpenEndedResponse(LoncapaResponse): #Update grader payload with student id. If grader payload not json, error. try: - grader_payload=json.loads(grader_payload) + grader_payload = json.loads(grader_payload) # NOTE: self.system.location is valid because the capa_module # __init__ adds it (easiest way to get problem location into # response types) @@ -1900,7 +1900,7 @@ class OpenEndedResponse(LoncapaResponse): 'prompt' : prompt_string, 'rubric' : rubric_string, }) - grader_payload=json.dumps(grader_payload) + grader_payload = json.dumps(grader_payload) except Exception as err: log.error("Grader payload is not a json object!") @@ -1924,9 +1924,9 @@ class OpenEndedResponse(LoncapaResponse): top_score = oeparam.find('max_score') if top_score is not None: try: - self.max_score= int(top_score.text) + self.max_score = int(top_score.text) except: - self.top_score=1 + self.max_score = 1 else: self.max_score = 1 @@ -1992,7 +1992,8 @@ class OpenEndedResponse(LoncapaResponse): # 2) Frontend: correctness='incomplete' eventually trickles down # through inputtypes.textbox and .filesubmission to inform the # browser to poll the LMS - cmap.set(self.answer_id, queuestate=queuestate, correctness='incomplete', msg=msg) + cmap.set(self.answer_id, queuestate=queuestate, + correctness='incomplete', msg=msg) return cmap @@ -2001,7 +2002,7 @@ class OpenEndedResponse(LoncapaResponse): (valid_score_msg, correct, points, msg) = self._parse_score_msg(score_msg) if not valid_score_msg: oldcmap.set(self.answer_id, - msg='Invalid grader reply. Please contact the course staff.') + msg = 'Invalid grader reply. Please contact the course staff.') return oldcmap correctness = 'correct' if correct else 'incorrect' @@ -2132,11 +2133,11 @@ class OpenEndedResponse(LoncapaResponse): feedback = self._format_feedback(score_result) - score_ratio=int(score_result['score'])/self.max_score - correct=False - if score_ratio>=.66: - correct=True + # HACK: for now, just assume it's correct if you got more than 2/3. + # Also assumes that score_result['score'] is an integer. + score_ratio = int(score_result['score']) / self.max_score + correct = (score_ratio >= 0.66) log.debug(feedback) try: From a44e8887a87394a13e833e484b9ed3325b39d1fa Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 3 Dec 2012 13:22:50 -0500 Subject: [PATCH 67/89] add docs on staff grading tab type - also updated some out-of-date docs --- doc/xml-format.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/xml-format.md b/doc/xml-format.md index 46082b90ad..d7c5027a79 100644 --- a/doc/xml-format.md +++ b/doc/xml-format.md @@ -418,6 +418,10 @@ If you want to customize the courseware tabs displayed for your course, specify * "external_link". Parameters "name", "link". * "textbooks". No parameters--generates tab names from book titles. * "progress". Parameter "name". +* "static_tab". Parameters "name", 'url_slug'--will look for tab contents in + 'tabs/{course_url_name}/{tab url_slug}.html' +* "staff_grading". No parameters. If specified, displays the staff grading tab for instructors. + # Tips for content developers @@ -429,9 +433,7 @@ before the week 1 material to make it easy to find in the file. * Come up with a consistent pattern for url_names, so that it's easy to know where to look for any piece of content. It will also help to come up with a standard way of splitting your content files. As a point of departure, we suggest splitting chapters, sequences, html, and problems into separate files. -* A heads up: our content management system will allow you to develop content through a web browser, but will be backed by this same xml at first. Once that happens, every element will be in its own file to make access and updates faster. - -* Prefer the most "semantic" name for containers: e.g., use problemset rather than vertical for a problem set. That way, if we decide to display problem sets differently, we don't have to change the xml. +* Prefer the most "semantic" name for containers: e.g., use problemset rather than sequential for a problem set. That way, if we decide to display problem sets differently, we don't have to change the xml. # Other file locations (info and about) From 65c56edb5cabea51f65b121e3bcc0a28122ebe5b Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 3 Dec 2012 15:04:34 -0500 Subject: [PATCH 68/89] Better error logging when login into queue fails --- common/lib/capa/capa/xqueue_interface.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index f145cad23c..798867955b 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -81,7 +81,11 @@ class XQueueInterface(object): # Log in, then try again if error and (msg == 'login_required'): - self._login() + (error, content) = self._login() + if error != 0: + # when the login fails + log.debug("Failed to login to queue: %s", content) + return (error, content) if files_to_upload is not None: # Need to rewind file pointers for f in files_to_upload: From 9b1cad90b5ed56d7745ddefbc4885d8ddd34d0a7 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 3 Dec 2012 16:52:29 -0500 Subject: [PATCH 69/89] Fix location that gets used for open-ended responses --- common/lib/xmodule/xmodule/capa_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index ead138a225..4c10a1703a 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -149,7 +149,7 @@ class CapaModule(XModule): # Need the problem location in openendedresponse to send out. Adding # it to the system here seems like the least clunky way to get it # there. - self.system.set('location', self.location) + self.system.set('location', self.location.url()) try: # TODO (vshnayder): move as much as possible of this work and error From d4a3e4d0449db3b3b40fb7a6d8b366fcc56ebbf9 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 3 Dec 2012 14:08:23 -0500 Subject: [PATCH 70/89] fix logger name --- lms/djangoapps/instructor/staff_grading_service.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/instructor/staff_grading_service.py b/lms/djangoapps/instructor/staff_grading_service.py index c070bd6835..61654997bd 100644 --- a/lms/djangoapps/instructor/staff_grading_service.py +++ b/lms/djangoapps/instructor/staff_grading_service.py @@ -15,7 +15,7 @@ from courseware.access import has_access from util.json_request import expect_json from xmodule.course_module import CourseDescriptor -log = logging.getLogger("mitx.courseware") +log = logging.getLogger(__name__) class GradingServiceError(Exception): @@ -54,7 +54,6 @@ class StaffGradingService(object): self.get_next_url = self.url + '/get_next_submission/' self.save_grade_url = self.url + '/save_grade/' - # TODO: add auth self.session = requests.session() @@ -114,6 +113,7 @@ class StaffGradingService(object): return r.text + def save_grade(self, course_id, grader_id, submission_id, score, feedback): """ Save a score and feedback for a submission. From c57b9f324034a6fc620e60f4dd30ee1b479c2f7e Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 4 Dec 2012 14:43:40 -0500 Subject: [PATCH 71/89] merge prompt and ml accuracy display --- .../src/staff_grading/staff_grading.coffee | 19 ++++++++++++++++--- lms/templates/instructor/staff_grading.html | 6 ++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index c953b38c2e..161fb33b7c 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -66,10 +66,16 @@ class StaffGrading # all the jquery selectors @error_container = $('.error-container') @message_container = $('.message-container') + + @prompt_container = $('.prompt-container') + @prompt_wrapper = $('.prompt-wrapper') + @submission_container = $('.submission-container') - @rubric_container = $('.rubric-container') @submission_wrapper = $('.submission-wrapper') + + @rubric_container = $('.rubric-container') @rubric_wrapper = $('.rubric-wrapper') + @feedback_area = $('.feedback-area') @score_selection_container = $('.score-selection-container') @submit_button = $('.submit-button') @@ -78,6 +84,7 @@ class StaffGrading # model state @state = state_no_data @submission_id = null + @prompt = '' @submission = '' @rubric = '' @error_msg = '' @@ -130,7 +137,7 @@ class StaffGrading if response.success if response.submission - @data_loaded(response.submission, response.rubric, response.submission_id, response.max_score, response.ml_error_info) + @data_loaded(response.prompt, response.submission, response.rubric, response.submission_id, response.max_score, response.ml_error_info) else @no_more(response.message) else @@ -153,7 +160,8 @@ class StaffGrading @error_msg = msg @state = state_error - data_loaded: (submission, rubric, submission_id, max_score, ml_error_info) -> + data_loaded: (prompt, submission, rubric, submission_id, max_score, ml_error_info) -> + @prompt = prompt @submission = submission @rubric = rubric @submission_id = submission_id @@ -162,8 +170,11 @@ class StaffGrading @score = null @ml_error_info=ml_error_info @state = state_grading + if not @max_score? + @error("No max score specified for submission.") no_more: (message) -> + @prompt = null @submission = null @rubric = null @ml_error_info = null @@ -189,6 +200,7 @@ class StaffGrading else if @state == state_grading @ml_error_info_container.html(@ml_error_info) + @prompt_container.html(@prompt) @submission_container.html(@submission) @rubric_container.html(@rubric) show_grading_elements = true @@ -210,6 +222,7 @@ class StaffGrading @error('System got into invalid state ' + @state) @submit_button.toggle(show_submit_button) + @prompt_wrapper.toggle(show_grading_elements) @submission_wrapper.toggle(show_grading_elements) @rubric_wrapper.toggle(show_grading_elements) @ml_error_info_container.toggle(show_grading_elements) diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index ad704ae5a4..a44ef68831 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -28,6 +28,12 @@
+
+

Question prompt

+
+
+
+

Submission

From 1d87dab3a77468c7221a880ec631610e32e748c4 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 4 Dec 2012 17:45:51 -0500 Subject: [PATCH 72/89] remove queueing spinner and polling logic, change message - no polling, since instructors may take a long time to grade --- common/lib/capa/capa/inputtypes.py | 7 ++++--- .../lib/capa/capa/templates/openendedinput.html | 5 +---- common/lib/xmodule/xmodule/css/capa/display.scss | 15 +++++---------- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index 70fe5dd6c8..73056bc09e 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -746,8 +746,9 @@ class OpenEndedInput(InputTypeBase): tags = ['openendedinput'] # pulled out for testing - submitted_msg = ("Submitted. As soon as your submission is" - " graded, this message will be replaced with the grader's feedback.") + submitted_msg = ("Feedback not yet available. Reload to check again. " + "Once the problem is graded, this message will be " + "replaced with the grader's feedback") @classmethod def get_attributes(cls): @@ -781,4 +782,4 @@ class OpenEndedInput(InputTypeBase): registry.register(OpenEndedInput) -#----------------------------------------------------------------------------- \ No newline at end of file +#----------------------------------------------------------------------------- diff --git a/common/lib/capa/capa/templates/openendedinput.html b/common/lib/capa/capa/templates/openendedinput.html index 697bff8082..8cc19a2705 100644 --- a/common/lib/capa/capa/templates/openendedinput.html +++ b/common/lib/capa/capa/templates/openendedinput.html @@ -13,15 +13,12 @@ % elif status == 'incorrect': Incorrect % elif status == 'queued': - Queued - + Submitted for grading % endif % if hidden:
% endif - -

${status}

diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index 58ba7b00ed..bbde1063ed 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -121,16 +121,6 @@ section.problem { } } - &.processing { - p.status { - @include inline-block(); - background: url('../images/spinner.gif') center center no-repeat; - height: 20px; - width: 20px; - text-indent: -9999px; - } - } - &.correct, &.ui-icon-check { p.status { @include inline-block(); @@ -266,6 +256,11 @@ section.problem { margin: -7px 7px 0 0; } + .grading { + text-indent: 0px; + margin: 0px 7px 0 0; + } + p { line-height: 20px; text-transform: capitalize; From 40f34d19a3c2591c0f923effb14c45cf2a1564cb Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 4 Dec 2012 17:46:34 -0500 Subject: [PATCH 73/89] add initial mock of get_problems call --- .../src/staff_grading/staff_grading.coffee | 17 ++++++++++++++++- 1 file changed, 16 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 161fb33b7c..bca577b0c6 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -18,6 +18,7 @@ class StaffGradingBackend mock: (cmd, data) -> # Return a mock response to cmd and data + # TODO: needs (optional?) arg for problem location if cmd == 'get_next' @mock_cnt++ response = @@ -26,12 +27,26 @@ class StaffGradingBackend rubric: 'A rubric! ' + @mock_cnt submission_id: @mock_cnt max_score: 2 + @mock_cnt % 3 - ml_error_info : 'ML error info!' + @mock_cnt + ml_error_info : 'ML accuracy info: ' + @mock_cnt else if cmd == 'save_grade' console.log("eval: #{data.score} pts, Feedback: #{data.feedback}") response = @mock('get_next', {}) + else if cmd == 'get_problems' + # this one didn't have a name in the LMS--lookup fail + p1 = {'location': 'i4x://MITx/3.091x/problem/open_ended_demo',\ + 'name': 'i4x://MITx/3.091x/problem/open_ended_demo',\ + 'num_graded': 10,\ + 'num_to_grade': 90} + + p2 = {'location': 'i4x://MITx/3.091x/problem/open_ended_demo2',\ + 'name': 'Open ended demo',\ + 'num_graded': 42,\ + 'num_to_grade': 63} + + response = + problems: [p1, p2] else response = success: false From 90076c39e6c46cf17d87c8d46c57786a5f32702b Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Wed, 5 Dec 2012 09:25:05 -0500 Subject: [PATCH 74/89] add in some new mock responses for our staff grading API --- .../coffee/src/staff_grading/staff_grading.coffee | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lms/static/coffee/src/staff_grading/staff_grading.coffee b/lms/static/coffee/src/staff_grading/staff_grading.coffee index 161fb33b7c..be7837ec93 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -18,10 +18,15 @@ class StaffGradingBackend mock: (cmd, data) -> # Return a mock response to cmd and data + # should take a problem id as an argument if cmd == 'get_next' @mock_cnt++ response = success: true + problem_name: 'Problem 1' + num_left: 3 + num_total: 5 + prompt: 'This is a fake prompt' submission: 'submission! ' + @mock_cnt rubric: 'A rubric! ' + @mock_cnt submission_id: @mock_cnt @@ -32,6 +37,16 @@ class StaffGradingBackend console.log("eval: #{data.score} pts, Feedback: #{data.feedback}") response = @mock('get_next', {}) + # get_probblem_list + # sends in a course_id and a grader_id + # should get back a list of problem_ids, problem_names, num_left, num_total + else if cmd == 'get_problem_list' + response = + success: true + problem_list: [ + {problem_id: 1, problem_name: "Problem 1", num_left: 3, num_total: 5} + {problem_id: 2, problem_name: "Problem 2", num_left: 1, num_total: 5} + ] else response = success: false From 7887f556d9e224b39d183737afd80edbaa1999ff Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 5 Dec 2012 09:29:42 -0500 Subject: [PATCH 75/89] Add docs on using a queue server on aws from a local LMS --- doc/development.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/doc/development.md b/doc/development.md index b4ac52d202..ebc56fbf1b 100644 --- a/doc/development.md +++ b/doc/development.md @@ -67,6 +67,15 @@ To run a single nose test: Very handy: if you uncomment the `--pdb` argument in `NOSE_ARGS` in `lms/envs/test.py`, it will drop you into pdb on error. This lets you go up and down the stack and see what the values of the variables are. Check out http://docs.python.org/library/pdb.html +## Testing using queue servers + +When testing problems that use a queue server on AWS (e.g. sandbox-xqueue.edx.org), you'll need to run your server on your public IP, like so. + +`django-admin.py runserver --settings=lms.envs.dev --pythonpath=. 0.0.0.0:8000` + +When you connect to the LMS, you need to use the public ip. Use `ifconfig` to figure out the numnber, and connect e.g. to `http://18.3.4.5:8000/` + + ## Content development If you change course content, while running the LMS in dev mode, it is unnecessary to restart to refresh the modulestore. From 5ebde2a93e1f72008c89c2971c1cd95d82e8e316 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Wed, 5 Dec 2012 09:42:30 -0500 Subject: [PATCH 76/89] fix comment --- 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 7e8f39acac..f4831e35c4 100644 --- a/lms/static/coffee/src/staff_grading/staff_grading.coffee +++ b/lms/static/coffee/src/staff_grading/staff_grading.coffee @@ -18,7 +18,7 @@ class StaffGradingBackend mock: (cmd, data) -> # Return a mock response to cmd and data - # TODO: needs (optional?) arg for problem location + # should take a location as an argument if cmd == 'get_next' @mock_cnt++ response = From 3795b3a7851b922cfe2a5ca81ef8e64e27a2e039 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 5 Dec 2012 10:56:12 -0500 Subject: [PATCH 77/89] Address Dave's comments - factor out some common code - improve variable names --- common/lib/capa/capa/responsetypes.py | 53 +++++++-------------------- common/lib/capa/capa/util.py | 22 +++++++++++ 2 files changed, 36 insertions(+), 39 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 98421ce0b3..cf804f381d 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1140,7 +1140,7 @@ class CodeResponse(LoncapaResponse): else: self._parse_coderesponse_xml(codeparam) - def _parse_coderesponse_xml(self,codeparam): + def _parse_coderesponse_xml(self, codeparam): ''' Parse the new CodeResponse XML format. When successful, sets: self.initial_display @@ -1152,17 +1152,9 @@ class CodeResponse(LoncapaResponse): grader_payload = grader_payload.text if grader_payload is not None else '' self.payload = {'grader_payload': grader_payload} - answer_display = codeparam.find('answer_display') - if answer_display is not None: - self.answer = answer_display.text - else: - self.answer = 'No answer provided.' - - initial_display = codeparam.find('initial_display') - if initial_display is not None: - self.initial_display = initial_display.text - else: - self.initial_display = '' + self.initial_display = find_with_default(codeparam, 'initial_display', '') + self.answer = find_with_default(codeparam, 'answer_display', + 'No answer provided.') def _parse_externalresponse_xml(self): ''' @@ -1890,44 +1882,27 @@ class OpenEndedResponse(LoncapaResponse): #Update grader payload with student id. If grader payload not json, error. try: - grader_payload = json.loads(grader_payload) + parsed_grader_payload = json.loads(grader_payload) # NOTE: self.system.location is valid because the capa_module # __init__ adds it (easiest way to get problem location into # response types) - grader_payload.update({ + parsed_grader_payload.update({ 'location' : self.system.location, 'course_id' : self.system.course_id, 'prompt' : prompt_string, 'rubric' : rubric_string, }) - grader_payload = json.dumps(grader_payload) + updated_grader_payload = json.dumps(parsed_grader_payload) except Exception as err: - log.error("Grader payload is not a json object!") + log.exception("Grader payload %r is not a json object!", grader_payload) - self.payload = {'grader_payload': grader_payload} + self.payload = {'grader_payload': updated_grader_payload} - #Parse initial display - initial_display = oeparam.find('initial_display') - if initial_display is not None: - self.initial_display = initial_display.text - else: - self.initial_display = '' - - #Parse answer display - answer_display = oeparam.find('answer_display') - if answer_display is not None: - self.answer= answer_display.text - else: - self.answer = "No answer given." - - #Parse max_score - top_score = oeparam.find('max_score') - if top_score is not None: - try: - self.max_score = int(top_score.text) - except: - self.max_score = 1 - else: + self.initial_display = find_with_default(oeparam, 'initial_display', '') + self.answer = find_with_default(oeparam, 'answer_display', 'No answer given.') + try: + self.max_score = int(find_with_default(oeparam, 'max_score', 1)) + except ValueError: self.max_score = 1 def get_score(self, student_answers): diff --git a/common/lib/capa/capa/util.py b/common/lib/capa/capa/util.py index 10e984611b..0df58c216f 100644 --- a/common/lib/capa/capa/util.py +++ b/common/lib/capa/capa/util.py @@ -65,3 +65,25 @@ def is_file(file_to_test): Duck typing to check if 'file_to_test' is a File object ''' return all(hasattr(file_to_test, method) for method in ['read', 'name']) + + +def find_with_default(node, path, default): + """ + Look for a child of node using , and return its text if found. + Otherwise returns default. + + Arguments: + node: lxml node + path: xpath search expression + default: value to return if nothing found + + Returns: + node.find(path).text if the find succeeds, default otherwise. + + """ + v = node.find(path) + if v is not None: + return v.text + else: + return default + From e813dc35147a739064f4b9d18cfc3796f539ec59 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 5 Dec 2012 12:08:26 -0500 Subject: [PATCH 78/89] Address format issue and exception --- common/lib/capa/capa/responsetypes.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index cf804f381d..dda3ce0e43 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1910,9 +1910,8 @@ class OpenEndedResponse(LoncapaResponse): try: submission = student_answers[self.answer_id] except Exception as err: - log.error('Error in OpenEndedResponse %s: cannot get student answer for %s;' - ' student_answers=%s', err, self.answer_id) - raise Exception(err) + log.error('Error in OpenEndedResponse {0}: cannot get student answer for {1}'.format(err,self.answer_id)) + raise # Prepare xqueue request #------------------------------------------------------------ @@ -1958,8 +1957,8 @@ class OpenEndedResponse(LoncapaResponse): cmap = CorrectMap() if error: cmap.set(self.answer_id, queuestate=None, - msg='Unable to deliver your submission to grader. (Reason: %s.)' - ' Please try again later.' % msg) + msg='Unable to deliver your submission to grader. (Reason: {0}.)' + ' Please try again later.'.format(msg)) else: # Queueing mechanism flags: # 1) Backend: Non-null CorrectMap['queuestate'] indicates that From 1eaf424c2036dc0bc666ba9b4283c68f579912ed Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 5 Dec 2012 13:17:16 -0500 Subject: [PATCH 79/89] Change some things to use format --- common/lib/capa/capa/responsetypes.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index dda3ce0e43..b69119cf6d 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1996,13 +1996,13 @@ class OpenEndedResponse(LoncapaResponse): oldcmap.set(self.answer_id, npoints=points, correctness=correctness, msg=msg.replace(' ', ' '), queuestate=None) else: - log.debug('OpenEndedResponse: queuekey %s does not match for answer_id=%s.' % - (queuekey, self.answer_id)) + log.debug('OpenEndedResponse: queuekey {0} does not match for answer_id={1}.'.format( + queuekey, self.answer_id)) return oldcmap def get_answers(self): - anshtml = '
%s
' % self.answer + anshtml = '
{0}
'.format(self.answer) return {self.answer_id: anshtml} def get_initial_display(self): @@ -2089,11 +2089,11 @@ class OpenEndedResponse(LoncapaResponse): score_result = json.loads(score_msg) except (TypeError, ValueError): log.error("External grader message should be a JSON-serialized dict." - " Received score_msg = %s" % score_msg) + " Received score_msg = {0}".format(score_msg)) return fail if not isinstance(score_result, dict): log.error("External grader message should be a JSON-serialized dict." - " Received score_result = %s" % score_result) + " Received score_result = {0}".format(score_result)) return fail for tag in ['score', 'feedback', 'grader_type', 'success']: if tag not in score_result: From 0bc06579395083619c4c2f8be9e85b3c7a4ed3d1 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Fri, 7 Dec 2012 09:21:23 -0500 Subject: [PATCH 80/89] Add in support for ordering tags properly in feedback box --- common/lib/capa/capa/responsetypes.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index b69119cf6d..e4a4469d03 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -2017,6 +2017,10 @@ class OpenEndedResponse(LoncapaResponse): String """ + #Tags that need to be shown at the end of the feedback block (in this order) + tags_displayed_last=['markup-text', 'markup_text'] + tags_displayed_first=['spelling', 'grammar'] + feedback_item_start='
' feedback_item_end='
' @@ -2032,12 +2036,21 @@ class OpenEndedResponse(LoncapaResponse): success=response_items['success'] + if success: feedback_long="" + #Add in feedback that needs to be shown first 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 k in tags_displayed_first: + feedback_long+= feedback_item_start.format(feedback_key=k) +str(v) + feedback_item_end + #Add in feedback whose order does not matter + for k,v in feedback_items.items(): + if k not in tags_displayed_last and k not in tags_displayed_first: + feedback_long+= feedback_item_start.format(feedback_key=k) +str(v) + feedback_item_end + #Add in feedback that needs to be displayed last + for k,v in feedback_items.items(): + if k in tags_displayed_last: + feedback_long+= feedback_item_start.format(feedback_key=k) +str(v) + feedback_item_end if len(feedback_items)==0: feedback_long=feedback_item_start.format(feedback_key="feedback") + "No feedback available." + feedback_item_end From 6d2688cb66b7b8f492c6b9c003f615aab6d42287 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 6 Dec 2012 11:05:28 -0500 Subject: [PATCH 81/89] unicode fix and cleanup in stringify children --- common/lib/capa/capa/responsetypes.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index e4a4469d03..f55bea4187 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1845,24 +1845,18 @@ class OpenEndedResponse(LoncapaResponse): rubric = self.xml.find('openendedrubric') self._parse(oeparam, prompt, rubric) - def stringify_children(self, node, strip_tags=True): + @staticmethod + def stringify_children(node): """ Modify code from stringify_children in xmodule. Didn't import directly in order to avoid capa depending on xmodule (seems to be avoided in code) """ parts=[node.text] - [parts.append((etree.tostring(p, with_tail=True))) for p in node.getchildren()] - node_string=' '.join(parts) + for p in node.getchildren(): + parts.append(etree.tostring(p, with_tail=True, encoding='unicode')) - # Strip html tags from result. This may need to be removed in order to - # display prompt to instructors properly. - # TODO: what breaks if this is removed? The ML code can strip tags - # as part of its input filtering. - if strip_tags: - node_string=re.sub('<[^<]+?>', '', node_string) - - return node_string + return ' '.join(parts) def _parse(self, oeparam, prompt, rubric): ''' From 3da727125f86fa9f783b37d3c1409ea206292823 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 6 Dec 2012 11:06:13 -0500 Subject: [PATCH 82/89] fix feedback formatting --- common/lib/capa/capa/responsetypes.py | 99 +++++++++++++++------------ 1 file changed, 55 insertions(+), 44 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index f55bea4187..c05918382f 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -2002,57 +2002,67 @@ class OpenEndedResponse(LoncapaResponse): def get_initial_display(self): return {self.answer_id: self.initial_display} - def _convert_longform_feedback_to_html(self,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. + Take in a dictionary, and return html strings for display to student. Input: - Dictionary with keys success, feedback, and errors + response_items: Dictionary with keys success, feedback. + if success is True, feedback should be a dictionary, with keys for + types of feedback, and the corresponding feedback values. + if success is False, feedback is actually an error string. + + NOTE: this will need to change when we integrate peer grading, because + that will have more complex feedback. + Output: - String + String -- html that can be displayed to the student. """ - #Tags that need to be shown at the end of the feedback block (in this order) - tags_displayed_last=['markup-text', 'markup_text'] - tags_displayed_first=['spelling', 'grammar'] + # We want to display available feedback in a particular order. + # This dictionary specifies which goes first--lower first. + priorities = {# These go at the start of the feedback + 'spelling': 0, + 'grammar': 1, + # needs to be after all the other feedback + 'markup_text': 3} - feedback_item_start='
' - feedback_item_end='
' + default_priority = 2 - for tag in ['status', 'feedback']: + def get_priority(elt): + """ + Args: + elt: a tuple of feedback-type, feedback + Returns: + the priority for this feedback type + """ + return priorities.get(elt[0], default_priority) + + def format_feedback(feedback_type, value): + return """ +
+ {value} +
+ """.format(feedback_type, value) + + for tag in ['success', 'feedback']: if tag not in response_items: - feedback_long=feedback_item_start.format(feedback_key="errors") + "Error getting feedback." + feedback_item_end + return format_feedback('errors', 'Error getting feedback') - feedback_items=response_items['feedback'] + feedback_items = response_items['feedback'] try: - feedback_items=json.loads(feedback_items) - except: - pass + feedback = json.loads(feedback_items) + except ValueError: + log.exception("feedback_items have invalid json %r", feedback_items) + return format_feedback('errors', 'Could not parse feedback') - success=response_items['success'] - - - if success: - feedback_long="" - #Add in feedback that needs to be shown first - for k,v in feedback_items.items(): - if k in tags_displayed_first: - feedback_long+= feedback_item_start.format(feedback_key=k) +str(v) + feedback_item_end - #Add in feedback whose order does not matter - for k,v in feedback_items.items(): - if k not in tags_displayed_last and k not in tags_displayed_first: - feedback_long+= feedback_item_start.format(feedback_key=k) +str(v) + feedback_item_end - #Add in feedback that needs to be displayed last - for k,v in feedback_items.items(): - if k in tags_displayed_last: - feedback_long+= feedback_item_start.format(feedback_key=k) +str(v) + feedback_item_end - - if len(feedback_items)==0: - feedback_long=feedback_item_start.format(feedback_key="feedback") + "No feedback available." + feedback_item_end + if response_items['success']: + if len(feedback) == 0: + return format_feedback('errors', 'No feedback available') + feedback_lst = sorted(feedback.items(), key=get_priority) + return u"\n".join(format_feedback(k, v) for k, v in feedback_lst) else: - feedback_long=feedback_item_start.format(feedback_key="errors") + response_items['feedback'] + feedback_item_end - - return feedback_long + return format_feedback('errors', response_items['feedback']) def _format_feedback(self, response_items): @@ -2063,15 +2073,16 @@ class OpenEndedResponse(LoncapaResponse): Return error message or feedback template """ - feedback=self._convert_longform_feedback_to_html(response_items) + feedback = self._convert_longform_feedback_to_html(response_items) if not response_items['success']: - return self.system.render_template("open_ended_error.html", {'errors' : feedback}) + return self.system.render_template("open_ended_error.html", + {'errors' : feedback}) - feedback_template=self.system.render_template("open_ended_feedback.html",{ - 'grader_type' : response_items['grader_type'], - 'score' : response_items['score'], - 'feedback' : feedback, + feedback_template = self.system.render_template("open_ended_feedback.html", { + 'grader_type': response_items['grader_type'], + 'score': response_items['score'], + 'feedback': feedback, }) return feedback_template From 42f8afb08346567b9bd88e5873fc6128b767d8c7 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 6 Dec 2012 11:07:28 -0500 Subject: [PATCH 83/89] address minor code review comments --- common/lib/capa/capa/responsetypes.py | 31 ++++++++++++++------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index c05918382f..89bc7e4a2c 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1880,15 +1880,15 @@ class OpenEndedResponse(LoncapaResponse): # NOTE: self.system.location is valid because the capa_module # __init__ adds it (easiest way to get problem location into # response types) - parsed_grader_payload.update({ - 'location' : self.system.location, - 'course_id' : self.system.course_id, - 'prompt' : prompt_string, - 'rubric' : rubric_string, - }) - updated_grader_payload = json.dumps(parsed_grader_payload) - except Exception as err: + except ValueError: log.exception("Grader payload %r is not a json object!", grader_payload) + parsed_grader_payload.update({ + 'location' : self.system.location, + 'course_id' : self.system.course_id, + 'prompt' : prompt_string, + 'rubric' : rubric_string, + }) + updated_grader_payload = json.dumps(parsed_grader_payload) self.payload = {'grader_payload': updated_grader_payload} @@ -1903,9 +1903,11 @@ class OpenEndedResponse(LoncapaResponse): try: submission = student_answers[self.answer_id] - except Exception as err: - log.error('Error in OpenEndedResponse {0}: cannot get student answer for {1}'.format(err,self.answer_id)) - raise + except KeyError: + msg = ('Cannot get student answer for answer_id: {0}. student_answers {1}' + .format(self.answer_id, student_answers)) + log.exception(msg) + raise LoncapaProblemError(msg) # Prepare xqueue request #------------------------------------------------------------ @@ -1959,7 +1961,7 @@ class OpenEndedResponse(LoncapaResponse): # the problem has been queued # 2) Frontend: correctness='incomplete' eventually trickles down # through inputtypes.textbox and .filesubmission to inform the - # browser to poll the LMS + # browser that the submission is queued (and it could e.g. poll) cmap.set(self.answer_id, queuestate=queuestate, correctness='incomplete', msg=msg) @@ -1984,9 +1986,8 @@ class OpenEndedResponse(LoncapaResponse): # Sanity check on returned points if points < 0: points = 0 - elif points > self.maxpoints[self.answer_id]: - points = self.maxpoints[self.answer_id] - # Queuestate is consumed + + # Queuestate is consumed, so reset it to None oldcmap.set(self.answer_id, npoints=points, correctness=correctness, msg=msg.replace(' ', ' '), queuestate=None) else: From bf593c6603a79a31d425bb20d2acb43227afdbeb Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 6 Dec 2012 11:27:58 -0500 Subject: [PATCH 84/89] clean up imports --- common/lib/capa/capa/responsetypes.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 89bc7e4a2c..6d15aba54f 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -8,21 +8,23 @@ Used by capa_problem.py ''' # standard library imports +import abc import cgi +import hashlib import inspect import json import logging import numbers import numpy +import os import random import re import requests -import traceback -import hashlib -import abc -import os import subprocess +import traceback import xml.sax.saxutils as saxutils + +from collections import namedtuple from shapely.geometry import Point, MultiPoint # specific library imports From 7190e484f11b526b783bf16d18917c9fa8b06f53 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 6 Dec 2012 11:32:33 -0500 Subject: [PATCH 85/89] Use a namedtuple for parsing ScoreMessages --- common/lib/capa/capa/responsetypes.py | 50 +++++++++++++++------------ 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 6d15aba54f..5e496df649 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1103,6 +1103,15 @@ class SymbolicResponse(CustomResponse): #----------------------------------------------------------------------------- +""" +valid: Flag indicating valid score_msg format (Boolean) +correct: Correctness of submission (Boolean) +score: Points to be assigned (numeric, can be float) +msg: Message from grader to display to student (string) +""" +ScoreMessage = namedtuple('ScoreMessage', + ['valid', 'correct', 'points', 'msg']) + class CodeResponse(LoncapaResponse): """ @@ -1882,7 +1891,7 @@ class OpenEndedResponse(LoncapaResponse): # NOTE: self.system.location is valid because the capa_module # __init__ adds it (easiest way to get problem location into # response types) - except ValueError: + except TypeError, ValueError: log.exception("Grader payload %r is not a json object!", grader_payload) parsed_grader_payload.update({ 'location' : self.system.location, @@ -1971,13 +1980,13 @@ class OpenEndedResponse(LoncapaResponse): def update_score(self, score_msg, oldcmap, queuekey): log.debug(score_msg) - (valid_score_msg, correct, points, msg) = self._parse_score_msg(score_msg) - if not valid_score_msg: + score_msg = self._parse_score_msg(score_msg) + if not score_msg.valid: oldcmap.set(self.answer_id, msg = 'Invalid grader reply. Please contact the course staff.') return oldcmap - correctness = 'correct' if correct else 'incorrect' + correctness = 'correct' if score_msg.correct else 'incorrect' # TODO: Find out how this is used elsewhere, if any self.context['correct'] = correctness @@ -1986,12 +1995,13 @@ class OpenEndedResponse(LoncapaResponse): # does not match, we keep waiting for the score_msg whose key actually matches if oldcmap.is_right_queuekey(self.answer_id, queuekey): # Sanity check on returned points + points = score_msg.points if points < 0: points = 0 # Queuestate is consumed, so reset it to None oldcmap.set(self.answer_id, npoints=points, correctness=correctness, - msg=msg.replace(' ', ' '), queuestate=None) + msg = score_msg.msg.replace(' ', ' '), queuestate=None) else: log.debug('OpenEndedResponse: queuekey {0} does not match for answer_id={1}.'.format( queuekey, self.answer_id)) @@ -2047,6 +2057,10 @@ class OpenEndedResponse(LoncapaResponse):
""".format(feedback_type, value) + # TODO (vshnayder): design and document the details of this format so + # that we can do proper escaping here (e.g. are the graders allowed to + # include HTML?) + for tag in ['success', 'feedback']: if tag not in response_items: return format_feedback('errors', 'Error getting feedback') @@ -2054,7 +2068,7 @@ class OpenEndedResponse(LoncapaResponse): feedback_items = response_items['feedback'] try: feedback = json.loads(feedback_items) - except ValueError: + except (TypeError, ValueError): log.exception("feedback_items have invalid json %r", feedback_items) return format_feedback('errors', 'Could not parse feedback') @@ -2105,45 +2119,35 @@ class OpenEndedResponse(LoncapaResponse): correct: Correctness of submission (Boolean) score: Points to be assigned (numeric, can be float) """ - fail = (False, False, 0, '') + fail = ScoreMessage(valid=False, correct=False, points=0, msg='') try: score_result = json.loads(score_msg) except (TypeError, ValueError): log.error("External grader message should be a JSON-serialized dict." " Received score_msg = {0}".format(score_msg)) return fail + if not isinstance(score_result, dict): log.error("External grader message should be a JSON-serialized dict." " Received score_result = {0}".format(score_result)) return fail + 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)) + 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 - # 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 = self._format_feedback(score_result) - # HACK: for now, just assume it's correct if you got more than 2/3. # Also assumes that score_result['score'] is an integer. score_ratio = int(score_result['score']) / self.max_score correct = (score_ratio >= 0.66) - log.debug(feedback) - try: - etree.fromstring(feedback) - except etree.XMLSyntaxError as err: - log.error("Unable to parse external grader message as valid" - "\n Feedback : score_result['feedback'] = %r",feedback) - return fail - #Currently ignore msg and only return feedback (which takes the place of msg) - return (True, correct, score_result['score'], feedback) + return ScoreMessage(valid=True, correct=correct, + score=score_result['score'], msg=feedback) #----------------------------------------------------------------------------- # TEMPORARY: List of all response subclasses From bd94474a50856e46576f26bae525858d8603dd0d Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 6 Dec 2012 11:38:21 -0500 Subject: [PATCH 86/89] improve docstring --- common/lib/xmodule/xmodule/self_assessment_module.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py index 8498a210cd..eb8a275d35 100644 --- a/common/lib/xmodule/xmodule/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/self_assessment_module.py @@ -373,6 +373,14 @@ class SelfAssessmentModule(XModule): def save_answer(self, get): """ After the answer is submitted, show the rubric. + + Args: + get: the GET dictionary passed to the ajax request. Should contain + a key 'student_answer' + + Returns: + Dictionary with keys 'success' and either 'error' (if not success), + or 'rubric_html' (if success). """ # Check to see if attempts are less than max if self.attempts > self.max_attempts: From 53580af95253cab12da3cd0ce3461b0fa608b7c5 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 6 Dec 2012 11:59:18 -0500 Subject: [PATCH 87/89] fix aws setting name --- lms/envs/aws.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index d1abce8a6d..0516bddc56 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -76,7 +76,7 @@ DATABASES = AUTH_TOKENS['DATABASES'] XQUEUE_INTERFACE = AUTH_TOKENS['XQUEUE_INTERFACE'] -STAFF_GRADING_BACKEND = AUTH_TOKENS.get('STAFF_GRADING_INTERFACE') +STAFF_GRADING_INTERFACE = AUTH_TOKENS.get('STAFF_GRADING_INTERFACE') PEARSON_TEST_USER = "pearsontest" From 730cdd3b4fff2be9a1aebbbaca8ea4b3a6d30345 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 6 Dec 2012 12:00:03 -0500 Subject: [PATCH 88/89] minor changes from pull request comments - comments - more info in log msg - better error --- .../instructor/staff_grading_service.py | 43 +++++++++++++------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/lms/djangoapps/instructor/staff_grading_service.py b/lms/djangoapps/instructor/staff_grading_service.py index 61654997bd..a5b29e2c5b 100644 --- a/lms/djangoapps/instructor/staff_grading_service.py +++ b/lms/djangoapps/instructor/staff_grading_service.py @@ -98,8 +98,19 @@ class StaffGradingService(object): def get_next(self, course_id, grader_id): """ - Get the next thing to grade. Returns json, or raises GradingServiceError - if there's a problem. + Get the next thing to grade. + + Args: + course_id: course id to get submission for + grader_id: who is grading this? The anonymous user_id of the grader. + + Returns: + json string 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) + + Raises: + GradingServiceError: something went wrong with the connection. """ op = lambda: self.session.get(self.get_next_url, allow_redirects=False, @@ -113,16 +124,18 @@ class StaffGradingService(object): return r.text - + def save_grade(self, course_id, grader_id, submission_id, score, feedback): """ Save a score and feedback for a submission. - Returns json dict with keys - 'success': bool - 'error': error msg, if something went wrong. + Returns: + json dict with keys + 'success': bool + 'error': error msg, if something went wrong. - Raises GradingServiceError if there's a problem connecting. + Raises: + GradingServiceError if there's a problem connecting. """ try: data = {'course_id': course_id, @@ -216,8 +229,10 @@ def _get_next(course_id, grader_id): try: return grading_service().get_next(course_id, grader_id) except GradingServiceError: - log.exception("Error from grading service") - return json.dumps({'success': False, 'error': 'Could not connect to grading service'}) + log.exception("Error from grading service. server url: {0}" + .format(grading_service().url)) + return json.dumps({'success': False, + 'error': 'Could not connect to grading service'}) @expect_json @@ -239,10 +254,12 @@ def save_grade(request, course_id): if request.method != 'POST': raise Http404 - required = ('score', 'feedback', 'submission_id') - for k in required: - if k not in request.POST.keys(): - return _err_response('Missing required key {0}'.format(k)) + required = set('score', 'feedback', 'submission_id') + actual = set(request.POST.keys()) + missing = required - actual + if len(missing) != 0: + return _err_response('Missing required keys {0}'.format( + ', '.join(missing))) grader_id = request.user.id p = request.POST From 44a8f31d0693db18ce1cf334cb8fa3e2bbf942df Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 6 Dec 2012 13:39:07 -0500 Subject: [PATCH 89/89] Change are-we-logged-in detection to be less hackish. - has matching changes in controller staff_grading view. --- lms/djangoapps/instructor/staff_grading_service.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/instructor/staff_grading_service.py b/lms/djangoapps/instructor/staff_grading_service.py index a5b29e2c5b..b3b0f99e74 100644 --- a/lms/djangoapps/instructor/staff_grading_service.py +++ b/lms/djangoapps/instructor/staff_grading_service.py @@ -77,15 +77,16 @@ class StaffGradingService(object): def _try_with_login(self, operation): """ Call operation(), which should return a requests response object. If - the response status code is 302, call _login() and try the operation - again. NOTE: use requests.get(..., allow_redirects=False) to have - requests not auto-follow redirects. + the request fails with a 'login_required' error, call _login() and try + the operation again. Returns the result of operation(). Does not catch exceptions. """ response = operation() - if response.status_code == 302: - # redirect means we aren't logged in + if (response.json + and response.json.get('success') == False + and response.json.get('error') == 'login_required'): + # apparrently we aren't logged in. Try to fix that. r = self._login() if r and not r.get('success'): log.warning("Couldn't log into staff_grading backend. Response: %s",