From 6f1b265a2b6fc8a6ca856d67864dd9f5ee844120 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Thu, 21 May 2015 18:09:27 +0100 Subject: [PATCH] MDL-49368 qtypes match & multichice: cope with editing after attempt Teachers should not radically edit a question after it has been attempted. However, if they do, we should handle it gracefully, rather than triggering PHP errors. --- question/type/match/lang/en/qtype_match.php | 2 ++ question/type/match/question.php | 23 +++++++++++++++++-- question/type/match/renderer.php | 3 +++ .../multichoice/lang/en/qtype_multichoice.php | 1 + question/type/multichoice/question.php | 18 +++++++++++++++ question/type/multichoice/questiontype.php | 5 ++++ 6 files changed, 50 insertions(+), 2 deletions(-) diff --git a/question/type/match/lang/en/qtype_match.php b/question/type/match/lang/en/qtype_match.php index 3552ac1d09dcb..cfdc5ada4551c 100644 --- a/question/type/match/lang/en/qtype_match.php +++ b/question/type/match/lang/en/qtype_match.php @@ -26,6 +26,8 @@ $string['availablechoices'] = 'Available choices'; $string['blanksforxmorequestions'] = 'Blanks for {no} more questions'; $string['correctansweris'] = 'The correct answer is: {$a}'; +$string['deletedchoice'] = '[Deleted choice]'; +$string['deletedsubquestion'] = 'This part of the question was deleted after the attempt was started.'; $string['filloutthreeqsandtwoas'] = 'You must provide at least two questions and three answers. You can provide extra wrong answers by giving an answer with a blank question. Entries where both the question and the answer are blank will be ignored.'; $string['nomatchinganswer'] = 'You must specify an answer matching the question \'{$a}\'.'; $string['nomatchinganswerforq'] = 'You must specify an answer for this question.'; diff --git a/question/type/match/question.php b/question/type/match/question.php index 5ed2b5fd00200..671047405c816 100644 --- a/question/type/match/question.php +++ b/question/type/match/question.php @@ -71,6 +71,25 @@ public function start_attempt(question_attempt_step $step, $variant) { public function apply_attempt_state(question_attempt_step $step) { $this->stemorder = explode(',', $step->get_qt_var('_stemorder')); $this->set_choiceorder(explode(',', $step->get_qt_var('_choiceorder'))); + + // Add any missing subquestions. Sometimes people edit questions after they + // have been attempted which breaks things. + foreach ($this->stemorder as $stemid) { + if (!isset($this->stems[$stemid])) { + $this->stems[$stemid] = html_writer::span( + get_string('deletedsubquestion', 'qtype_match'), 'notifyproblem'); + $this->stemformat[$stemid] = FORMAT_HTML; + $this->right[$stemid] = 0; + } + } + + // Add any missing choices. Sometimes people edit questions after they + // have been attempted which breaks things. + foreach ($this->choiceorder as $choiceid) { + if (!isset($this->choices[$choiceid])) { + $this->choices[$choiceid] = get_string('deletedchoice', 'qtype_match'); + } + } } /** @@ -80,8 +99,8 @@ public function apply_attempt_state(question_attempt_step $step) { */ protected function set_choiceorder($choiceorder) { $this->choiceorder = array(); - foreach ($choiceorder as $key => $value) { - $this->choiceorder[$key + 1] = $value; + foreach ($choiceorder as $key => $choiceid) { + $this->choiceorder[$key + 1] = $choiceid; } } diff --git a/question/type/match/renderer.php b/question/type/match/renderer.php index 3ee066c743687..732efa82e731d 100644 --- a/question/type/match/renderer.php +++ b/question/type/match/renderer.php @@ -137,6 +137,9 @@ public function correct_response(question_attempt $qa) { $choices = $this->format_choices($question); $right = array(); foreach ($stemorder as $key => $stemid) { + if (!isset($choices[$question->get_right_choice_for($stemid)])) { + continue; + } $right[] = $question->make_html_inline($this->format_stem_text($qa, $stemid)) . ' – ' . $choices[$question->get_right_choice_for($stemid)]; } diff --git a/question/type/multichoice/lang/en/qtype_multichoice.php b/question/type/multichoice/lang/en/qtype_multichoice.php index b55d07bde6a52..63b6b6713a0a6 100644 --- a/question/type/multichoice/lang/en/qtype_multichoice.php +++ b/question/type/multichoice/lang/en/qtype_multichoice.php @@ -38,6 +38,7 @@ $string['clozeaid'] = 'Enter missing word'; $string['correctansweris'] = 'The correct answer is: {$a}'; $string['correctfeedback'] = 'For any correct response'; +$string['deletedchoice'] = 'This choice was deleted after the attempt was started.'; $string['errgradesetanswerblank'] = 'Grade set, but the Answer is blank'; $string['errfractionsaddwrong'] = 'The positive grades you have chosen do not add up to 100%
Instead, they add up to {$a}%'; $string['errfractionsnomax'] = 'One of the choices should be 100%, so that it is
possible to get a full grade for this question.'; diff --git a/question/type/multichoice/question.php b/question/type/multichoice/question.php index e04656d5b4ae1..a1b12a6035d20 100644 --- a/question/type/multichoice/question.php +++ b/question/type/multichoice/question.php @@ -64,6 +64,24 @@ public function start_attempt(question_attempt_step $step, $variant) { public function apply_attempt_state(question_attempt_step $step) { $this->order = explode(',', $step->get_qt_var('_order')); + + // Add any missing answers. Sometimes people edit questions after they + // have been attempted which breaks things. + foreach ($this->order as $ansid) { + if (isset($this->answers[$ansid])) { + continue; + } + $a = new stdClass(); + $a->id = 0; + $a->answer = html_writer::span(get_string('deletedchoice', 'qtype_multichoice'), + 'notifyproblem'); + $a->answerformat = FORMAT_HTML; + $a->fraction = 0; + $a->feedback = ''; + $a->feedbackformat = FORMAT_HTML; + $this->answers[$ansid] = $this->qtype->make_answer($a); + $this->answers[$ansid]->answerformat = FORMAT_HTML; + } } public function get_question_summary() { diff --git a/question/type/multichoice/questiontype.php b/question/type/multichoice/questiontype.php index d7cea13ce5fc2..c90db2cb7c163 100644 --- a/question/type/multichoice/questiontype.php +++ b/question/type/multichoice/questiontype.php @@ -174,6 +174,11 @@ protected function initialise_question_instance(question_definition $question, $ $this->initialise_question_answers($question, $questiondata, false); } + public function make_answer($answer) { + // Overridden just so we can make it public for use by question.php. + return parent::make_answer($answer); + } + public function delete_question($questionid, $contextid) { global $DB; $DB->delete_records('qtype_multichoice_options', array('questionid' => $questionid));