From 0105cdb0de4ffedcefbc0adb2c0e423814346a80 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Thu, 12 Jun 2025 13:32:20 +0100 Subject: [PATCH] MDL-85721 qtype: Cope with missing options records If we restore a question (or any other) which has had its qtype_xxx_options record deleted, we get a notification output when we try to build the options. This may be called from an AJAX request (such as when we duplicate a quiz), and outputting the notification breaks the AJAX response. Returning false also means we don't get the answers attached to the questiondata options, so the structure doesn't match the restored data, and we get duplication. This emits the errors via debugging instead, which allows it to be supressed or logged, and allows get_question_options() to continue running. --- question/type/questiontypebase.php | 10 +-- .../type/shortanswer/tests/restore_test.php | 89 +++++++++++++++++++ 2 files changed, 93 insertions(+), 6 deletions(-) create mode 100644 question/type/shortanswer/tests/restore_test.php diff --git a/question/type/questiontypebase.php b/question/type/questiontypebase.php index d338d9c0f56..e1e72b77897 100644 --- a/question/type/questiontypebase.php +++ b/question/type/questiontypebase.php @@ -904,7 +904,7 @@ class question_type { * specific information (it is passed by reference). */ public function get_question_options($question) { - global $DB, $OUTPUT; + global $DB; if (!isset($question->options)) { $question->options = new stdClass(); @@ -921,9 +921,8 @@ class question_type { $question->options->$field = $extra_data->$field; } } else { - echo $OUTPUT->notification('Failed to load question options from the table ' . + debugging('Failed to load question options from the table ' . $question_extension_table . ' for questionid ' . $question->id); - return false; } } @@ -938,9 +937,8 @@ class question_type { WHERE qa.question = ? ORDER BY qa.id", array($question->id)); if (!$answers) { - echo $OUTPUT->notification('Failed to load question answers from the table ' . - $answerextensiontable . 'for questionid ' . $question->id); - return false; + debugging('Failed to load question answers from the table ' . + $answerextensiontable . ' for questionid ' . $question->id); } } else { // Don't check for success or failure because some question types do diff --git a/question/type/shortanswer/tests/restore_test.php b/question/type/shortanswer/tests/restore_test.php new file mode 100644 index 00000000000..185387d802e --- /dev/null +++ b/question/type/shortanswer/tests/restore_test.php @@ -0,0 +1,89 @@ +. + +namespace qtype_shortanswer; + +/** + * Unit tests for restore_qtype_shortanswer_plugin + * + * @package qtype_shortanswer + * @copyright 2025 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \restore_qtype_shortanswer_plugin + */ +final class restore_test extends \advanced_testcase { + /** + * Duplicate a quiz containing a shortanswer question with no options record. + */ + public function test_restore_quiz_with_edited_questions(): void { + global $CFG, $DB, $USER; + require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php'); + require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php'); + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $qbank = $generator->get_plugin_generator('mod_qbank')->create_instance(['course' => $course1->id]); + $context = \context_module::instance($qbank->cmid); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $initialcount = $DB->count_records('question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $context->id]); + + // Create a quiz containing a multichoice question from the qbank. + $quiz = $this->getDataGenerator()->get_plugin_generator('mod_quiz')->create_instance(['course' => $course1->id]); + $question = $questiongenerator->create_question('shortanswer', 'frogtoad', ['category' => $cat->id]); + quiz_add_quiz_question($question->id, $quiz); + + // Delete the multichoice_options record. + $DB->delete_records('qtype_shortanswer_options', ['questionid' => $question->id]); + + // Confirm we have created 1 additional question. + $this->assertEquals($initialcount + 1, $DB->count_records('question')); + + // Backup quiz. + $bc = new \backup_controller(\backup::TYPE_1ACTIVITY, $quiz->cmid, \backup::FORMAT_MOODLE, + \backup::INTERACTIVE_NO, \backup::MODE_IMPORT, $USER->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup into the same course. + $rc = new \restore_controller($backupid, $course1->id, \backup::INTERACTIVE_NO, \backup::MODE_IMPORT, + $USER->id, \backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + $debugging = "Failed to load question options from the table qtype_shortanswer_options for questionid {$question->id}"; + $this->assertdebuggingcalledcount(2, [$debugging, $debugging]); + + // Both quizzes should refer to the same original question. + $quizzes = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $this->assertCount(2, $quizzes); + foreach ($quizzes as $quiz) { + $structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz->instance, $quiz->context); + $this->assertEquals($structure[1]->questionid, $question->id); + } + + // There should be no additional questions created during the restore. + $this->assertEquals($initialcount + 1, $DB->count_records('question')); + } +}