MDL-86154 questions: Delete unused questions after restore
After restoring an activity using shared questions without the question bank they belong to, we may have left over question categories in the target course context. These were being deleted, but any questions they contained weren't. This replaces the bulk delete of question category records with a call to `question_category_delete_safe()` for each category, which will also delete the questions they contain.
This commit is contained in:
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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]));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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]));
|
||||
}
|
||||
}
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user