L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'MDL-84305-main' of https://github.com/timhunt/moodle

Browse Source
This commit is contained in:
Huong Nguyen
2025-03-18 11:16:23 +07:00
parent 4af8f12476 0a34c6aa10
commit 1b07e67010
2 changed files with 212 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mod/qbank/classes/task/transfer_question_categories.php
+107 -11
View File
@@ -19,13 +19,11 @@ namespace mod_qbank\task;
use context_system;
use core\context;
use core\task\adhoc_task;
use core\task\manager;
use core_course_category;
use core_question\local\bank\question_bank_helper;
use stdClass;
/**
* /**
* This script transfers question categories at CONTEXT_SITE, CONTEXT_COURSE, & CONTEXT_COURSECAT to a new qbank instance
* context.
*
@@ -49,17 +47,22 @@ use stdClass;
class transfer_question_categories extends adhoc_task {
/**
* Run the install task.
*
* @return void
* @var array a cache [ context id => question category ] of the top category in each context.
* Used by get_top_category_id_for_context() to avoid repeated DB queries.
* 0 is cached if this context id has no corresponding top category.
*/
public function execute() {
private array $topcategorycache = [];
#[\Override]
public function execute(): void {
global $DB, $CFG;
require_once($CFG->dirroot . '/course/modlib.php');
require_once($CFG->libdir . '/questionlib.php');
$this->fix_wrong_parents();
$recordset = $DB->get_recordset('question_categories', ['parent' => 0]);
foreach ($recordset as $oldtopcategory) {
@@ -79,7 +82,7 @@ class transfer_question_categories extends adhoc_task {
);
// This gives us categories in parent -> child order so array_reverse it,
// because we should process stale categories from the bottom up.
$subcategories = array_reverse(\sort_categories_by_tree($subcategories, $oldtopcategory->id));
$subcategories = array_reverse(sort_categories_by_tree($subcategories, $oldtopcategory->id));
foreach ($subcategories as $subcategory) {
\qbank_managecategories\helper::question_remove_stale_questions_from_category($subcategory->id);
if ($this->question_category_is_empty($subcategory->id)) {
@@ -109,7 +112,7 @@ class transfer_question_categories extends adhoc_task {
break;
case CONTEXT_COURSECAT:
$coursecategory = core_course_category::get($oldcontext->instanceid);
$courseshortname = "{$coursecategory->name}-{$coursecategory->id}";
$courseshortname = "$coursecategory->name-$coursecategory->id";
$course = $this->create_course($coursecategory, $courseshortname);
$bankname = question_bank_helper::get_bank_name_string('sharedbank', 'mod_qbank', $coursecategory->name);
break;
@@ -146,14 +149,14 @@ class transfer_question_categories extends adhoc_task {
* @param string $shortname
* @return stdClass
*/
protected function create_course(\core_course_category $coursecategory, string $shortname): stdClass {
protected function create_course(core_course_category $coursecategory, string $shortname): stdClass {
$data = (object) [
'enablecompletion' => 0,
'fullname' => get_string('coursecategory', 'mod_qbank', $coursecategory->name),
'shortname' => $shortname,
'category' => $coursecategory->id,
];
return \create_course($data);
return create_course($data);
}
/**
@@ -175,12 +178,105 @@ class transfer_question_categories extends adhoc_task {
$DB->set_field('question_categories', 'parent', $newtopcategory->id, ['parent' => $oldtopcategory->id]);
}
/**
* Find the Top category for a context, if there is one.
*
* @param int $contextid the id of a context (which might not exist).
* @return int a Top category id, or 0 if none is found.
*/
protected function get_top_category_id_for_context(int $contextid): int {
global $DB;
// Use the cache if we have already loaded this.
if (array_key_exists($contextid, $this->topcategorycache)) {
return $this->topcategorycache[$contextid];
}
$topcategoryid = (int) $DB->get_field('question_categories', 'id',
['contextid' => $contextid, 'parent' => 0]);
$this->topcategorycache[$contextid] = $topcategoryid;
return $topcategoryid;
}
/**
* Fix the context of child categories whose contextid does not match that of their parents.
*
* Fix here means:
*
* - if the child category's context exists, and has a 'Top' category, we move the child
* category to be just under that Top category. That is where they would have appeared
* before, e.g. in the return from question_categorylist().
*
* - if the child category points to a context that does not exist at all, then we
* instead change its context to be the same as it's parent's context. This may
* break things like images in the question text of questions there, but there is
* no real alternative.
*
* This is necessary because, due to old bugs, for example in backup and restoree code,
* we know there can be question categories in the databases of old Moodle sites with
* the wrong context id.
*/
public function fix_wrong_parents(): void {
global $DB;
$categoriestofix = $this->get_categories_in_a_different_context_to_their_parent();
foreach ($categoriestofix as $childcategoryid => $childcontextid) {
$topcategoryid = $this->get_top_category_id_for_context($childcontextid);
if ($topcategoryid) {
// Suitable Top category in the child's current context, so move to be a parent of that.
$DB->set_field('question_categories', 'parent', $topcategoryid, ['id' => $childcategoryid]);
} else {
// Top not found. Change the child to have the same context as its parent.
// This is not efficient in DB queries, but we expect this to be a rare case, and this is simple and right.
$childcategory = $DB->get_record('question_categories', ['id' => $childcategoryid]);
$parentcontextid = $DB->get_field('question_categories', 'contextid', ['id' => $childcategory->parent]);
$this->move_category_and_its_children($childcategoryid, $parentcontextid);
}
}
}
/**
* Get question categories that are in a different context to their parent.
*
* @return int[] child category id => context id of the child category.
*/
public function get_categories_in_a_different_context_to_their_parent(): array {
global $DB;
return $DB->get_records_sql_menu('
SELECT c.id, c.contextid
FROM {question_categories} c
JOIN {question_categories} p ON p.id = c.parent
WHERE p.contextid <> c.contextid
ORDER BY c.id
');
}
/**
* Set the contextid of category $categoryid and all its children to $newcontextid.
*
* @param int $categoryid a question_category id.
* @param int $newcontextid the place to move to.
*/
public function move_category_and_its_children(int $categoryid, int $newcontextid): void {
global $DB;
$DB->set_field('question_categories', 'contextid', $newcontextid, ['id' => $categoryid]);
$children = $DB->get_records('question_categories', ['parent' => $categoryid], '', 'id, contextid');
foreach ($children as $child) {
if ($child->contextid != $newcontextid) {
$this->move_category_and_its_children($child->id, $newcontextid);
}
}
}
/**
* Recursively check if a question category or its children contain any questions.
*
* @param int $categoryid The parent category to check from.
* @return bool True if neither the category nor its children contain any questions.
* @throws \dml_exception
*/
protected function question_category_is_empty(int $categoryid): bool {
global $DB;
mod/qbank/tests/transfer_question_categories_test.php → mod/qbank/tests/task/transfer_question_categories_test.php
+105 -11
View File
@@ -14,8 +14,9 @@
// 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;
namespace mod_qbank\task;
use context;
use context_course;
use context_coursecat;
use context_module;
@@ -371,7 +372,7 @@ final class transfer_question_categories_test extends \advanced_testcase {
$this->resetAfterTest();
$this->setup_pre_install_data();
$task = new \mod_qbank\task\transfer_question_categories();
$task = new transfer_question_categories();
$task->execute();
// Site context checks.
@@ -421,10 +422,10 @@ final class transfer_question_categories_test extends \advanced_testcase {
$coursecat = $DB->get_record('course_categories', ['id' => $newcourse->category]);
// Make sure the new course shortname is a unique name based on the category name and id.
$this->assertEquals("{$coursecat->name}-{$coursecat->id}", $newcourse->shortname);
$this->assertEquals("$coursecat->name-$coursecat->id", $newcourse->shortname);
// Make sure the new course fullname is based on the category name.
$this->assertEquals("Shared teaching resources for category: {$coursecat->name}", $newcourse->fullname);
$this->assertEquals("Shared teaching resources for category: $coursecat->name", $newcourse->fullname);
$coursemodinfo = get_fast_modinfo($newcourse);
$coursecatqbanks = $coursemodinfo->get_instances_of('qbank');
@@ -434,7 +435,7 @@ final class transfer_question_categories_test extends \advanced_testcase {
$coursecatqbank = reset($coursecatqbanks);
// Make sure the new module name is what we expect.
$this->assertEquals("{$coursecat->name} shared question bank", $coursecatqbank->name);
$this->assertEquals("$coursecat->name shared question bank", $coursecatqbank->name);
$coursecatqcats = $DB->get_records('question_categories', ['contextid' => $coursecatqbank->context->id], 'parent ASC');
@@ -460,7 +461,7 @@ final class transfer_question_categories_test extends \advanced_testcase {
// The module name should be what we expect.
$courseqbank = reset($courseqbanks);
$this->assertEquals("{$course->shortname} shared question bank", $courseqbank->name);
$this->assertEquals("$course->shortname shared question bank", $courseqbank->name);
// Make sure the question categories still exist and that we have a new top one at the new module context.
$topcat = question_get_top_category($courseqbank->context->id);
@@ -507,7 +508,7 @@ final class transfer_question_categories_test extends \advanced_testcase {
$usedunusedcourse = $usedunusedmodinfo->get_course();
$usedunusedqbanks = $usedunusedmodinfo->get_instances_of('qbank');
$usedunusedqbank = reset($usedunusedqbanks);
$this->assertEquals("{$usedunusedcourse->shortname} shared question bank", $usedunusedqbank->name);
$this->assertEquals("$usedunusedcourse->shortname shared question bank", $usedunusedqbank->name);
// We should now only have 3 categories. Top, used and unused.
$usedunusedcats = $DB->get_records(
@@ -516,13 +517,106 @@ final class transfer_question_categories_test extends \advanced_testcase {
fields: 'name, id',
);
$this->assertCount(3, $usedunusedcats);
$this->assertTrue(array_key_exists('top', $usedunusedcats));
$this->assertTrue(array_key_exists('Used Question Cat', $usedunusedcats));
$this->assertTrue(array_key_exists('Unused Question Cat', $usedunusedcats));
$this->assertFalse(array_key_exists('Empty Question Cat', $usedunusedcats));
$this->assertArrayHasKey('top', $usedunusedcats);
$this->assertArrayHasKey('Used Question Cat', $usedunusedcats);
$this->assertArrayHasKey('Unused Question Cat', $usedunusedcats);
$this->assertArrayNotHasKey('Empty Question Cat', $usedunusedcats);
$this->assertEmpty($this->get_question_data([$usedunusedcats['top']->id]));
$this->assertCount(2, $this->get_question_data([$usedunusedcats['Used Question Cat']->id]));
$this->assertCount(2, $this->get_question_data([$usedunusedcats['Unused Question Cat']->id]));
}
public function test_fix_wrong_parents(): void {
$this->resetAfterTest();
$this->setup_pre_install_data();
// Create a second course.
$course2 = self::getDataGenerator()->create_course();
$course2context = context_course::instance($course2->id);
// In course2 we build this category structure:
// - $course2parentcat -- context $course2context
// - - $wrongchild1 -- context $this->coursecontext (wrong)
// - - - $wronggrandchild1 -- context $this->coursecontext (same wrong)
// - - - $doublywronggrandchild1 -- context $course2context (back right, but not matching its parent)
// - - $wrongchild2 -- context non-existant A
// - - - $wronggrandchild2 -- context non-existent A
// - - - $doublywronggrandchild2 -- context non-existent B.
$course2parentcat = $this->create_question_category(
'Course2 parent cat', $course2context->id);
$wrongchild1 = $this->create_question_category(
'Child cat with wrong context', $this->coursecontext->id, $course2parentcat->id);
$wronggrandchild1 = $this->create_question_category(
'Grandchild of child1 in same wrong context', $this->coursecontext->id, $wrongchild1->id);
$doublywronggrandchild1 = $this->create_question_category(
'Grandchild of child1 back in the right context', $course2context->id, $wrongchild1->id);
$wrongchild2 = $this->create_question_category(
'Child cat with non-existent context', $course2context->id + 1000, $course2parentcat->id);
$wronggrandchild2 = $this->create_question_category(
'Grandchild of child2 with same non-existent context', $course2context->id + 1000, $wrongchild2->id);
$doublywronggrandchild2 = $this->create_question_category(
'Grandchild of child2 with different non-existent context', $course2context->id + 2000, $wrongchild2->id);
// Before we clean up, check that the expected categories are picked up.
// $wronggrandchild1 & $wronggrandchild2 are not seen, because their contexts match
// their parent's even though both are wrong. They should still get fixed.
$task = new transfer_question_categories();
$this->assertEquals(
[
$wrongchild1->id => $wrongchild1->contextid,
$doublywronggrandchild1->id => $course2context->id,
$wrongchild2->id => $wrongchild2->contextid,
$doublywronggrandchild2->id => $doublywronggrandchild2->contextid,
],
$task->get_categories_in_a_different_context_to_their_parent(),
);
// Call the cleanup method.
$task->fix_wrong_parents();
// Now we expect no mismatches.
$this->assertEmpty($task->get_categories_in_a_different_context_to_their_parent());
// Assert that the child categories have been moved to the locations they should have been.
$this->assert_category_is_in_context_with_parent($this->coursecontext, null, $wrongchild1->id);
$this->assert_category_is_in_context_with_parent($this->coursecontext, $wrongchild1, $wronggrandchild1->id);
$this->assert_category_is_in_context_with_parent($course2context, null, $doublywronggrandchild1->id);
$this->assert_category_is_in_context_with_parent($course2context, $course2parentcat, $wrongchild2->id);
$this->assert_category_is_in_context_with_parent($course2context, $wrongchild2, $wronggrandchild2->id);
$this->assert_category_is_in_context_with_parent($course2context, $wrongchild2, $doublywronggrandchild2->id);
}
/**
* Assert that the category with id $categoryid is in context $expectedcontext, with the given parent.
*
* @param context $expectedcontext the expected context for the category with id $categoryid.
* @param stdClass|null $expectedparent the expected parent category.
* null means the Top category in $expectedcontext.
* @param int $categoryid the id of the category to check.
*/
protected function assert_category_is_in_context_with_parent(
context $expectedcontext,
?stdClass $expectedparent,
int $categoryid,
): void {
global $DB;
if ($expectedparent === null) {
$expectedparent = $DB->get_record(
'question_categories',
['contextid' => $expectedcontext->id, 'parent' => 0],
'*',
MUST_EXIST,
);
}
$actualcategory = $DB->get_record('question_categories', ['id' => $categoryid]);
$this->assertEquals($expectedparent->id, $actualcategory->parent,
"Checking parent of category $actualcategory->name.");
$this->assertEquals($expectedcontext->id, $actualcategory->contextid,
"Checking context of category $actualcategory->name.");
}
}