From ebf5ad4fa291e7312470e7686212818dbf96e4d2 Mon Sep 17 00:00:00 2001 From: Jean-Michel Vedrine Date: Sat, 17 Jan 2015 19:51:20 +0100 Subject: [PATCH] MDL-43387 mod_lesson: add an editor for essay grading comments --- .../backup/moodle2/backup_lesson_stepslib.php | 1 + .../moodle2/restore_lesson_stepslib.php | 2 + mod/lesson/essay.php | 33 ++++++--- mod/lesson/essay_form.php | 7 +- mod/lesson/lang/en/lesson.php | 1 + mod/lesson/lib.php | 22 ++++++ mod/lesson/locallib.php | 29 ++++++-- mod/lesson/pagetypes/essay.php | 40 ++++++++--- mod/lesson/report.php | 5 +- .../tests/behat/lesson_essay_question.feature | 10 ++- mod/lesson/tests/pagetypes_test.php | 70 +++++++++++++++++++ 11 files changed, 191 insertions(+), 29 deletions(-) create mode 100644 mod/lesson/tests/pagetypes_test.php diff --git a/mod/lesson/backup/moodle2/backup_lesson_stepslib.php b/mod/lesson/backup/moodle2/backup_lesson_stepslib.php index cfb746d0b05..c95f6a0b4ec 100644 --- a/mod/lesson/backup/moodle2/backup_lesson_stepslib.php +++ b/mod/lesson/backup/moodle2/backup_lesson_stepslib.php @@ -193,6 +193,7 @@ class backup_lesson_activity_structure_step extends backup_activity_structure_st $page->annotate_files('mod_lesson', 'page_contents', 'id'); $answer->annotate_files('mod_lesson', 'page_answers', 'id'); $answer->annotate_files('mod_lesson', 'page_responses', 'id'); + $attempt->annotate_files('mod_lesson', 'essay_responses', 'id'); // Prepare and return the structure we have just created for the lesson module. return $this->prepare_activity_structure($lesson); diff --git a/mod/lesson/backup/moodle2/restore_lesson_stepslib.php b/mod/lesson/backup/moodle2/restore_lesson_stepslib.php index 8be645e9bf0..815e2550051 100644 --- a/mod/lesson/backup/moodle2/restore_lesson_stepslib.php +++ b/mod/lesson/backup/moodle2/restore_lesson_stepslib.php @@ -131,6 +131,7 @@ class restore_lesson_activity_structure_step extends restore_activity_structure_ $data->timeseen = $this->apply_date_offset($data->timeseen); $newitemid = $DB->insert_record('lesson_attempts', $data); + $this->set_mapping('lesson_attempt', $oldid, $newitemid, true); // Has related fileareas. } protected function process_lesson_grade($data) { @@ -209,6 +210,7 @@ class restore_lesson_activity_structure_step extends restore_activity_structure_ $this->add_related_files('mod_lesson', 'page_contents', 'lesson_page'); $this->add_related_files('mod_lesson', 'page_answers', 'lesson_answer'); $this->add_related_files('mod_lesson', 'page_responses', 'lesson_answer'); + $this->add_related_files('mod_lesson', 'essay_responses', 'lesson_attempt'); // Remap all the restored prevpageid and nextpageid now that we have all the pages and their mappings $rs = $DB->get_recordset('lesson_pages', array('lessonid' => $this->task->get_activityid()), diff --git a/mod/lesson/essay.php b/mod/lesson/essay.php index 2625fc054ff..2610b4a8c77 100644 --- a/mod/lesson/essay.php +++ b/mod/lesson/essay.php @@ -25,6 +25,7 @@ require_once('../../config.php'); require_once($CFG->dirroot.'/mod/lesson/locallib.php'); +require_once($CFG->dirroot.'/mod/lesson/pagetypes/essay.php'); require_once($CFG->dirroot.'/mod/lesson/essay_form.php'); require_once($CFG->libdir.'/eventslib.php'); @@ -99,7 +100,13 @@ switch ($mode) { print_error('cannotfinduser', 'lesson'); } - $mform = new essay_grading_form(null, array('scoreoptions'=>$scoreoptions, 'user'=>$user)); + $editoroptions = array('noclean' => true, 'maxfiles' => EDITOR_UNLIMITED_FILES, + 'maxbytes' => $CFG->maxbytes, 'context' => $context); + $essayinfo = lesson_page_type_essay::extract_useranswer($attempt->useranswer); + $essayinfo = file_prepare_standard_editor($essayinfo, 'response', $editoroptions, $context, + 'mod_lesson', 'essay_responses', $attempt->id); + $mform = new essay_grading_form(null, array('scoreoptions' => $scoreoptions, 'user' => $user)); + $mform->set_data($essayinfo); if ($mform->is_cancelled()) { redirect("$CFG->wwwroot/mod/lesson/essay.php?id=$cm->id"); } @@ -108,12 +115,12 @@ switch ($mode) { print_error('cannotfindgrade', 'lesson'); } - $essayinfo = new stdClass; - $essayinfo = unserialize($attempt->useranswer); - $essayinfo->graded = 1; $essayinfo->score = $form->score; - $essayinfo->response = clean_param($form->response, PARAM_RAW); + $form = file_postupdate_standard_editor($form, 'response', $editoroptions, $context, + 'mod_lesson', 'essay_responses', $attempt->id); + $essayinfo->response = $form->response; + $essayinfo->responseformat = $form->responseformat; $essayinfo->sent = 0; if (!$lesson->custom && $essayinfo->score == 1) { $attempt->correct = 1; @@ -208,7 +215,7 @@ switch ($mode) { } foreach ($attempts as $attempt) { - $essayinfo = unserialize($attempt->useranswer); + $essayinfo = lesson_page_type_essay::extract_useranswer($attempt->useranswer); if ($essayinfo->graded && !$essayinfo->sent) { // Holds values for the essayemailsubject string for the email message $a = new stdClass; @@ -232,7 +239,10 @@ switch ($mode) { $a->question = format_text($currentpage->contents, $currentpage->contentsformat, $formattextdefoptions); $a->response = format_text($essayinfo->answer, $essayinfo->answerformat, array('context' => $context, 'para' => true)); - $a->comment = s($essayinfo->response); + $a->comment = $essayinfo->response; + $a->comment = file_rewrite_pluginfile_urls($a->comment, 'pluginfile.php', $context->id, + 'mod_lesson', 'essay_responses', $attempt->id); + $a->comment = format_text($a->comment, $essayinfo->responseformat, $formattextdefoptions); // Fetch message HTML and plain text formats $message = get_string('essayemailmessage2', 'lesson', $a); @@ -353,7 +363,7 @@ switch ($mode) { } // Start processing the attempt - $essayinfo = unserialize($essay->useranswer); + $essayinfo = lesson_page_type_essay::extract_useranswer($essay->useranswer); // link for each essay $url = new moodle_url('/mod/lesson/essay.php', array('id'=>$cm->id,'mode'=>'grade','attemptid'=>$essay->id,'sesskey'=>sesskey())); @@ -397,7 +407,7 @@ switch ($mode) { // Grading form // Expects the following to be set: $attemptid, $answer, $user, $page, $attempt - $essayinfo = unserialize($attempt->useranswer); + $essayinfo = lesson_page_type_essay::extract_useranswer($attempt->useranswer); $currentpage = $lesson->load_page($attempt->pageid); $mform = new essay_grading_form(null, array('scoreoptions'=>$scoreoptions, 'user'=>$user)); @@ -409,6 +419,11 @@ switch ($mode) { $data->studentanswer = format_text($essayinfo->answer, $essayinfo->answerformat, array('context' => $context, 'para' => true)); $data->response = $essayinfo->response; + $data->responseformat = $essayinfo->responseformat; + $editoroptions = array('noclean' => true, 'maxfiles' => EDITOR_UNLIMITED_FILES, + 'maxbytes' => $CFG->maxbytes, 'context' => $context); + $data = file_prepare_standard_editor($data, 'response', $editoroptions, $context, + 'mod_lesson', 'essay_responses', $attempt->id); $mform->set_data($data); $mform->display(); diff --git a/mod/lesson/essay_form.php b/mod/lesson/essay_form.php index 883c4e66d42..a1ab40c5cff 100644 --- a/mod/lesson/essay_form.php +++ b/mod/lesson/essay_form.php @@ -39,6 +39,8 @@ require_once($CFG->libdir.'/formslib.php'); class essay_grading_form extends moodleform { public function definition() { + global $CFG; + $mform = $this->_form; $mform->addElement('header', 'formheader', get_string('question', 'lesson')); @@ -55,8 +57,9 @@ class essay_grading_form extends moodleform { $mform->addElement('static', 'question', get_string('question', 'lesson')); $mform->addElement('static', 'studentanswer', get_string('studentresponse', 'lesson', fullname($this->_customdata['user'], true))); - $mform->addElement('textarea', 'response', get_string('comments', 'lesson'), array('rows'=>'15', 'cols'=>'60')); - $mform->setType('response', PARAM_TEXT); + $editoroptions = array('noclean' => true, 'maxfiles' => EDITOR_UNLIMITED_FILES, 'maxbytes' => $CFG->maxbytes); + $mform->addElement('editor', 'response_editor', get_string('comments', 'lesson'), null, $editoroptions); + $mform->setType('response', PARAM_RAW); $mform->addElement('select', 'score', get_string('essayscore', 'lesson'), $this->_customdata['scoreoptions']); $mform->setType('score', PARAM_INT); diff --git a/mod/lesson/lang/en/lesson.php b/mod/lesson/lang/en/lesson.php index 32e448932a2..500d025cc65 100644 --- a/mod/lesson/lang/en/lesson.php +++ b/mod/lesson/lang/en/lesson.php @@ -171,6 +171,7 @@ $string['essay'] = 'Essay'; $string['essayemailmessage'] = '

