diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index b9f7fe2b031..d1691ac3f46 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -5715,20 +5715,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/lib/db/upgrade.php b/lib/db/upgrade.php index afcb215654e..224aa2becac 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2008,5 +2008,14 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2025041406.01); } + if ($oldversion < 2025041406.05) { + $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, 2025041406.05); + } + return true; } diff --git a/question/classes/category_manager.php b/question/classes/category_manager.php index 10fe2368c69..ba2c118e458 100644 --- a/question/classes/category_manager.php +++ b/question/classes/category_manager.php @@ -16,6 +16,7 @@ namespace core_question; +use core\attribute\deprecated; use stdClass; use core\exception\moodle_exception; use core\context; @@ -403,4 +404,30 @@ 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. + */ + 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/question/tests/backup_test.php b/question/tests/backup_test.php index 6266385346e..add46dc0651 100644 --- a/question/tests/backup_test.php +++ b/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/question/tests/category_manager_test.php b/question/tests/category_manager_test.php index a8354ccdb82..18870f0cefc 100644 --- a/question/tests/category_manager_test.php +++ b/question/tests/category_manager_test.php @@ -664,4 +664,47 @@ 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. + */ + 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/version.php b/version.php index 730aa387c59..3b23f7a55a3 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2025041406.04; // 20250414 = branching date YYYYMMDD - do not modify! +$version = 2025041406.05; // 20250414 = branching date YYYYMMDD - do not modify! // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '5.0.6+ (Build: 20260227)'; // Human-friendly version name