From 9cd0fe59a37f2dafe6c154b7916ccc60f3de4d2a Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Fri, 4 Jul 2025 12:04:48 +0100 Subject: [PATCH 1/2] MDL-85018 qtype_multianswer: Prevent errors on backup/restore If a multianswer question has its question_multianswer record missing for some reason, attempting to backup and restore a bank containing the question will result a dml_missing_record exception. The qtype_multianswer::get_question_options() method already copes with the sequence from this record being empty, so we can just remove the MUST_EXIST check and let it fall back to that. --- question/type/multianswer/questiontype.php | 3 +- .../type/multianswer/tests/restore_test.php | 97 +++++++++++++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 question/type/multianswer/tests/restore_test.php diff --git a/question/type/multianswer/questiontype.php b/question/type/multianswer/questiontype.php index 0c44b18a56f..9c662ab6fc0 100644 --- a/question/type/multianswer/questiontype.php +++ b/question/type/multianswer/questiontype.php @@ -110,8 +110,7 @@ class qtype_multianswer extends question_type { parent::get_question_options($question); // Get relevant data indexed by positionkey from the multianswers table. - $sequence = $DB->get_field('question_multianswer', 'sequence', - array('question' => $question->id), MUST_EXIST); + $sequence = $DB->get_field('question_multianswer', 'sequence', ['question' => $question->id]); if (empty($sequence)) { $question->options->questions = []; diff --git a/question/type/multianswer/tests/restore_test.php b/question/type/multianswer/tests/restore_test.php new file mode 100644 index 00000000000..543e8ab5a29 --- /dev/null +++ b/question/type/multianswer/tests/restore_test.php @@ -0,0 +1,97 @@ +. + +namespace qtype_multianswer; + +/** + * Unit tests for + * + * @package qtype_multianswer + * @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_multianswer_plugin + */ +final class restore_test extends \advanced_testcase { + /** + * Duplicate a quiz containing a multianswer question with no multianswer 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(); + $context = \context_course::instance($course1->id); + $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 multianswer question from the qbank. + $quiz = $this->getDataGenerator()->get_plugin_generator('mod_quiz')->create_instance(['course' => $course1->id]); + $question = $questiongenerator->create_question('multianswer', 'twosubq', ['category' => $cat->id]); + quiz_add_quiz_question($question->id, $quiz); + + // Delete the multianswer record. + $DB->delete_records('question_multianswer', ['question' => $question->id]); + + // Confirm we have created 3 additional questions (one parent, 2 children). + $this->assertEquals($initialcount + 3, $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(); + + // 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 + 3, $DB->count_records('question')); + } +} From a576b9db5d6ca7f2c37820e9a0c4e60f89297dc0 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Fri, 4 Jul 2025 14:52:25 +0100 Subject: [PATCH 2/2] MDL-85018 questionlib: Update broken question test A multianswer question with a missing database record no longer throws an exception, so doesn't work for the "broken question" test, so switch to ddwtos. --- lib/tests/questionlib_test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/tests/questionlib_test.php b/lib/tests/questionlib_test.php index 270c7bf75f0..aec79a6bb1c 100644 --- a/lib/tests/questionlib_test.php +++ b/lib/tests/questionlib_test.php @@ -1676,11 +1676,11 @@ final class questionlib_test extends \advanced_testcase { ]); // Create a cloze question. - $question = $questiongenerator->create_question('multianswer', null, [ + $question = $questiongenerator->create_question('ddwtos', null, [ 'category' => $questioncat->id, ]); // Now, break the question. - $DB->delete_records('question_multianswer', ['question' => $question->id]); + $DB->delete_records('question_ddwtos', ['questionid' => $question->id]); $this->setAdminUser();