From c629f23da85dc6728e3ff6e6dd4334375c95b8a8 Mon Sep 17 00:00:00 2001 From: Philipp Memmel Date: Tue, 18 Nov 2025 08:46:41 +0000 Subject: [PATCH] MDL-79221 core_grade: Fix fetching context for disabled mod plugins Activity plugins that are disabled on the site are not included in the modinfo cache. That's why their context must not be fetched via modinfo cache. --- public/lib/grade/grade_item.php | 17 +++++-- public/lib/grade/tests/fixtures/lib.php | 24 +++++++++ public/lib/grade/tests/grade_item_test.php | 59 +++++++++++++++++++++- 3 files changed, 94 insertions(+), 6 deletions(-) diff --git a/public/lib/grade/grade_item.php b/public/lib/grade/grade_item.php index cd7a70ce839..c5144f58697 100644 --- a/public/lib/grade/grade_item.php +++ b/public/lib/grade/grade_item.php @@ -2633,12 +2633,19 @@ class grade_item extends grade_object { rebuild_course_cache($this->courseid, true); $modinfo = get_fast_modinfo($this->courseid); } - // Even with a rebuilt cache the module does not exist. This means the - // database is in an invalid state - we will log an error and return - // the course context but the calling code should be updated. + + // Even with a rebuilt cache the module does not exist. This means we are dealing + // with a mod plugin type that is disabled on the site (which are not included in the + // modinfo cache) or the database is in an invalid state. In the latter case we will + // log an error and return the course context, but the calling code should be updated. if (!isset($modinfo->instances[$this->itemmodule][$this->iteminstance])) { - mtrace(get_string('moduleinstancedoesnotexist', 'error')); - $context = \context_course::instance($this->courseid); + if ($cm = get_coursemodule_from_instance($this->itemmodule, $this->iteminstance)) { + // Cache does not contain module plugins that are disabled. + $context = \context_module::instance($cm->id); + } else { + debugging(get_string('moduleinstancedoesnotexist', 'error')); + $context = \context_course::instance($this->courseid); + } } else { $cm = $modinfo->instances[$this->itemmodule][$this->iteminstance]; $context = \context_module::instance($cm->id); diff --git a/public/lib/grade/tests/fixtures/lib.php b/public/lib/grade/tests/fixtures/lib.php index 1aa0b0d2d6e..a33981bc407 100644 --- a/public/lib/grade/tests/fixtures/lib.php +++ b/public/lib/grade/tests/fixtures/lib.php @@ -109,6 +109,9 @@ abstract class grade_base_testcase extends advanced_testcase { $this->activities[7] = $this->getDataGenerator()->create_module('quiz', array('course'=>$this->course->id)); $this->course_module[7] = get_coursemodule_from_instance('quiz', $this->activities[7]->id); + + $this->activities[8] = $this->getDataGenerator()->create_module('forum', ['course' => $this->course->id]); + $this->course_module[8] = get_coursemodule_from_instance('forum', $this->activities[8]->id); } private function load_scales() { @@ -677,6 +680,27 @@ abstract class grade_base_testcase extends advanced_testcase { $this->grade_items[16] = $grade_item; // $this->grade_items[17] loaded in load_grade_outcomes() in order to use an outcome id. + + // Item with id = 18: Is being used to test what happens if the related mod plugin is disabled. + $grade_item = new stdClass(); + + $grade_item->courseid = $this->course->id; + $grade_item->categoryid = $this->grade_categories[3]->id; + $grade_item->itemname = 'singleparentitem1'; + $grade_item->itemtype = 'mod'; + $grade_item->itemmodule = $this->course_module[8]->modname; + $grade_item->iteminstance = $this->course_module[8]->instance; + $grade_item->gradetype = GRADE_TYPE_SCALE; + $grade_item->scaleid = $this->scale[0]->id; + $grade_item->grademin = 0; + $grade_item->grademax = $this->scalemax[0]; + $grade_item->iteminfo = 'Grade item 18 used for unit testing'; + $grade_item->timecreated = time(); + $grade_item->timemodified = time(); + $grade_item->sortorder = 18; + + $grade_item->id = $DB->insert_record('grade_items', $grade_item); + $this->grade_items[18] = $grade_item; } /** diff --git a/public/lib/grade/tests/grade_item_test.php b/public/lib/grade/tests/grade_item_test.php index 15048c4c26c..2d07a0c4bc5 100644 --- a/public/lib/grade/tests/grade_item_test.php +++ b/public/lib/grade/tests/grade_item_test.php @@ -33,6 +33,7 @@ final class grade_item_test extends \grade_base_testcase { $this->sub_test_grade_item_construct(); $this->sub_test_grade_item_insert(); $this->sub_test_grade_item_delete(); + $this->sub_test_grade_item_delete_disabled_modules(); $this->sub_test_grade_item_update(); $this->sub_test_grade_item_load_scale(); $this->sub_test_grade_item_load_outcome(); @@ -108,7 +109,7 @@ final class grade_item_test extends \grade_base_testcase { $last_grade_item = end($this->grade_items); $this->assertEquals($grade_item->id, $last_grade_item->id + 1); - $this->assertEquals(18, $grade_item->sortorder); + $this->assertEquals(19, $grade_item->sortorder); // Keep our reference collection the same as what is in the database. $this->grade_items[] = $grade_item; @@ -150,6 +151,62 @@ final class grade_item_test extends \grade_base_testcase { unset($this->grade_items[7]); } + /** + * Tests the deletion of a grade item of an activity instance that has been globally disabled. + * + * @covers \grade_item::delete + */ + protected function sub_test_grade_item_delete_disabled_modules(): void { + global $CFG, $DB; + require_once($CFG->dirroot . '/course/lib.php'); + $grade_item = new \grade_item($this->grade_items[18], false); // Use a grade item not touched by previous (or future) tests. + $this->assertTrue(method_exists($grade_item, 'delete')); + + // Add two files. + $dummy = [ + 'contextid' => $grade_item->get_context()->id, + 'component' => GRADE_FILE_COMPONENT, + 'filearea' => GRADE_HISTORY_FEEDBACK_FILEAREA, + 'itemid' => 1, + 'filepath' => '/', + 'filename' => 'feedback1.txt', + ]; + + $fs = get_file_storage(); + $fs->create_file_from_string($dummy, ''); + + $dummy['itemid'] = 2; + $fs->create_file_from_string($dummy, ''); + + $files = $fs->get_area_files($grade_item->get_context()->id, GRADE_FILE_COMPONENT, GRADE_HISTORY_FEEDBACK_FILEAREA); + // Includes directories. + $this->assertCount(4, $files); + + // Now disable the mod plugin. + $class = \core_plugin_manager::resolve_plugininfo_class('mod'); + $class::enable_plugin($this->course_module[8]->modname, false); + $this->assertFalse(in_array($this->course_module[8]->modname, \core\plugininfo\mod::get_enabled_plugins())); + rebuild_course_cache($this->course->id); + + ob_start(); + $this->assertTrue($grade_item->delete()); + $result = ob_get_contents(); + ob_end_clean(); + // No mtrace warning about missing module when fetching the context should appear. + $this->assertEmpty($result); + + $this->assertFalse($DB->get_record('grade_items', ['id' => $grade_item->id])); + + $files = $fs->get_area_files($grade_item->get_context()->id, GRADE_FILE_COMPONENT, GRADE_HISTORY_FEEDBACK_FILEAREA); + $this->assertEmpty($files); + + // Keep our reference collection the same as the database. + unset($this->grade_items[18]); + $class::enable_plugin($this->course_module[8]->modname, true); + $this->assertTrue(in_array($this->course_module[8]->modname, \core\plugininfo\mod::get_enabled_plugins())); + rebuild_course_cache($this->course->id); + } + protected function sub_test_grade_item_update() { global $DB; $grade_item = new \grade_item($this->grade_items[0], false);