From 29864f6c14cd308ea26e8695a18bde64172e762e Mon Sep 17 00:00:00 2001 From: Gordon Bateson Date: Thu, 6 Apr 2017 15:29:22 +0900 Subject: [PATCH] MDL-79863 qtype_ordering: qtype_ordering fix display of images in feedback and explanatino of correct response --- question/type/ordering/edit_ordering_form.php | 4 +- question/type/ordering/question.php | 43 +++++- question/type/ordering/renderer.php | 128 ++++++++++-------- question/type/ordering/version.php | 4 +- 4 files changed, 109 insertions(+), 70 deletions(-) diff --git a/question/type/ordering/edit_ordering_form.php b/question/type/ordering/edit_ordering_form.php index 27aa18ba761..ad1d2e5a080 100644 --- a/question/type/ordering/edit_ordering_form.php +++ b/question/type/ordering/edit_ordering_form.php @@ -141,10 +141,10 @@ class qtype_ordering_edit_form extends question_edit_form { // Adjust HTML editor and removal buttons. $this->adjust_html_editors($mform, $name, $repeats); - // Adding feedback fields. + // Adding feedback fields (=Combined feedback). $this->add_combined_feedback_fields(false); - // Adding interactive settings. + // Adding interactive settings (=Multiple tries). $this->add_interactive_settings(false, false); } diff --git a/question/type/ordering/question.php b/question/type/ordering/question.php index 05efe56833d..1c710d6bfc1 100644 --- a/question/type/ordering/question.php +++ b/question/type/ordering/question.php @@ -371,7 +371,7 @@ class qtype_ordering_question extends question_graded_automatically { } /** - * Checks whether the users is allow to be served a particular file. + * Checks whether the user has permission to access a particular file. * * @param question_attempt $qa the question attempt being displayed. * @param question_display_options $options the options that control display of the question. @@ -388,7 +388,7 @@ class qtype_ordering_question extends question_graded_automatically { return array_key_exists($answerid, $this->answers); } if (in_array($filearea, $this->qtype->feedbackfields)) { - return $this->check_combined_feedback_file_access($qa, $options, $filearea); + return $this->check_combined_feedback_file_access($qa, $options, $filearea, $args); } if ($filearea == 'hint') { return $this->check_hint_file_access($qa, $options, $args); @@ -397,11 +397,42 @@ class qtype_ordering_question extends question_graded_automatically { return parent::check_file_access($qa, $options, $component, $filearea, $args, $forcedownload); } - /* - * ------------------ - * Custom methods - * ------------------ + /////////////////////////////////////////////////////// + // methods from "question_graded_automatically" class + // see "question/type/questionbase.php" + /////////////////////////////////////////////////////// + + /** + * Check a request for access to a file belonging to a combined feedback field. + * + * Fix a bug in Moodle 2.9 & 3.0, in which this method does not declare $args, + * so trying to use $args[0] always fails and images in feedback are not shown. + * + * @param question_attempt $qa the question attempt being displayed. + * @param question_display_options $options the options that control display of the question. + * @param string $filearea the name of the file area. + * @param array $args the remaining bits of the file path. + * @return bool whether access to the file should be allowed. */ + protected function check_combined_feedback_file_access($qa, $options, $filearea, $args = null) { + $state = $qa->get_state(); + if (! $state->is_finished()) { + $response = $qa->get_last_qt_data(); + if (! $this->is_gradable_response($response)) { + return false; + } + list($fraction, $state) = $this->grade_response($response); + } + if ($state->get_feedback_class().'feedback' == $filearea) { + return ($this->id == reset($args)); + } else { + return false; + } + } + + /////////////////////////////////////////////////////// + // Custom methods + /////////////////////////////////////////////////////// /** * Returns response mform field name diff --git a/question/type/ordering/renderer.php b/question/type/ordering/renderer.php index 648d77a8f82..c3097968fd6 100644 --- a/question/type/ordering/renderer.php +++ b/question/type/ordering/renderer.php @@ -163,13 +163,14 @@ class qtype_ordering_renderer extends qtype_with_combined_feedback_renderer { // Format the answer text. $answer = $question->answers[$answerid]; - $answer->answer = $question->format_text($answer->answer, $answer->answerformat, - $qa, 'question', 'answer', $answerid); + $answertext = $question->format_text($answer->answer, $answer->answerformat, + $qa, 'question', 'answer', $answerid); + // The original "id" revealed the correct order of the answers // because $answer->fraction holds the correct order number. // Therefore we use the $answer's md5key for the "id". $params = array('class' => $class, 'id' => $answer->md5key); - $result .= html_writer::tag('li', $img.$answer->answer, $params); + $result .= html_writer::tag('li', $img.$answertext, $params); } } @@ -204,71 +205,76 @@ class qtype_ordering_renderer extends qtype_with_combined_feedback_renderer { $gradedetails = ''; $scoredetails = ''; - // If required, add explanation of grade calculation. + // Decide if we should show grade explanation for "partial" or "wrong" states. + // This should detect "^graded(partial|wrong)$" and possibly others. if ($step = $qa->get_last_step()) { - $state = $step->get_state(); - if ($state == 'gradedpartial' || $state == 'gradedwrong') { + $show = preg_match('/(partial|wrong)$/', $step->get_state()); + } else { + $show = false; + } - $plugin = 'qtype_ordering'; - $question = $qa->get_question(); + // If required, add explanation of grade calculation. + if ($show) { - // show grading details if they are required - if ($question->options->showgrading) { + $plugin = 'qtype_ordering'; + $question = $qa->get_question(); - // Fetch grading type. - $gradingtype = $question->options->gradingtype; - $gradingtype = qtype_ordering_question::get_grading_types($gradingtype); + // show grading details if they are required + if ($question->options->showgrading) { - // Format grading type, e.g. Grading type: Relative to next item, excluding last item. - if ($gradingtype) { - $gradingtype = get_string('gradingtype', $plugin).': '.$gradingtype; - $gradingtype = html_writer::tag('p', $gradingtype, array('class' => 'gradingtype')); + // Fetch grading type. + $gradingtype = $question->options->gradingtype; + $gradingtype = qtype_ordering_question::get_grading_types($gradingtype); + + // Format grading type, e.g. Grading type: Relative to next item, excluding last item. + if ($gradingtype) { + $gradingtype = get_string('gradingtype', $plugin).': '.$gradingtype; + $gradingtype = html_writer::tag('p', $gradingtype, array('class' => 'gradingtype')); + } + + // Fetch grade details and score details. + if ($currentresponse = $question->currentresponse) { + + $totalscore = 0; + $totalmaxscore = 0; + + $layoutclass = $question->get_ordering_layoutclass(); + $params = array('class' => $layoutclass); + + $scoredetails .= html_writer::tag('p', get_string('scoredetails', $plugin)); + $scoredetails .= html_writer::start_tag('ol', array('class' => 'scoredetails')); + + // Format scoredetails, e.g. 1 /2 = 50%, for each item. + foreach ($currentresponse as $position => $answerid) { + if (array_key_exists($answerid, $question->answers)) { + $answer = $question->answers[$answerid]; + $score = $this->get_ordering_item_score($question, $position, $answerid); + list($score, $maxscore, $fraction, $percent, $class, $img) = $score; + if ($maxscore === null) { + $score = get_string('noscore', $plugin); + } else { + $totalscore += $score; + $totalmaxscore += $maxscore; + $score = "$score / $maxscore = $percent%"; + } + $scoredetails .= html_writer::tag('li', $score, $params); + } } - // Fetch grade details and score details. - if ($currentresponse = $question->currentresponse) { + $scoredetails .= html_writer::end_tag('ol'); - $totalscore = 0; - $totalmaxscore = 0; - - $layoutclass = $question->get_ordering_layoutclass(); - $params = array('class' => $layoutclass); - - $scoredetails .= html_writer::tag('p', get_string('scoredetails', $plugin)); - $scoredetails .= html_writer::start_tag('ol', array('class' => 'scoredetails')); - - // Format scoredetails, e.g. 1 /2 = 50%, for each item. - foreach ($currentresponse as $position => $answerid) { - if (array_key_exists($answerid, $question->answers)) { - $answer = $question->answers[$answerid]; - $score = $this->get_ordering_item_score($question, $position, $answerid); - list($score, $maxscore, $fraction, $percent, $class, $img) = $score; - if ($maxscore === null) { - $score = get_string('noscore', $plugin); - } else { - $totalscore += $score; - $totalmaxscore += $maxscore; - $score = "$score / $maxscore = $percent%"; - } - $scoredetails .= html_writer::tag('li', $score, $params); - } - } - - $scoredetails .= html_writer::end_tag('ol'); - - if ($totalmaxscore == 0) { - $scoredetails = ''; // ALL_OR_NOTHING. + if ($totalmaxscore == 0) { + $scoredetails = ''; // ALL_OR_NOTHING. + } else { + // Format gradedetails, e.g. 4 /6 = 67%. + if ($totalscore == 0) { + $gradedetails = 0; } else { - // Format gradedetails, e.g. 4 /6 = 67%. - if ($totalscore == 0) { - $gradedetails = 0; - } else { - $gradedetails = round(100 * $totalscore / $totalmaxscore, 0); - } - $gradedetails = "$totalscore / $totalmaxscore = $gradedetails%"; - $gradedetails = get_string('gradedetails', $plugin).': '.$gradedetails; - $gradedetails = html_writer::tag('p', $gradedetails, array('class' => 'gradedetails')); + $gradedetails = round(100 * $totalscore / $totalmaxscore, 0); } + $gradedetails = "$totalscore / $totalmaxscore = $gradedetails%"; + $gradedetails = get_string('gradedetails', $plugin).': '.$gradedetails; + $gradedetails = html_writer::tag('p', $gradedetails, array('class' => 'gradedetails')); } } } @@ -278,7 +284,7 @@ class qtype_ordering_renderer extends qtype_with_combined_feedback_renderer { } /** - * Gereate an automatic description of the correct response to this question. + * Generate an automatic description of the correct response to this question. * Not all question types can do this. If it is not possible, this method * should just return an empty string. * @@ -316,7 +322,9 @@ class qtype_ordering_renderer extends qtype_with_combined_feedback_renderer { $correctresponse = $question->correctresponse; foreach ($correctresponse as $position => $answerid) { $answer = $question->answers[$answerid]; - $output .= html_writer::tag('li', $answer->answer, array('class' => $layoutclass)); + $answertext = $question->format_text($answer->answer, $answer->answerformat, + $qa, 'question', 'answer', $answerid); + $output .= html_writer::tag('li', $answertext, array('class' => $layoutclass)); } $output .= html_writer::end_tag('ol'); } diff --git a/question/type/ordering/version.php b/question/type/ordering/version.php index 966b02d0ea9..c66be5c2b09 100644 --- a/question/type/ordering/version.php +++ b/question/type/ordering/version.php @@ -29,5 +29,5 @@ $plugin->cron = 0; $plugin->component = 'qtype_ordering'; $plugin->maturity = MATURITY_STABLE; $plugin->requires = 2011070100; // Moodle 2.1. -$plugin->version = 2016102657; -$plugin->release = '2016-10-26 (57)'; +$plugin->version = 2017040658; +$plugin->release = '2017-04-26 (58)';