Merge branch 'MDL-86154_500_STABLE' of https://github.com/marxjohnson/moodle into MOODLE_500_STABLE

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);
+        }
     }
 }
 

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;
 }

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);
+    }
 }

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]));
     }
 
     /**

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]));
+    }
 }

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