Merge branch 'MDL-86259-M500_transfer_question_categories-task-resilience' of https://github.com/ziegenberg/moodle into MOODLE_500_STABLE

mod/qbank/classes/task/transfer_question_categories.php

@@ -66,8 +66,6 @@ class transfer_question_categories extends adhoc_task {
 
         $recordset = $DB->get_recordset('question_categories', ['parent' => 0]);
 
-        $movedcategorycontexts = [];
-
         foreach ($recordset as $oldtopcategory) {
 
             if (!$oldcontext = context::instance_by_id($oldtopcategory->contextid, IGNORE_MISSING)) {
@@ -137,7 +135,14 @@ class transfer_question_categories extends adhoc_task {
             // We have our new mod instance, now move all the subcategories of the old 'top' category to this new context.
             $movedcategories = $this->move_question_category($oldtopcategory, $newmod->context);
 
-            $movedcategorycontexts += array_fill_keys($movedcategories, $oldtopcategory->contextid);
+            // Create a set of new tasks to update the questions in each category to the new contexts.
+            // The category itself is already in the new context. We record the old context
+            // so we know where to move files and tags from.
+            foreach ($movedcategories as $categoryid) {
+                $task = new transfer_questions();
+                $task->set_custom_data(['categoryid' => $categoryid, 'contextid' => $oldtopcategory->contextid]);
+                manager::queue_adhoc_task($task);
+            }
 
             // Job done, lets delete the old 'top' category.
             $DB->delete_records('question_categories', ['id' => $oldtopcategory->id]);
@@ -145,15 +150,6 @@ class transfer_question_categories extends adhoc_task {
         }
 
         $recordset->close();
-
-        // Create a set of new tasks to update the questions in each category to the new contexts.
-        // The category itself is already in the new context. We record the old context so we know where to move
-        // files and tags from.
-        foreach ($movedcategorycontexts as $categoryid => $oldcontextid) {
-            $task = new transfer_questions();
-            $task->set_custom_data(['categoryid' => $categoryid, 'contextid' => $oldcontextid]);
-            manager::queue_adhoc_task($task);
-        }
     }
 
     /**

mod/qbank/tests/fixtures/testable_transfer_question_categories.php

@@ -0,0 +1,50 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+namespace mod_qbank\task;
+
+use core\context\module;
+use core\exception\moodle_exception;
+
+/**
+ * Testable version of the transfer_question_categories class.
+ *
+ * @package    mod_qbank
+ * @copyright  2024 onwards Catalyst IT EU {@link https://catalyst-eu.net}
+ * @author     Simon Adams <[email protected]>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class testable_transfer_question_categories extends transfer_question_categories {
+    /** @var int tracks the number of test transfers. */
+    private int $testcounter = 0;
+
+    /**
+     * Summary of move_question_category
+     * @param \stdClass $oldtopcategory
+     * @param module $newcontext
+     * @return void
+     */
+    #[\Override]
+    protected function move_question_category(\stdClass $oldtopcategory, module $newcontext): array {
+        if ($this->testcounter >= 1) {
+            // We simulate a failure after successfully transferring two question categories
+            // and creating two corresponding transfer_questions tasks.
+            throw new moodle_exception('This is a mocked exception for testing purposes.');
+        }
+        $this->testcounter++;
+        return parent::move_question_category($oldtopcategory, $newcontext);
+    }
+}

mod/qbank/tests/task/transfer_question_categories_test.php

@@ -21,6 +21,7 @@ use context_course;
 use context_coursecat;
 use context_module;
 use context_system;
+use core\exception\moodle_exception;
 use core\task\manager;
 use core_question\local\bank\random_question_loader;
 use core_question\local\bank\question_bank_helper;
@@ -961,4 +962,23 @@ final class transfer_question_categories_test extends \advanced_testcase {
 
         $this->assertTrue(question_bank_helper::has_bank_migration_task_completed_successfully());
     }
+
+    public function test_qbank_install_resilience(): void {
+        global $DB;
+        $this->resetAfterTest();
+        $this->setup_pre_install_data();
+
+        require_once(__DIR__ . '/../fixtures/testable_transfer_question_categories.php');
+        $task = new testable_transfer_question_categories();
+        try {
+            $task->execute();
+        } catch (moodle_exception $e) {
+            // We expect a failure here, but we ignore this.
+            $this->assertStringContainsString('This is a mocked exception for testing purposes.', $e->getMessage());
+        }
+        // We want to verify a failure does not prevent the creation of tasks with hitherto transferred categories and their data.
+        // We should have a transfer_questions task for two of the categories that were moved.
+        $questiontasks = manager::get_adhoc_tasks(transfer_questions::class);
+        $this->assertCount(2, $questiontasks);
+    }
 }