diff --git a/public/backup/moodle2/restore_root_task.class.php b/public/backup/moodle2/restore_root_task.class.php index b3c31cf246b..132a710ae86 100644 --- a/public/backup/moodle2/restore_root_task.class.php +++ b/public/backup/moodle2/restore_root_task.class.php @@ -78,6 +78,9 @@ class restore_root_task extends restore_task { // Unconditionally, load create all the needed outcomes $this->add_step(new restore_outcomes_structure_step('create_scales', 'outcomes.xml')); + // If we haven't preloaded information, load all the question banks to temp_ids_table. + $this->add_step(new \core\backup\restore_load_questionbanks('load_questionbanks')); + // If we haven't preloaded information, load all the needed categories and questions (reduced) to temp_ids_table $this->add_step(new restore_load_categories_and_questions('load_categories_and_questions')); diff --git a/public/backup/moodle2/restore_stepslib.php b/public/backup/moodle2/restore_stepslib.php index 86994a1576e..b9f7fe2b031 100644 --- a/public/backup/moodle2/restore_stepslib.php +++ b/public/backup/moodle2/restore_stepslib.php @@ -6510,6 +6510,12 @@ trait restore_question_set_reference_data_trait { $data->usingcontextid = $this->get_mappingid('context', $data->usingcontextid); $data->itemid = $this->get_new_parentid('quiz_question_instance'); + $originalbankinbackup = (bool) restore_dbops::get_backup_ids_record( + $this->get_restoreid(), + 'questionbank', + $data->questionscontextid, + ); + if ($context = $this->get_mappingid('context', $data->questionscontextid)) { $data->questionscontextid = $context; } else { @@ -6538,7 +6544,7 @@ trait restore_question_set_reference_data_trait { $qbankfeature = new $qbankfeatureclass(); $filters = $qbankfeature->get_question_filters(); foreach ($filters as $filter) { - $filtercondition = $filter->restore_filtercondition($filtercondition, $data, $this); + $filtercondition = $filter->restore_filtercondition($filtercondition, $data, $this, $originalbankinbackup); } } diff --git a/public/lib/classes/backup/restore_load_questionbanks.php b/public/lib/classes/backup/restore_load_questionbanks.php new file mode 100644 index 00000000000..fc069e6d96a --- /dev/null +++ b/public/lib/classes/backup/restore_load_questionbanks.php @@ -0,0 +1,48 @@ +. + +namespace core\backup; + +defined('MOODLE_INTERNAL') || die(); +require_once($CFG->dirroot . '/backup/util/plan/restore_execution_step.class.php'); + +/** + * Load question bank context IDs. + * + * Execution step that, *conditionally* (if there isn't preloaded information) + * will load the context IDs of activities in the backup containing questions + * to backup_temp_ids. They will be stored with "questionbank" itemname and their + * original context ID as itemid. + * + * @package core + * @copyright 2026 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_load_questionbanks extends \restore_execution_step { + /** + * If restore data has not already been loaded in this request, load the question bank context IDs. + */ + protected function define_execution(): void { + global $CFG; + require_once($CFG->dirroot . '/backup/util/dbops/restore_dbops.class.php'); + if ($this->task->get_preloaded_information()) { // If info is already preloaded, nothing to do. + return; + } + $path = $this->get_basepath() . '/activities'; + \restore_dbops::load_questionbanks_to_tempids($this->get_restoreid(), $path); + } +} diff --git a/public/mod/qbank/tests/backup/restore_test.php b/public/mod/qbank/tests/backup/restore_test.php index 74bf01213b7..7e75d736538 100644 --- a/public/mod/qbank/tests/backup/restore_test.php +++ b/public/mod/qbank/tests/backup/restore_test.php @@ -17,6 +17,11 @@ namespace mod_qbank\backup; use core\context\module; +use core_question\local\bank\condition; +use mod_quiz\external\add_random_questions; +use mod_quiz\question\bank\filter\custom_category_condition; +use mod_quiz\quiz_settings; +use mod_quiz\structure; /** * Tests to cover restoring or import a question bank and its questions multiple times to the same course. @@ -233,4 +238,97 @@ final class restore_test extends \advanced_testcase { $this->assertCount(2, array_filter($qbankquestions, fn($question) => $question->parent == $qbankmaq->id)); } } + + public function test_update_set_reference_on_restore(): void { + global $DB, $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(); + + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + [, $course, $qcat, , $qbank] = $questiongenerator->setup_course_and_questions(); + $qbankcontext = module::instance($qbank->cmid); + $filtercondition = [ + 'cat' => "{$qcat->id},{$qbankcontext->id}", + 'sortdata' => [], + 'filter' => [ + 'category' => [ + 'jointype' => condition::JOINTYPE_DEFAULT, + 'values' => [$qcat->id], + 'filteroptions' => ['includesubcategories' => false], + ], + ], + ]; + $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance(['course' => $course->id]); + $settings = quiz_settings::create_for_cmid($quiz->cmid); + $structure = structure::create_for_quiz($settings); + $structure->add_random_questions(1, 1, $filtercondition); + + $setreference = $DB->get_record( + 'question_set_references', + [ + 'component' => 'mod_quiz', + 'questionarea' => 'slot', + 'usingcontextid' => $settings->get_context()->id, + 'questionscontextid' => $qbankcontext->id, + ], + ); + $this->assertNotFalse($setreference); + + // Backup qbank. + $bc = new \backup_controller( + \backup::TYPE_1COURSE, + $course->id, + \backup::FORMAT_MOODLE, + \backup::INTERACTIVE_NO, + \backup::MODE_IMPORT, + $USER->id, + ); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + $course2 = $this->getDataGenerator()->create_course(); + // Restore the backup into another course twice. + $rc = new \restore_controller( + $backupid, + $course2->id, + \backup::INTERACTIVE_NO, + \backup::MODE_IMPORT, + $USER->id, + \backup::TARGET_CURRENT_ADDING, + ); + $rc->execute_precheck(droptemptablesafter: true); // Dropping temp tables simulates an asynchronous restore. + $rc->execute_plan(); + $rc->destroy(); + + $modinfo = get_fast_modinfo($course2->id); + $newquizzes = $modinfo->get_instances_of('quiz'); + $newquiz = reset($newquizzes); + $newqbanks = $modinfo->get_instances_of('qbank'); + $newqbank = reset($newqbanks); + $newqcat = $DB->get_record('question_categories', ['contextid' => $newqbank->context->id, 'stamp' => $qcat->stamp]); + + $newsetreference = $DB->get_record( + 'question_set_references', + [ + 'component' => 'mod_quiz', + 'questionarea' => 'slot', + 'usingcontextid' => $newquiz->context->id, + 'questionscontextid' => $newqbank->context->id, + ], + ); + $this->assertNotFalse($newsetreference); + $newfiltercondition = json_decode($newsetreference->filtercondition, true); + $this->assertEquals($newfiltercondition['cat'], "{$newqcat->id},{$newqbank->context->id}"); + $this->assertEquals($newfiltercondition['filter']['category']['values'][0], $newqcat->id); + + $newsettings = quiz_settings::create_for_cmid($newquiz->id); + $newstructure = structure::create_for_quiz($newsettings); + $randomslot = $newstructure->get_slot_by_number(1); + $this->assertEquals($randomslot->id, $newsetreference->itemid); + } } diff --git a/public/question/bank/managecategories/classes/category_condition.php b/public/question/bank/managecategories/classes/category_condition.php index aa28e7dc2a9..48ff11d3ffc 100644 --- a/public/question/bank/managecategories/classes/category_condition.php +++ b/public/question/bank/managecategories/classes/category_condition.php @@ -311,6 +311,7 @@ class category_condition extends condition { array $filtercondition, stdClass $setreference, restore_questions_activity_structure_step $restorestep, + bool $originalbankinbackup = false, ): array { global $DB; // Map category id used for category filter condition and corresponding context id. @@ -325,13 +326,12 @@ class category_condition extends condition { !$restorestep->get_task()->is_samesite() || !$questionscontext || !$DB->record_exists('question_categories', ['id' => $oldcategoryid]) + || $originalbankinbackup || $setreference->usingcontextid == $setreference->questionscontextid || !has_capability('moodle/question:useall', $questionscontext) ) { $newcategoryid = $restorestep->get_mappingid('question_category', $oldcategoryid); $filtercondition['filter']['category']['values'][0] = $newcategoryid; - // Make sure the questions context matches the new category. - $setreference->questionscontextid = $DB->get_field('question_categories', 'contextid', ['id' => $newcategoryid]); } $filtercondition['cat'] = implode( diff --git a/public/question/bank/managecategories/tests/category_condition_test.php b/public/question/bank/managecategories/tests/category_condition_test.php index 7e13c0c9526..60a7b682b25 100644 --- a/public/question/bank/managecategories/tests/category_condition_test.php +++ b/public/question/bank/managecategories/tests/category_condition_test.php @@ -249,31 +249,26 @@ final class category_condition_test extends \advanced_testcase { } /** - * When the category is updated in the filter condition, the context ID is also updated. + * As {@see test_restore_filtercondition}, but the original question bank was present in the backup. */ - public function test_restore_filtercondition_update_context(): void { + public function test_restore_filtercondition_questionbankinbackup(): void { $this->resetAfterTest(); $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); $category = $questiongenerator->create_question_category(); - $fakecategory = $category->id + 1; - $fakecontext = $category->contextid + 1; $filtercondition = [ 'filter' => [ 'category' => [ 'values' => [ - $fakecategory, + $category->id, ], ], ], - 'cat' => [ - "{$fakecategory},{$fakecontext}", - ], ]; $setreference = (object) [ - 'questionscontextid' => $fakecontext, - 'usingcontextid' => $category->contextid, + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid + 1, ]; - $mappedid = $category->id; + $mappedid = $category->id + 1; $mockstep = $this->get_mock_step($this->get_samesite_task()); $mockstep->method('get_mappingid')->willReturn($mappedid); @@ -281,10 +276,8 @@ final class category_condition_test extends \advanced_testcase { $condition = new category_condition(); - $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep); + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep, true); $this->assertEquals($mappedid, $filtercondition['filter']['category']['values'][0]); - $this->assertEquals($category->contextid, $setreference->questionscontextid); - $this->assertEquals($filtercondition['cat'], "{$mappedid},{$category->contextid}"); } } diff --git a/public/question/bank/tagquestion/classes/tag_condition.php b/public/question/bank/tagquestion/classes/tag_condition.php index 02c6a3f3c9c..b6360fce405 100644 --- a/public/question/bank/tagquestion/classes/tag_condition.php +++ b/public/question/bank/tagquestion/classes/tag_condition.php @@ -167,6 +167,7 @@ class tag_condition extends condition { array $filtercondition, stdClass $setreference, restore_questions_activity_structure_step $restorestep, + bool $originalbankinbackup = false, ): array { if (isset($filtercondition['filter']['qtagids'])) { $newtagids = []; diff --git a/public/question/bank/tagquestion/tests/tag_condition_test.php b/public/question/bank/tagquestion/tests/tag_condition_test.php index 41f49e59e1a..7ce8d7e8bb3 100644 --- a/public/question/bank/tagquestion/tests/tag_condition_test.php +++ b/public/question/bank/tagquestion/tests/tag_condition_test.php @@ -521,4 +521,37 @@ final class tag_condition_test extends \advanced_testcase { $this->assertArrayNotHasKey('qtagids', $filtercondition['filter']); } + + /** + * Restoring the filter when the original question bank was in the backup doesn't change the behaviour. + */ + public function test_restore_filtercondition_originalbankinbackup(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $category = $questiongenerator->create_question_category(); + $tag1 = $this->getDataGenerator()->create_tag(); + $tag2 = $this->getDataGenerator()->create_tag(); + $filtercondition = [ + 'filter' => [ + 'qtagids' => [ + 'values' => [ + $tag1->id, + $tag2->id, + ], + ], + ], + ]; + $setreference = (object) [ + 'questionscontextid' => $category->contextid, + 'usingcontextid' => $category->contextid, + ]; + $mockstep = $this->get_mock_step($this->get_samesite_task()); + $mockstep->method('get_mappingid')->willReturn($tag2->id + 1); + + $condition = new tag_condition(); + + $filtercondition = $condition->restore_filtercondition($filtercondition, $setreference, $mockstep, true); + + $this->assertEquals([$tag1->id, $tag2->id], $filtercondition['filter']['qtagids']['values']); + } } diff --git a/public/question/classes/local/bank/condition.php b/public/question/classes/local/bank/condition.php index 8897881d7bc..92c6dcc2293 100644 --- a/public/question/classes/local/bank/condition.php +++ b/public/question/classes/local/bank/condition.php @@ -249,12 +249,14 @@ abstract class condition { * @param array $filtercondition The origin $filtercondition with preceding conversions applied. * @param stdClass $setreference The set reference record from the backup. * @param restore_questions_activity_structure_step $restorestep The restore step. + * @param bool $originalbankinbackup Was the original question bank these questions belonged to included in the backup? * @return array the modified $filtercondition */ public function restore_filtercondition( array $filtercondition, stdClass $setreference, restore_questions_activity_structure_step $restorestep, + bool $originalbankinbackup = false, ): array { return $filtercondition; }