From de49076931233ab285919ad47d521ec48aae2741 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Fri, 7 Nov 2025 14:01:50 +0000 Subject: [PATCH 1/2] MDL-86136 qbank: Restore entire activity when it is in the backup When restoring a backup containing a question bank, we were always trying to match questions to an existing category on the target course if once existed. This meant that when duplicating a qbank on the same course, or importing the same qbank to another course multiple times, only one of the restored qbanks would end up with questions in it. This adds a mechanism to record any activities in the backup which support `FEATURE_PUBLISHES_QUESTIONS`. Then when we restore the questions, we check if we will be restoring the original activity they belong to, and skip trying to match them to an existing category. This means a new copy of each question will be created, and moved to the new copy of the qbank once it is created. --- .../backup/util/dbops/restore_dbops.class.php | 48 ++- .../helper/restore_prechecks_helper.class.php | 1 + ...restore_questionbanks_parser_processor.php | 59 +++ .../backup/util/includes/restore_includes.php | 1 + .../mod/qbank/tests/backup/restore_test.php | 236 +++++++++++ .../tests/backup/repeated_restore_test.php | 399 ++++++++++++++++-- 6 files changed, 717 insertions(+), 27 deletions(-) create mode 100644 public/backup/util/helper/restore_questionbanks_parser_processor.php create mode 100644 public/mod/qbank/tests/backup/restore_test.php diff --git a/public/backup/util/dbops/restore_dbops.class.php b/public/backup/util/dbops/restore_dbops.class.php index 60872fb42c3..cd68126ff9f 100644 --- a/public/backup/util/dbops/restore_dbops.class.php +++ b/public/backup/util/dbops/restore_dbops.class.php @@ -464,6 +464,39 @@ abstract class restore_dbops { $xmlparser->process(); } + /** + * Store ids associated with any activity in the backup that supports FEATURE_PUBLISHES_QUESTIONS. + * + * @param string $restoreid The restore ID. + * @param string $activitiespath The path to the `activities` folder in the backup being restored. + */ + public static function load_questionbanks_to_tempids(string $restoreid, string $activitiespath): void { + if (!is_dir($activitiespath)) { + return; + } + // Get modules that publish questions. + $qmodules = array_filter( + array_keys(core\component::get_all_plugins_list('mod')), + fn($module) => plugin_supports('mod', $module, FEATURE_PUBLISHES_QUESTIONS), + ); + foreach (scandir($activitiespath) as $activitydir) { + [$modname] = explode('_', $activitydir); + if (!in_array($modname, $qmodules)) { + continue; + } + $activityfile = "{$activitiespath}/{$activitydir}/{$modname}.xml"; + if (!file_exists($activityfile)) { // Shouldn't happen ever, but... + throw new backup_helper_exception('missing_moodle_backup_xml_file', $activityfile); + } + // Parse each activity's file, storing the relevant data in the database. + $xmlparser = new progressive_parser(); + $xmlparser->set_file($activityfile); + $xmlprocessor = new restore_questionbanks_parser_processor($restoreid); + $xmlparser->set_processor($xmlprocessor); + $xmlparser->process(); + } + } + /** * Check all the included categories and questions, deciding the action to perform * for each one (mapping / creation) and returning one array of problems in case @@ -609,9 +642,18 @@ abstract class restore_dbops { $topcats = 0; // get categories in context (bank) $categories = self::restore_get_question_categories($restoreid, $contextid, $contextlevel); - - // cache permissions if $targetcontext is found - if ($targetcontext = self::restore_find_best_target_context($categories, $courseid, $contextlevel)) { + if ( + $contextlevel == \core\context\module::LEVEL + && self::get_backup_ids_record($restoreid, 'questionbank', $contextid) + ) { + // Don't look for an existing module context, we have the original context in the backup, + // so we'll put the categories in the course context for now and move them once the activity is restored. + $targetcontext = core\context\course::instance($courseid); + } else { + $targetcontext = self::restore_find_best_target_context($categories, $courseid, $contextlevel); + } + if ($targetcontext) { + // Cache permissions if $targetcontext is found. $canmanagecategory = has_capability('moodle/question:managecategory', $targetcontext, $userid); $canadd = has_capability('moodle/question:add', $targetcontext, $userid); } diff --git a/public/backup/util/helper/restore_prechecks_helper.class.php b/public/backup/util/helper/restore_prechecks_helper.class.php index 803cd77903d..4dac448851c 100644 --- a/public/backup/util/helper/restore_prechecks_helper.class.php +++ b/public/backup/util/helper/restore_prechecks_helper.class.php @@ -173,6 +173,7 @@ abstract class restore_prechecks_helper { $progress->progress($majorstep++); // Check we are able to restore and the categories and questions + restore_dbops::load_questionbanks_to_tempids($restoreid, $controller->get_plan()->get_basepath() . '/activities'); $file = $controller->get_plan()->get_basepath() . '/questions.xml'; restore_dbops::load_categories_and_questions_to_tempids($restoreid, $file); if ($problems = restore_dbops::precheck_categories_and_questions($restoreid, $courseid, $userid, $samesite)) { diff --git a/public/backup/util/helper/restore_questionbanks_parser_processor.php b/public/backup/util/helper/restore_questionbanks_parser_processor.php new file mode 100644 index 00000000000..10a76750dbb --- /dev/null +++ b/public/backup/util/helper/restore_questionbanks_parser_processor.php @@ -0,0 +1,59 @@ +. + +defined('MOODLE_INTERNAL' || die()); + +require_once($CFG->dirroot . '/backup/util/xml/parser/processors/grouped_parser_processor.class.php'); + +/** + * Parse and store activity data for activities that publish questions. + * + * @package core + * @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 + */ +class restore_questionbanks_parser_processor extends grouped_parser_processor { + /** + * Store the restore ID and register paths. + * + * @param string $restoreid ID of the backup being restored. + */ + public function __construct( + /** @var string ID of the backup being restored */ + protected string $restoreid, + ) { + parent::__construct(); + $this->add_path('/activity'); + } + + #[\Override] + protected function dispatch_chunk($data): void { + // Recieved one chunk, store the context ID as that's what we will match question categories against. + $itemid = $data['tags']['contextid']; + restore_dbops::set_backup_ids_record($this->restoreid, 'questionbank', $itemid); + } + + #[\Override] + protected function notify_path_start($path) { + // Nothing to do. + } + + #[\Override] + protected function notify_path_end($path) { + // Nothing to do. + } +} diff --git a/public/backup/util/includes/restore_includes.php b/public/backup/util/includes/restore_includes.php index 9b8ab15e84d..94b5a38eb63 100644 --- a/public/backup/util/includes/restore_includes.php +++ b/public/backup/util/includes/restore_includes.php @@ -39,6 +39,7 @@ require_once($CFG->dirroot . '/backup/util/helper/backup_anonymizer_helper.class require_once($CFG->dirroot . '/backup/util/helper/backup_file_manager.class.php'); require_once($CFG->dirroot . '/backup/util/helper/copy_helper.class.php'); require_once($CFG->dirroot . '/backup/util/helper/restore_prechecks_helper.class.php'); +require_once($CFG->dirroot . '/backup/util/helper/restore_questionbanks_parser_processor.php'); require_once($CFG->dirroot . '/backup/util/helper/restore_moodlexml_parser_processor.class.php'); require_once($CFG->dirroot . '/backup/util/helper/restore_inforef_parser_processor.class.php'); require_once($CFG->dirroot . '/backup/util/helper/restore_users_parser_processor.class.php'); diff --git a/public/mod/qbank/tests/backup/restore_test.php b/public/mod/qbank/tests/backup/restore_test.php new file mode 100644 index 00000000000..74bf01213b7 --- /dev/null +++ b/public/mod/qbank/tests/backup/restore_test.php @@ -0,0 +1,236 @@ +. + +namespace mod_qbank\backup; + +use core\context\module; + +/** + * Tests to cover restoring or import a question bank and its questions multiple times to the same course. + * + * When we restore/import/duplicate an activity that publishes questions, all of its questions should + * be restored into the restored activity, even if they already exist in another activity on the same course. + * + * @package mod_qbank + * @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_dbops::load_questionbanks_to_tempids + * @covers \restore_questionbanks_parser_processor + */ +final class restore_test extends \advanced_testcase { + /** + * Given a context, find the default question category, return the question IDs and records. + * + * @param int $contextid + * @return array[int[], \stdClass[]] An array of the question ID, and an numerically-indexed array of question records. + */ + protected function get_questions_in_default_category(int $contextid): array { + global $DB; + $records = $DB->get_records_sql( + " + SELECT q.* + FROM {question} q + JOIN {question_versions} qv ON q.id = qv.questionid + JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid + JOIN {question_categories} qc ON qc.id = qbe.questioncategoryid + WHERE qc.contextid = :contextid + AND qc.name != :top + ", + [ + 'contextid' => $contextid, + 'top' => 'top', + ], + ); + return [array_keys($records), array_values($records)]; + } + + /** + * Create a qbank containing questions and verify the correct records exist. + * + * @param int $courseid + * @return array + */ + protected function create_qbank_with_questions(int $courseid): array { + // Create a quiz with questions in the first course. + $qbank = $this->getDataGenerator()->get_plugin_generator('mod_qbank')->create_instance(['course' => $courseid]); + $context = \context_module::instance($qbank->cmid); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + $cat = question_get_default_category($context->id); + + // Create a short answer question. + $saq = $questiongenerator->create_question('shortanswer', null, ['category' => $cat->id]); + // Create a multi-answer question with children. + $maq = $questiongenerator->create_question('multianswer', 'twosubq', ['category' => $cat->id]); + + // Verify that we have 1 qbank, with a default category containing 4 questions (2, plus 2 children). + $qbanks = get_fast_modinfo($courseid)->get_instances_of('qbank'); + $this->assertCount(1, $qbanks); + $qbank1 = reset($qbanks); + $qbankcontext = module::instance($qbank1->id); + [, $qbankquestions] = $this->get_questions_in_default_category($qbankcontext->id); + $this->assertCount(4, $qbankquestions); + return [$qbank, $saq, $maq]; + } + + /** + * Importing (duplicating) a bank in the same course should give you a second bank with a copy of all the questions. + */ + public function test_import_qbank_into_same_course(): void { + global $CFG, $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(); + + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + + [$qbank, $originalsaq, $originalmaq] = $this->create_qbank_with_questions($course1->id); + + // Backup qbank. + $bc = new \backup_controller( + \backup::TYPE_1ACTIVITY, + $qbank->cmid, + \backup::FORMAT_MOODLE, + \backup::INTERACTIVE_NO, + \backup::MODE_IMPORT, + $teacher->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, + $teacher->id, + \backup::TARGET_CURRENT_ADDING, + ); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify that we now have 2 qbanks. + $qbanks = get_fast_modinfo($course1->id)->get_instances_of('qbank'); + $this->assertCount(2, $qbanks); + // The first qbank should be the same as before. + $qbank1 = reset($qbanks); + $qbank1context = module::instance($qbank1->id); + [$qbank1questionids, $qbank1questions] = $this->get_questions_in_default_category($qbank1context->id); + $this->assertCount(4, $qbank1questions); + $this->assertContains((int) $originalsaq->id, $qbank1questionids); + $this->assertContains((int) $originalmaq->id, $qbank1questionids); + + // The second qbank should have its own categories and a copy of each question. + $qbank2 = end($qbanks); + $qbank2context = module::instance($qbank2->id); + [$qbank2questionids, $qbank2questions] = $this->get_questions_in_default_category($qbank2context->id); + $this->assertCount(4, $qbank2questions); + foreach ($qbank2questions as $key => $qbank2question) { + $this->assertNotContains((int) $qbank2question->id, $qbank1questionids); + $this->assertEquals($qbank2question->questiontext, $qbank1questions[$key]->questiontext); + if ($qbank2question->parent != 0) { + // Check that child questions are linked to the parent in the same qbank. + $this->assertNotContains((int) $qbank2question->parent, $qbank1questionids); + $this->assertContains((int) $qbank2question->parent, $qbank2questionids); + } + } + } + + /** + * Importing a bank into a different course multiple times should copy all the questions each time. + */ + public function test_import_qbank_into_different_course_twice(): void { + global $CFG, $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(); + + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $course2 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $generator->enrol_user($teacher->id, $course2->id, 'editingteacher'); + + [$qbank, $originalsaq, $originalmaq] = $this->create_qbank_with_questions($course1->id); + + for ($i = 0, $j = 2; $i < $j; $i++) { + // Backup qbank. + $bc = new \backup_controller( + \backup::TYPE_1ACTIVITY, + $qbank->cmid, + \backup::FORMAT_MOODLE, + \backup::INTERACTIVE_NO, + \backup::MODE_IMPORT, + $teacher->id, + ); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup into another course twice. + $rc = new \restore_controller( + $backupid, + $course2->id, + \backup::INTERACTIVE_NO, + \backup::MODE_IMPORT, + $teacher->id, + \backup::TARGET_CURRENT_ADDING, + ); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + } + + $qbankcontext = module::instance($qbank->cmid); + [$seenquestionids] = $this->get_questions_in_default_category($qbankcontext->id); + + // Verify that we have 2 qbanks on the destination course, each with its own categories and questions. + $qbanks = get_fast_modinfo($course2->id)->get_instances_of('qbank'); + $this->assertCount(2, $qbanks); + foreach ($qbanks as $qbank) { + $qbankcontext = module::instance($qbank->id); + [$qbankquestionids, $qbankquestions] = $this->get_questions_in_default_category($qbankcontext->id); + $this->assertCount(4, $qbankquestions); + // The new question bank doesn't contain the original questions. + $this->assertNotContains((int) $originalsaq->id, $qbankquestionids); + $this->assertNotContains((int) $originalmaq->id, $qbankquestionids); + // The new question bank doesn't contain questions from any other question bank. + $this->assertEmpty(array_intersect($seenquestionids, $qbankquestionids)); + $seenquestionids = array_merge($seenquestionids, $qbankquestionids); + // The new question bank does contain its own copy of the questions. + $this->assertNotEmpty( + array_filter($qbankquestions, fn($question) => $question->questiontext == $originalsaq->questiontext) + ); + $qbankmaq = array_filter($qbankquestions, fn($question) => $question->questiontext == $originalmaq->questiontext); + $qbankmaq = reset($qbankmaq); + $this->assertNotFalse($qbankmaq); + // It also contains 2 children of the multianswer question. + $this->assertCount(2, array_filter($qbankquestions, fn($question) => $question->parent == $qbankmaq->id)); + } + } +} diff --git a/public/mod/quiz/tests/backup/repeated_restore_test.php b/public/mod/quiz/tests/backup/repeated_restore_test.php index 86be7538eef..bdf8bba3689 100644 --- a/public/mod/quiz/tests/backup/repeated_restore_test.php +++ b/public/mod/quiz/tests/backup/repeated_restore_test.php @@ -46,14 +46,14 @@ final class repeated_restore_test extends advanced_testcase { use quiz_question_helper_test_trait; /** - * Restore a quiz twice into the same target course, and verify the quiz uses the restored questions both times. + * Create 2 courses, and a quiz with questions on the first course. + * + * @param bool $sharedquestions If true, create the questions in a qbank module rather than the quiz itself. + * @return array */ - public function test_restore_quiz_into_other_course_twice(): void { + protected function create_courses_and_quiz(bool $sharedquestions = false): array { global $USER; - $this->resetAfterTest(); - $this->setAdminUser(); - - // Step 1: Create two courses and a user with editing teacher capabilities. + // Create two courses and a user with editing teacher capabilities. $generator = $this->getDataGenerator(); $course1 = $generator->create_course(); $course2 = $generator->create_course(); @@ -63,8 +63,12 @@ final class repeated_restore_test extends advanced_testcase { // Create a quiz with questions in the first course. $quiz = $this->create_test_quiz($course1); - $qbank = $generator->get_plugin_generator('mod_qbank')->create_instance(['course' => $course1->id]); - $context = \context_module::instance($qbank->cmid); + if ($sharedquestions) { + $qbank = $generator->get_plugin_generator('mod_qbank')->create_instance(['course' => $course1->id]); + $context = \context_module::instance($qbank->cmid); + } else { + $context = \context_module::instance($quiz->cmid); + } $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); // Create a question category. @@ -97,23 +101,56 @@ final class repeated_restore_test extends advanced_testcase { $modules1 = get_fast_modinfo($course1->id)->get_instances_of('quiz'); $module1 = reset($modules1); $questionscourse1 = \mod_quiz\question\bank\qbank_helper::get_question_structure( - $module1->instance, $module1->context); + $module1->instance, + $module1->context, + ); $originalquestionids = []; foreach ($questionscourse1 as $slot) { array_push($originalquestionids, intval($slot->questionid)); } - // Step 2: Backup the first course. - $bc = new backup_controller(backup::TYPE_1COURSE, $course1->id, backup::FORMAT_MOODLE, - backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + return [ + $course1, + $course2, + $quiz, + $teacher, + $originalquestionids, + ]; + } + + /** + * Restore a quiz using private questions twice into the same target course, + * and verify the quiz uses a newly-restored copy of the questions each time. + */ + public function test_restore_quiz_with_own_questions_into_other_course_twice(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + [, $course2, $quiz, $teacher, $originalquestionids] = $this->create_courses_and_quiz(); + + // Backup the quiz course. + $bc = new backup_controller( + backup::TYPE_1ACTIVITY, + $quiz->cmid, + backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + ); $backupid = $bc->get_backupid(); $bc->execute_plan(); $bc->destroy(); - // Step 3: Import the backup into the second course. - $rc = new restore_controller($backupid, $course2->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, - $teacher->id, backup::TARGET_CURRENT_ADDING); + // Import the backup into the second course. + $rc = new restore_controller( + $backupid, + $course2->id, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + backup::TARGET_CURRENT_ADDING, + ); $rc->execute_precheck(); $rc->execute_plan(); $rc->destroy(); @@ -123,34 +160,348 @@ final class repeated_restore_test extends advanced_testcase { $modules2 = get_fast_modinfo($course2->id)->get_instances_of('quiz'); $module2 = reset($modules2); $questionscourse2firstimport = \mod_quiz\question\bank\qbank_helper::get_question_structure( - $module2->instance, $module2->context); + $module2->instance, + $module2->context, + ); foreach ($questionscourse2firstimport as $slot) { - $this->assertNotContains(intval($slot->questionid), $originalquestionids, - "Question ID $slot->questionid should not be in the original course's question IDs."); + $this->assertNotContains( + intval($slot->questionid), + $originalquestionids, + "Question ID $slot->questionid should not be in the original course's question IDs.", + ); } // Repeat the backup and import process to simulate a second import. - $bc = new backup_controller(backup::TYPE_1COURSE, $course1->id, backup::FORMAT_MOODLE, - backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $bc = new backup_controller( + backup::TYPE_1ACTIVITY, + $quiz->cmid, + backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + ); $backupid = $bc->get_backupid(); $bc->execute_plan(); $bc->destroy(); - $rc = new restore_controller($backupid, $course2->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, - $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc = new restore_controller( + $backupid, + $course2->id, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + backup::TARGET_CURRENT_ADDING, + ); $rc->execute_precheck(); $rc->execute_plan(); $rc->destroy(); - // Verify that the second restore has used the same new questions that were created by the first restore. + // Verify that the quiz in the second restore uses a third copy of the questions. $modules3 = get_fast_modinfo($course2->id)->get_instances_of('quiz'); $module3 = end($modules3); $questionscourse2secondimport = \mod_quiz\question\bank\qbank_helper::get_question_structure( - $module3->instance, $module3->context); + $module3->instance, + $module3->context, + ); + + foreach ($questionscourse2secondimport as $slot) { + $this->assertNotEquals($questionscourse2firstimport[$slot->slot]->questionid, $slot->questionid); + $this->assertNotContains( + intval($slot->questionid), + $originalquestionids, + "Question ID $slot->questionid should not be in the original course's question IDs.", + ); + } + } + + /** + * Restore a quiz using shared questions twice into the same target course, + * and verify the quiz uses the original questions each time. + */ + public function test_restore_quiz_with_shared_questions_into_other_course_twice(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + [, $course2, $quiz, $teacher, $originalquestionids] = $this->create_courses_and_quiz(true); + + // Backup the quiz. + $bc = new backup_controller( + backup::TYPE_1ACTIVITY, + $quiz->cmid, + backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + ); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Import the backup into the second course. + $rc = new restore_controller( + $backupid, + $course2->id, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + backup::TARGET_CURRENT_ADDING, + ); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify the question ids from the quiz in the original course are the same as + // the question ids in the duplicated quiz in the second course. + $modules2 = get_fast_modinfo($course2->id)->get_instances_of('quiz'); + $module2 = reset($modules2); + $questionscourse2firstimport = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $module2->instance, + $module2->context, + ); + + foreach ($questionscourse2firstimport as $slot) { + $this->assertContains( + intval($slot->questionid), + $originalquestionids, + "Question ID $slot->questionid should be in the original course's question IDs.", + ); + } + + // Repeat the backup and import process to simulate a second import. + $bc = new backup_controller( + backup::TYPE_1ACTIVITY, + $quiz->cmid, + backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + ); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + $rc = new restore_controller( + $backupid, + $course2->id, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + backup::TARGET_CURRENT_ADDING, + ); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify that the quiz in the second restore uses the same questions from the original quiz and the first restore. + $modules3 = get_fast_modinfo($course2->id)->get_instances_of('quiz'); + $module3 = end($modules3); + $questionscourse2secondimport = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $module3->instance, + $module3->context, + ); foreach ($questionscourse2secondimport as $slot) { $this->assertEquals($questionscourse2firstimport[$slot->slot]->questionid, $slot->questionid); + $this->assertContains( + intval($slot->questionid), + $originalquestionids, + "Question ID $slot->questionid should be in the original course's question IDs.", + ); + } + } + + /** + * Restore a quiz using shared questions twice into the same target course with the qbank, + * and verify the quiz uses a newly-restored copy of the questions each time. + */ + public function test_restore_quiz_with_qbank_into_other_course_twice(): void { + global $DB; + $this->resetAfterTest(); + $this->setAdminUser(); + + [$course1, $course2, , $teacher, $originalquestionids] = $this->create_courses_and_quiz(true); + + // Backup the first course. + $bc = new backup_controller( + backup::TYPE_1COURSE, + $course1->id, + backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + ); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Import the backup into the second course. + $rc = new restore_controller( + $backupid, + $course2->id, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + backup::TARGET_CURRENT_ADDING, + ); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify the question ids from the quiz in the original course are different + // from the question ids in the duplicated quiz in the second course. + $modules2 = get_fast_modinfo($course2->id)->get_instances_of('quiz'); + $module2 = reset($modules2); + $questionscourse2firstimport = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $module2->instance, + $module2->context, + ); + + foreach ($questionscourse2firstimport as $slot) { + $this->assertNotContains( + intval($slot->questionid), + $originalquestionids, + "Question ID $slot->questionid should not be in the original course's question IDs.", + ); + } + + // Repeat the backup and import process to simulate a second import. + $bc = new backup_controller( + backup::TYPE_1COURSE, + $course1->id, + backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + ); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + $rc = new restore_controller( + $backupid, + $course2->id, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + backup::TARGET_CURRENT_ADDING, + ); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify that the quiz in the second restore uses a third copy of the questions. + $modules3 = get_fast_modinfo($course2->id)->get_instances_of('quiz'); + $module3 = end($modules3); + $questionscourse2secondimport = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $module3->instance, + $module3->context, + ); + + foreach ($questionscourse2secondimport as $slot) { + $this->assertNotEquals($questionscourse2firstimport[$slot->slot]->questionid, $slot->questionid); + $this->assertNotContains( + intval($slot->questionid), + $originalquestionids, + "Question ID $slot->questionid should not be in the original course's question IDs.", + ); + } + } + + /** + * Import the quiz and qbank from course 1 to course 2, then import just the quiz a second time. + * + * As the user can use questions from the original qbank, the second quiz will use the original questions. + */ + public function test_restore_quiz_with_shared_questions_then_just_quiz(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + [$course1, $course2, $quiz, $teacher, $originalquestionids] = $this->create_courses_and_quiz(true); + + // Backup the first course. + $bc = new backup_controller( + backup::TYPE_1COURSE, + $course1->id, + backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + ); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Import the backup into the second course. + $rc = new restore_controller( + $backupid, + $course2->id, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + backup::TARGET_CURRENT_ADDING, + ); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify the question ids from the quiz in the original course are different + // from the question ids in the duplicated quiz in the second course. + $modules2 = get_fast_modinfo($course2->id)->get_instances_of('quiz'); + $module2 = reset($modules2); + $questionscourse2firstimport = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $module2->instance, + $module2->context, + ); + + foreach ($questionscourse2firstimport as $slot) { + $this->assertNotContains( + intval($slot->questionid), + $originalquestionids, + "Question ID $slot->questionid should not be in the original course's question IDs.", + ); + } + + // Repeat the backup and import process with just the quiz. + $bc = new backup_controller( + backup::TYPE_1ACTIVITY, + $quiz->cmid, + backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + ); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + $rc = new restore_controller( + $backupid, + $course2->id, + backup::INTERACTIVE_NO, + backup::MODE_IMPORT, + $teacher->id, + backup::TARGET_CURRENT_ADDING, + ); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify that the quiz in the second restore uses the questions from the original course. + $modules3 = get_fast_modinfo($course2->id)->get_instances_of('quiz'); + $module3 = end($modules3); + $questionscourse2secondimport = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $module3->instance, + $module3->context, + ); + + foreach ($questionscourse2secondimport as $slot) { + $this->assertNotEquals($questionscourse2firstimport[$slot->slot]->questionid, $slot->questionid); + $this->assertContains( + intval($slot->questionid), + $originalquestionids, + "Question ID $slot->questionid should be in the original course's question IDs.", + ); } } From 94a802093b5d9f2e31630081ac77462bb34d0c49 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Mon, 12 Jan 2026 10:33:10 +0000 Subject: [PATCH 2/2] MDL-86136 question: Amend version restore test Restoring a whole course will now restore the backed-up questions to a new qbank instance. This means the test to restore versions was not restoring the deleted versions to the original question bank entires, but to a new question bank entry in the new qbank. Updating the test to just backup and restore the quiz resolves this. --- public/question/tests/backup_test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/public/question/tests/backup_test.php b/public/question/tests/backup_test.php index 14b5e017b47..6266385346e 100644 --- a/public/question/tests/backup_test.php +++ b/public/question/tests/backup_test.php @@ -890,7 +890,7 @@ final class backup_test extends \advanced_testcase { quiz_add_quiz_question($questionv2->id, $testdata->quiz); $structure1->update_slot_version($structure1->get_slot_id_for_slot(1), 2); - $backupid = $this->backup_course($testdata->course); + $backupid = $this->backup_course_module($testdata->quiz->cmid); question_delete_question($questionv4->id); // Actually deleted. question_delete_question($questionv3->id); // Actually deleted. @@ -1006,7 +1006,7 @@ final class backup_test extends \advanced_testcase { quiz_add_quiz_question($questionv2->id, $testdata->quiz); $structure1->update_slot_version($structure1->get_slot_id_for_slot(1), 2); - $backupid = $this->backup_course($testdata->course); + $backupid = $this->backup_course_module($testdata->quiz->cmid); question_delete_question($questionv4->id); // Actually deleted. question_delete_question($questionv3->id); // Actually deleted.