diff --git a/mod/qbank/classes/task/transfer_question_categories.php b/mod/qbank/classes/task/transfer_question_categories.php index 25e9bd69c81..e80bd3ec379 100644 --- a/mod/qbank/classes/task/transfer_question_categories.php +++ b/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); - } } /** diff --git a/mod/qbank/tests/fixtures/testable_transfer_question_categories.php b/mod/qbank/tests/fixtures/testable_transfer_question_categories.php new file mode 100644 index 00000000000..8b576430ecc --- /dev/null +++ b/mod/qbank/tests/fixtures/testable_transfer_question_categories.php @@ -0,0 +1,50 @@ +. + +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 + * @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); + } +} diff --git a/mod/qbank/tests/task/transfer_question_categories_test.php b/mod/qbank/tests/task/transfer_question_categories_test.php index ba30745ade9..2aca9d6aec8 100644 --- a/mod/qbank/tests/task/transfer_question_categories_test.php +++ b/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); + } }