diff --git a/public/lib/db/upgrade.php b/public/lib/db/upgrade.php index 9d7ae1af279..e1e365a7b84 100644 --- a/public/lib/db/upgrade.php +++ b/public/lib/db/upgrade.php @@ -2385,5 +2385,11 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2025100601.04); } + if ($oldversion < 2025100601.06) { + \core_question\category_manager::fix_restored_category_parents(); + // Main savepoint reached. + upgrade_main_savepoint(true, 2025100601.06); + } + return true; } diff --git a/public/question/classes/category_manager.php b/public/question/classes/category_manager.php index 1138e69cb12..10fe2368c69 100644 --- a/public/question/classes/category_manager.php +++ b/public/question/classes/category_manager.php @@ -377,4 +377,30 @@ class category_manager { $lastmax = $DB->get_field_sql($sql, ['parent' => $parentid]); return $lastmax ?? 0; } + + /** + * Upgrade step to find question categories with the wrong parent category. + * + * This will find question categories that have a parent in a different context, and set the parent to the top category + * of the current context. GROUP BY is to ensure we only get one result for each category, just in case we somehow end up + * with multiple top-level categories in a context. + * + * This could occur before the fix for MDL-86300, where a course restore left question categories that were the child of a top + * category with the original top category as the parent, rather than the new top category. + * + * This only needs to one once on upgrade, so is deprecated for removal in 6.0. + */ + public static function fix_restored_category_parents(): void { + global $DB; + $categoriestofix = $DB->get_records_sql(" + SELECT qc.id as id, MIN(qc3.id) AS parent + FROM {question_categories} qc + JOIN {question_categories} qc2 ON qc2.id = qc.parent AND qc.contextid != qc2.contextid + JOIN {question_categories} qc3 ON qc3.contextid = qc.contextid AND qc3.parent = 0 + GROUP BY qc.id + "); + foreach ($categoriestofix as $categorytofix) { + $DB->update_record('question_categories', $categorytofix, true); + } + } } diff --git a/public/question/tests/category_manager_test.php b/public/question/tests/category_manager_test.php index 4d33ddcb183..a8354ccdb82 100644 --- a/public/question/tests/category_manager_test.php +++ b/public/question/tests/category_manager_test.php @@ -605,4 +605,63 @@ final class category_manager_test extends \advanced_testcase { $questiongenerator->create_question_category(['contextid' => $context->id, 'parent' => $qcategory2->id]); $this->assertEquals(1, $manager->get_max_sortorder($qcategory2->id)); } + + /** + * Check that question categories with the wrong parent are fixed. + */ + public function test_fix_restored_category_parents(): void { + global $CFG, $DB, $USER; + require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php'); + + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course. + $generator = $this->getDataGenerator(); + $course = $generator->create_course(); + + // Add 2 quizzes with question categories. + $quiz1 = $generator->create_module('quiz', ['course' => $course->id]); + $quiz1context = \context_module::instance($quiz1->cmid); + $quiz1top = question_get_top_category($quiz1context->id); + $quiz1questioncats = $DB->get_records('question_categories', ['contextid' => $quiz1context->id]); + $this->assertCount(2, $quiz1questioncats); + + $quiz2 = $generator->create_module('quiz', ['course' => $course->id]); + $quiz2context = \context_module::instance($quiz2->cmid); + $quiz2top = question_get_top_category($quiz2context->id); + $quiz2questioncats = $DB->get_records('question_categories', ['contextid' => $quiz2context->id]); + $this->assertCount(2, $quiz2questioncats); + + // Add 2 question banks with question categories. + $qbank1 = $this->getDataGenerator()->create_module('qbank', ['course' => $course->id]); + $qbank1context = \context_module::instance($qbank1->cmid); + $qbank1top = question_get_top_category($qbank1context->id); + $qbank1questioncats = $DB->get_records('question_categories', ['contextid' => $qbank1context->id]); + $this->assertCount(2, $qbank1questioncats); + + $qbank2 = $this->getDataGenerator()->create_module('qbank', ['course' => $course->id]); + $qbank2context = \context_module::instance($qbank2->cmid); + $qbank2top = question_get_top_category($qbank2context->id); + $qbank2questioncats = $DB->get_records('question_categories', ['contextid' => $qbank2context->id]); + $this->assertCount(2, $qbank2questioncats); + + // Modify the child question categories of the second quiz and qbank instances so they are children of the + // first instances' top categories. + $quiz2nontop = question_get_default_category($quiz2context->id); + $DB->set_field('question_categories', 'parent', $quiz1top->id, ['id' => $quiz2nontop->id]); + + $qbank2nontop = question_get_default_category($qbank2context->id); + $DB->set_field('question_categories', 'parent', $qbank1top->id, ['id' => $qbank2nontop->id]); + + $this->assertEquals($quiz1top->id, $DB->get_field('question_categories', 'parent', ['id' => $quiz2nontop->id])); + $this->assertEquals($qbank1top->id, $DB->get_field('question_categories', 'parent', ['id' => $qbank2nontop->id])); + + // Run the fix. + category_manager::fix_restored_category_parents(); + + // Check that the child categories now have the correct parents. + $this->assertEquals($quiz2top->id, $DB->get_field('question_categories', 'parent', ['id' => $quiz2nontop->id])); + $this->assertEquals($qbank2top->id, $DB->get_field('question_categories', 'parent', ['id' => $qbank2nontop->id])); + } } diff --git a/public/version.php b/public/version.php index 4b2f12e19f5..08428584489 100644 --- a/public/version.php +++ b/public/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2025100601.05; // 20251006 = branching date YYYYMMDD - do not modify! +$version = 2025100601.06; // 20251006 = branching date YYYYMMDD - do not modify! // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '5.1.1+ (Build: 20260109)'; // Human-friendly version name