L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-87315 backup: Update set reference question category ID on restore.

Browse Source 
When a question bank is restored along with a quiz that uses a random question from that bank,
the set reference for the random question should now point to the category in the restored bank,
rather than original.

This uses the question bank context IDs recorded using the method added in MDL-86136
to decide if the set reference's question context is being restored, and update it
accordingly.

This required adding a new restore step in the restore_root_task, since when a restore is
performed asynchronously, the temp IDs table is deleted after the prechecks are performed
when the task is queued. Adding this step allows the IDs to be recorded again when the restore
runs.
This commit is contained in:
Mark Johnson
2026-02-27 13:44:30 +00:00
parent abfa0b8d72
commit 1890a0b94d
9 changed files with 201 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backup/moodle2/restore_root_task.class.php
+3
View File
@@ -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'));
backup/moodle2/restore_stepslib.php
+7 -1
View File
@@ -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);
}
}
lib/classes/backup/restore_load_questionbanks.php
+48
View File
@@ -0,0 +1,48 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
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 <[email protected]>
* @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);
}
}
mod/qbank/tests/backup/restore_test.php
+98
View File
@@ -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);
}
}
question/bank/managecategories/classes/category_condition.php
+2 -2
View File
@@ -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(
question/bank/managecategories/tests/category_condition_test.php
+7 -14
View File
@@ -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}");
}
}
question/bank/tagquestion/classes/tag_condition.php
+1
View File
@@ -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 = [];
question/bank/tagquestion/tests/tag_condition_test.php
+33
View File
@@ -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']);
}
}
question/classes/local/bank/condition.php
+2
View File
@@ -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;
}