diff --git a/public/backup/moodle2/restore_stepslib.php b/public/backup/moodle2/restore_stepslib.php index dde023e0303..49bb8ef8c7f 100644 --- a/public/backup/moodle2/restore_stepslib.php +++ b/public/backup/moodle2/restore_stepslib.php @@ -5723,20 +5723,20 @@ class restore_move_module_questions_categories extends restore_execution_step { ); } } - // Remove any remaining course-level question categories from the restored course. + // Remove any remaining course-level question categories and their questions from the restored course. $coursecatsql = " - SELECT qc.id AS categoryid + SELECT qc.id AS id, qc.contextid AS contextid FROM {question_categories} qc JOIN {context} c ON c.id = qc.contextid WHERE c.contextlevel = :courselevel AND c.instanceid = :courseid "; - $DB->delete_records_subquery( - 'question_categories', - 'id', - 'categoryid', + $categories = $DB->get_records_sql( $coursecatsql, - ['courselevel' => context_course::LEVEL, 'courseid' => $this->task->get_courseid()] + ['courselevel' => context_course::LEVEL, 'courseid' => $this->task->get_courseid()], ); + foreach ($categories as $category) { + question_category_delete_safe($category); + } } } diff --git a/public/lib/db/upgrade.php b/public/lib/db/upgrade.php index 1982286b31a..7b30caadcc5 100644 --- a/public/lib/db/upgrade.php +++ b/public/lib/db/upgrade.php @@ -1740,5 +1740,14 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2026021000.01); } + if ($oldversion < 2026022700.01) { + $orphanedquestions = core_question\category_manager::cleanup_questions_without_categories(); + if ($orphanedquestions > 0) { + upgrade_log(UPGRADE_LOG_NORMAL, null, "Cleaned up {$orphanedquestions} questions left over from restores."); + } + + upgrade_main_savepoint(true, 2026022700.01); + } + return true; } diff --git a/public/question/classes/category_manager.php b/public/question/classes/category_manager.php index 3dbb4696085..dadbcd14169 100644 --- a/public/question/classes/category_manager.php +++ b/public/question/classes/category_manager.php @@ -409,4 +409,31 @@ class category_manager { $DB->update_record('question_categories', $categorytofix, true); } } + + /** + * Upgrade step to find questions with no category and delete them. + * + * Due to MDL-86154, there may be questions left in the database after a restore, whose category has been deleted. This will + * find any questions like that and delete them. These questions will always be unused. + * + * Now that we have prevented this occurring, this function is used by the upgrade process to clean up these questions. + * + * @return int A count of deleted questions. + * @todo Deprecate in 6.0 MDL-87844 for Removal in 7.0 MDL-87845. + */ + public static function cleanup_questions_without_categories(): int { + global $DB; + $questionids = $DB->get_fieldset_sql(" + SELECT q.id + FROM {question_bank_entries} qbe + JOIN {question_versions} qv ON qv.questionbankentryid = qbe.id + JOIN {question} q ON qv.questionid = q.id + LEFT JOIN {question_categories} qc ON qbe.questioncategoryid = qc.id + WHERE qc.id IS NULL + "); + foreach ($questionids as $questionid) { + question_delete_question($questionid); + } + return count($questionids); + } } diff --git a/public/question/tests/backup_test.php b/public/question/tests/backup_test.php index 6266385346e..add46dc0651 100644 --- a/public/question/tests/backup_test.php +++ b/public/question/tests/backup_test.php @@ -716,6 +716,8 @@ final class backup_test extends \advanced_testcase { // Check we have the expected restored categories. $this->assertEquals(2, $DB->count_records('question_categories', ['stamp' => $data->quizcategory->stamp])); $this->assertEquals(1, $DB->count_records('question_categories', ['stamp' => $data->qbankcategory->stamp])); + // Check there is no additional copy of the referenced question bank question. + $this->assertEquals(1, $DB->count_records('question', ['name' => $data->qbankquestion->name])); } /** diff --git a/public/question/tests/category_manager_test.php b/public/question/tests/category_manager_test.php index 3fb830c9ae2..a1a3e19f478 100644 --- a/public/question/tests/category_manager_test.php +++ b/public/question/tests/category_manager_test.php @@ -666,4 +666,49 @@ final class category_manager_test extends \advanced_testcase { $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])); } + + /** + * A question with no category should be deleted, while other questions remain as-is. + * + * @todo Deprecate in 6.0 MDL-87844 for Removal in 7.0 MDL-87845. + */ + public function test_cleanup_questions_without_categories(): void { + global $DB; + $this->setAdminUser(); + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $quiz = $this->getDataGenerator()->create_module('quiz', ['course' => $course->id]); + $context = \context_module::instance($quiz->cmid); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $topcategory = question_get_top_category($context->id, true); + $defaultcategory = question_get_default_category($context->id); + $deletedcategory = $questiongenerator->create_question_category( + ['contextid' => $context->id, 'parent' => $topcategory->id], + ); + // Create 2 questions. One in the default category, and in the category being deleted. + $question = $questiongenerator->create_question('truefalse', overrides: ['category' => $defaultcategory->id]); + $orphan = $questiongenerator->create_question('truefalse', overrides: ['category' => $deletedcategory->id]); + + $DB->delete_records('question_categories', ['id' => $deletedcategory->id]); + + $this->assertEquals(1, category_manager::cleanup_questions_without_categories()); + + // The default category question is unchanged. + $this->assertTrue( + $DB->record_exists_sql( + "SELECT * + FROM {question} q + JOIN {question_versions} qv on qv.questionid = q.id + JOIN {question_bank_entries} qbe ON qv.questionbankentryid = qbe.id + WHERE q.id = :questionid AND qbe.questioncategoryid = :categoryid", + [ + 'questionid' => $question->id, + 'categoryid' => $defaultcategory->id, + ], + ), + ); + // The orphaned question has been deleted. + $this->assertFalse($DB->record_exists('question', ['id' => $orphan->id])); + } } diff --git a/public/version.php b/public/version.php index 827e40e7dcb..fa9caadc83c 100644 --- a/public/version.php +++ b/public/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2026022700.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2026022700.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '5.2dev (Build: 20260227)'; // Human-friendly version name