Essay prompt:

{$a->question}

Your response:

{$a->response}

{$a->teacher}\'s comments:

{$a->comment}

You have received {$a->earned} out of {$a->outof} for this essay question.

Your grade for the lesson has been changed to {$a->newgrade}%.

'; $string['essayemailmessage2'] = '

Essay prompt:

{$a->question}

Your response:

{$a->response}

Grader\'s comments:

{$a->comment}

You have received {$a->earned} out of {$a->outof} for this essay question.

Your grade for the lesson has been changed to {$a->newgrade}%.

'; $string['essayemailsubject'] = 'Your grade for {$a} question'; +$string['essayresponses'] = 'Essay responses'; $string['essays'] = 'Essays'; $string['essayscore'] = 'Essay score'; $string['eventessayassessed'] = 'Essay assessed'; diff --git a/mod/lesson/lib.php b/mod/lesson/lib.php index 088967d407e..138d41c0ed3 100644 --- a/mod/lesson/lib.php +++ b/mod/lesson/lib.php @@ -682,6 +682,20 @@ function lesson_reset_userdata($data) { WHERE l.course=:course"; $params = array ("course" => $data->courseid); + $lessons = $DB->get_records_sql($lessonssql, $params); + + // Get rid of attempts files. + $fs = get_file_storage(); + if ($lessons) { + foreach ($lessons as $lessonid => $unused) { + if (!$cm = get_coursemodule_from_instance('lesson', $lessonid)) { + continue; + } + $context = context_module::instance($cm->id); + $fs->delete_area_files($context->id, 'mod_lesson', 'essay_responses'); + } + } + $DB->delete_records_select('lesson_timer', "lessonid IN ($lessonssql)", $params); $DB->delete_records_select('lesson_high_scores', "lessonid IN ($lessonssql)", $params); $DB->delete_records_select('lesson_grades', "lessonid IN ($lessonssql)", $params); @@ -897,6 +911,13 @@ function lesson_pluginfile($course, $cm, $context, $filearea, $args, $forcedownl } $fullpath = "/$context->id/mod_lesson/$filearea/$itemid/".implode('/', $args); + } else if ($filearea === 'essay_responses') { + $itemid = (int)array_shift($args); + if (!$attempt = $DB->get_record('lesson_attempts', array('id' => $itemid))) { + return false; + } + $fullpath = "/$context->id/mod_lesson/$filearea/$itemid/".implode('/', $args); + } else if ($filearea === 'mediafile') { if (count($args) > 1) { // Remove the itemid when it appears to be part of the arguments. If there is only one argument @@ -931,6 +952,7 @@ function lesson_get_file_areas() { $areas['mediafile'] = get_string('mediafile', 'mod_lesson'); $areas['page_answers'] = get_string('pageanswers', 'mod_lesson'); $areas['page_responses'] = get_string('pageresponses', 'mod_lesson'); + $areas['essay_responses'] = get_string('essayresponses', 'mod_lesson'); return $areas; } diff --git a/mod/lesson/locallib.php b/mod/lesson/locallib.php index 1a3ba153b84..9cf7cc22815 100644 --- a/mod/lesson/locallib.php +++ b/mod/lesson/locallib.php @@ -1870,7 +1870,19 @@ abstract class lesson_page extends lesson_base { */ final public function delete() { global $DB; - // first delete all the associated records... + + $cm = get_coursemodule_from_instance('lesson', $this->lesson->id, $this->lesson->course); + $context = context_module::instance($cm->id); + + // Delete files associated with attempts. + $fs = get_file_storage(); + if ($attempts = $DB->get_records('lesson_attempts', array("pageid" => $this->properties->id))) { + foreach ($attempts as $attempt) { + $fs->delete_area_files($context->id, 'mod_lesson', 'essay_responses', $attempt->id); + } + } + + // Then delete all the associated records... $DB->delete_records("lesson_attempts", array("pageid" => $this->properties->id)); // ...now delete the answers... $DB->delete_records("lesson_answers", array("pageid" => $this->properties->id)); @@ -1878,9 +1890,6 @@ abstract class lesson_page extends lesson_base { $DB->delete_records("lesson_pages", array("id" => $this->properties->id)); // Delete files associated with this page. - $cm = get_coursemodule_from_instance('lesson', $this->lesson->id, $this->lesson->course); - $context = context_module::instance($cm->id); - $fs = get_file_storage(); $fs->delete_area_files($context->id, 'mod_lesson', 'page_contents', $this->properties->id); $fs->delete_area_files($context->id, 'mod_lesson', 'page_answers', $this->properties->id); $fs->delete_area_files($context->id, 'mod_lesson', 'page_responses', $this->properties->id); @@ -2088,8 +2097,7 @@ abstract class lesson_page extends lesson_base { $options->para = true; $options->overflowdiv = true; $options->context = $context; - $result->response = file_rewrite_pluginfile_urls($result->response, 'pluginfile.php', $context->id, - 'mod_lesson', 'page_responses', $result->answerid); + $result->feedback = $OUTPUT->box(format_text($this->get_contents(), $this->properties->contentsformat, $options), 'generalbox boxaligncenter'); if (isset($result->studentanswerformat)) { // This is the student's answer so it should be cleaned. @@ -2100,7 +2108,14 @@ abstract class lesson_page extends lesson_base { } $result->feedback .= '
' . get_string("youranswer", "lesson").' : ' . $studentanswer; - $result->feedback .= $OUTPUT->box($result->response, $class); // Already converted to HTML. + if (isset($result->responseformat)) { + $result->response = file_rewrite_pluginfile_urls($result->response, 'pluginfile.php', $context->id, + 'mod_lesson', 'page_responses', $result->answerid); + $result->feedback .= $OUTPUT->box(format_text($result->response, $result->responseformat, $options) + , $class); + } else { + $result->feedback .= $OUTPUT->box($result->response, $class); + } $result->feedback .= '
'; } } diff --git a/mod/lesson/pagetypes/essay.php b/mod/lesson/pagetypes/essay.php index cceef9ad8b4..be2edfa73ba 100644 --- a/mod/lesson/pagetypes/essay.php +++ b/mod/lesson/pagetypes/essay.php @@ -47,6 +47,23 @@ class lesson_page_type_essay extends lesson_page { public function get_idstring() { return $this->typeidstring; } + + /** + * Unserialize attempt useranswer and add missing responseformat if needed + * for compatibility with old records. + * + * @param string $useranswer serialized object + * @return object + */ + static public function extract_useranswer($useranswer) { + $essayinfo = unserialize($useranswer); + if (!isset($essayinfo->responseformat)) { + $essayinfo->response = text_to_html($essayinfo->response, false, false); + $essayinfo->responseformat = FORMAT_HTML; + } + return $essayinfo; + } + public function display($renderer, $attempt) { global $PAGE, $CFG, $USER; @@ -56,7 +73,7 @@ class lesson_page_type_essay extends lesson_page { $data->id = $PAGE->cm->id; $data->pageid = $this->properties->id; if (isset($USER->modattempts[$this->lesson->id])) { - $essayinfo = unserialize($attempt->useranswer); + $essayinfo = self::extract_useranswer($attempt->useranswer); $data->answer = $essayinfo->answer; } $mform->set_data($data); @@ -99,7 +116,7 @@ class lesson_page_type_essay extends lesson_page { $studentanswerformat = $data->answer['format']; } else { $studentanswer = $data->answer; - $studentanswerformat = FORMAT_MOODLE; + $studentanswerformat = FORMAT_HTML; } if (trim($studentanswer) === '') { @@ -119,7 +136,8 @@ class lesson_page_type_essay extends lesson_page { $userresponse->score = 0; $userresponse->answer = $studentanswer; $userresponse->answerformat = $studentanswerformat; - $userresponse->response = ""; + $userresponse->response = ''; + $userresponse->responseformat = FORMAT_HTML; $result->userresponse = serialize($userresponse); $result->studentanswerformat = $studentanswerformat; $result->studentanswer = $studentanswer; @@ -161,7 +179,7 @@ class lesson_page_type_essay extends lesson_page { // else, user attempted the question less than the max, so grab the last one $temp = end($tries); } - $essayinfo = unserialize($temp->useranswer); + $essayinfo = self::extract_useranswer($temp->useranswer); if ($essayinfo->graded) { if (isset($pagestats[$temp->pageid])) { $essaystats = $pagestats[$temp->pageid]; @@ -178,15 +196,21 @@ class lesson_page_type_essay extends lesson_page { return true; } public function report_answers($answerpage, $answerdata, $useranswer, $pagestats, &$i, &$n) { + $formattextdefoptions = new stdClass(); + $formattextdefoptions->noclean = true; + $formattextdefoptions->para = false; + $formattextdefoptions->context = $answerpage->context; $answers = $this->get_answers(); foreach ($answers as $answer) { if ($useranswer != null) { - $essayinfo = unserialize($useranswer->useranswer); + $essayinfo = self::extract_useranswer($useranswer->useranswer); if ($essayinfo->response == null) { $answerdata->response = get_string("nocommentyet", "lesson"); } else { - $answerdata->response = s($essayinfo->response); + $essayinfo->response = file_rewrite_pluginfile_urls($essayinfo->response, 'pluginfile.php', + $answerpage->context->id, 'mod_lesson', 'essay_responses', $useranswer->id); + $answerdata->response = format_text($essayinfo->response, $essayinfo->responseformat, $formattextdefoptions); } if (isset($pagestats[$this->properties->id])) { $percent = $pagestats[$this->properties->id]->totalscore / $pagestats[$this->properties->id]->total * 100; @@ -239,7 +263,7 @@ class lesson_page_type_essay extends lesson_page { return true; } public function get_earnedscore($answers, $attempt) { - $essayinfo = unserialize($attempt->useranswer); + $essayinfo = self::extract_useranswer($attempt->useranswer); return $essayinfo->score; } } @@ -273,7 +297,7 @@ class lesson_display_answer_form_essay extends moodleform { if (isset($USER->modattempts[$lessonid]->useranswer) && !empty($USER->modattempts[$lessonid]->useranswer)) { $attrs = array('disabled' => 'disabled'); $hasattempt = true; - $useranswertemp = unserialize($USER->modattempts[$lessonid]->useranswer); + $useranswertemp = lesson_page_type_essay::extract_useranswer($USER->modattempts[$lessonid]->useranswer); $useranswer = htmlspecialchars_decode($useranswertemp->answer, ENT_QUOTES); $useranswerraw = $useranswertemp->answer; } diff --git a/mod/lesson/report.php b/mod/lesson/report.php index 7312588f5c3..fcc63087a35 100644 --- a/mod/lesson/report.php +++ b/mod/lesson/report.php @@ -252,7 +252,7 @@ if ($action === 'delete') { $bestgradefound = false; // $tries holds all the tries/retries a student has done $tries = $studentdata[$student->id]; - $studentname = "{$student->lastname}, $student->firstname"; + $studentname = fullname($student, true); foreach ($tries as $try) { // start to build up the checkbox and link if (has_capability('mod/lesson:edit', $context)) { @@ -261,7 +261,8 @@ if ($action === 'delete') { $temp = ''; } - $temp .= "id&action=reportdetail&userid=".$try['userid'].'&try='.$try['try'].'">'; + $temp .= "id&action=reportdetail&userid=".$try['userid'] + .'&try='.$try['try'].'" class="lesson-attempt-link">'; if ($try["grade"] !== null) { // if null then not done yet // this is what the link does when the user has completed the try $timetotake = $try["timeend"] - $try["timestart"]; diff --git a/mod/lesson/tests/behat/lesson_essay_question.feature b/mod/lesson/tests/behat/lesson_essay_question.feature index 56fde01735d..5cce827871a 100644 --- a/mod/lesson/tests/behat/lesson_essay_question.feature +++ b/mod/lesson/tests/behat/lesson_essay_question.feature @@ -73,7 +73,15 @@ Feature: In a lesson activity, teacher can add an essay question And I should see "Student 1's response" And I should see "Once upon a time there was a little green frog." And I set the following fields to these values: - | Your comments | Well done. | + | Your comments |

Well done.

| | Essay score | 1 | And I press "Save changes" And I should see "Changes saved" + And I follow "Reports" + And I should see "Student 1" + And I click on ".lesson-attempt-link" "css_element" in the "Student 1" "table_row" + And I should see "Essay: Essay question" + And I should see "Please write a story about a frog." + And I should see "Once upon a time there was a little green frog." + And I should see "Well done." + And I should not see "<b>" diff --git a/mod/lesson/tests/pagetypes_test.php b/mod/lesson/tests/pagetypes_test.php new file mode 100644 index 00000000000..28eab4cb70e --- /dev/null +++ b/mod/lesson/tests/pagetypes_test.php @@ -0,0 +1,70 @@ +. + +/** + * Unit tests for page types classes + * + * @package mod_lesson + * @category test + * @copyright 2015 Jean-Michel Vedrine + * @license http://www.gnu.org/copyleft/gpl.html GNU Public License + */ + + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/mod/lesson/locallib.php'); +require_once($CFG->dirroot . '/mod/lesson/pagetypes/essay.php'); + + +/** + * This class contains the test cases for some of the functions in the lesson essay page type class. + * + * @copyright 2015 Jean-Michel Vedrine + * @license http://www.gnu.org/copyleft/gpl.html GNU Public License + */ +class mod_lesson_essay_page_type_test extends advanced_testcase { + public function test_lesson_essay_extract_useranswer() { + // Test that reponseformat is added when not present. + $answer = 'O:8:"stdClass":6:{s:4:"sent";i:1;s:6:"graded";i:1;s:5:"score";s:1:"1";' + . 's:6:"answer";s:64:"

This is my answer with bold and italics

";' + . 's:12:"answerformat";s:1:"1";s:8:"response";s:10:"Well done!";}'; + $userresponse = new stdClass; + $userresponse->sent = 1; + $userresponse->graded = 1; + $userresponse->score = 1; + $userresponse->answer = "

This is my answer with bold and italics

"; + $userresponse->answerformat = FORMAT_HTML; + $userresponse->response = "Well done!"; + $userresponse->responseformat = FORMAT_HTML; + $this->assertEquals($userresponse, lesson_page_type_essay::extract_useranswer($answer)); + + // Test that reponseformat is not modified when present. + $answer = 'O:8:"stdClass":7:{s:4:"sent";i:0;s:6:"graded";i:1;s:5:"score";s:1:"0";' + . 's:6:"answer";s:64:"

This is my answer with bold and italics

";' + . 's:12:"answerformat";s:1:"1";s:8:"response";s:10:"Well done!";s:14:"responseformat";s:1:"2";}'; + $userresponse = new stdClass; + $userresponse->sent = 0; + $userresponse->graded = 1; + $userresponse->score = 0; + $userresponse->answer = "

This is my answer with bold and italics

"; + $userresponse->answerformat = FORMAT_HTML; + $userresponse->response = "Well done!"; + $userresponse->responseformat = FORMAT_PLAIN; + $this->assertEquals($userresponse, lesson_page_type_essay::extract_useranswer($answer)); + } +}