diff --git a/course/format/classes/sectiondelegate.php b/course/format/classes/sectiondelegate.php index d5ab54608a1..33887ac4729 100644 --- a/course/format/classes/sectiondelegate.php +++ b/course/format/classes/sectiondelegate.php @@ -59,7 +59,11 @@ abstract class sectiondelegate { if ($classname === null) { return null; } - return new $classname($sectioninfo); + $instance = new $classname($sectioninfo); + if (!$instance->is_enabled()) { + return null; + } + return $instance; } /** @@ -84,6 +88,16 @@ abstract class sectiondelegate { return self::get_delegate_class_name($pluginname) !== null; } + /** + * Check if the delegate is enabled. + * + * Usually this happens when the delegate plugin is disabled. + * @return bool + */ + public function is_enabled(): bool { + return true; + } + /** * Define the section final name. * diff --git a/course/format/classes/sectiondelegatemodule.php b/course/format/classes/sectiondelegatemodule.php index 4834d17ac73..18bae324e42 100644 --- a/course/format/classes/sectiondelegatemodule.php +++ b/course/format/classes/sectiondelegatemodule.php @@ -51,11 +51,26 @@ abstract class sectiondelegatemodule extends sectiondelegate { ) { parent::__construct($sectioninfo); - [$this->course, $this->cm] = get_course_and_cm_from_instance( - $this->sectioninfo->itemid, - $this->get_module_name(), - $this->sectioninfo->course, - ); + try { + // Disabled or missing plugins can throw exceptions. + [$this->course, $this->cm] = get_course_and_cm_from_instance( + $this->sectioninfo->itemid, + $this->get_module_name(), + $this->sectioninfo->course, + ); + } catch (\Exception $e) { + $this->cm = null; + $this->course = null; + } + } + + /** + * Check if the delegated component is enabled. + * + * @return bool + */ + public function is_enabled(): bool { + return $this->cm !== null; } /** diff --git a/course/format/tests/sectiondelegate_test.php b/course/format/tests/sectiondelegate_test.php index 36c32bc266f..e8d8d52be6b 100644 --- a/course/format/tests/sectiondelegate_test.php +++ b/course/format/tests/sectiondelegate_test.php @@ -76,6 +76,33 @@ class sectiondelegate_test extends \advanced_testcase { $this->assertNull(sectiondelegate::instance($sectioninfos[3])); } + /** + * Test that the instance method returns null when the delegate class is disabled. + * + * @covers ::instance + */ + public function test_instance_disabled(): void { + global $DB; + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(['format' => 'topics', 'numsections' => 3]); + + // Section 2 has an existing delegate class. + course_update_section( + $course, + $DB->get_record('course_sections', ['course' => $course->id, 'section' => 2]), + [ + 'component' => 'test_component', + 'itemid' => testsectiondelegate::DISABLEDITEMID, + ] + ); + + $modinfo = get_fast_modinfo($course->id); + $sectioninfos = $modinfo->get_section_info_all(); + + $this->assertNull(sectiondelegate::instance($sectioninfos[2])); + } + /** * Test has_delegate_class(). * diff --git a/lib/navigationlib.php b/lib/navigationlib.php index 829efae2a20..8c4991d0714 100644 --- a/lib/navigationlib.php +++ b/lib/navigationlib.php @@ -2298,11 +2298,9 @@ class global_navigation extends navigation_node { $parentnode = $coursenode; // Set the parent node to the parent section if this is a delegated section. - if ($section->is_delegated()) { - $parentsection = $section->get_component_instance()->get_parent_section(); - if ($parentsection) { - $parentnode = $coursenode->find($parentsection->id, self::TYPE_SECTION) ?: $coursenode; - } + $parentsection = $section->get_component_instance()?->get_parent_section(); + if ($parentsection) { + $parentnode = $coursenode->find($parentsection->id, self::TYPE_SECTION) ?: $coursenode; } $sectionname = get_section_name($course, $section); diff --git a/lib/tests/fixtures/sectiondelegatetest.php b/lib/tests/fixtures/sectiondelegatetest.php index 1843bae90d2..5faedd54b1b 100644 --- a/lib/tests/fixtures/sectiondelegatetest.php +++ b/lib/tests/fixtures/sectiondelegatetest.php @@ -42,6 +42,9 @@ class sectiondelegate extends sectiondelegatebase { /** @var string force a null action menu. */ public const MENUNULL = 'null'; + /** @var int The itemid to use to simulate disabled component. */ + public const DISABLEDITEMID = 999; + /** * @var string|null Status to define which action menu to return when calling get_section_action_menu(). * Alternatively, different testing classes could be created, but it wasn't worth it for this case. @@ -114,4 +117,18 @@ class sectiondelegate extends sectiondelegatebase { return null; } } + + /** + * Check if the delegate is enabled. + * + * To simulate a disabled component, the itemid is set to DISABLEDITEMID. + * + * @return bool + */ + public function is_enabled(): bool { + if ($this->sectioninfo->itemid === self::DISABLEDITEMID) { + return false; + } + return true; + } } diff --git a/mod/subsection/classes/manager.php b/mod/subsection/classes/manager.php index 612d6e53e7a..24c32953c00 100644 --- a/mod/subsection/classes/manager.php +++ b/mod/subsection/classes/manager.php @@ -186,17 +186,21 @@ class manager { /** * Get the delegated section info. * - * @return section_info the delegated section info + * @return section_info|null the delegated section info */ - public function get_delegated_section_info(): section_info { + public function get_delegated_section_info(): ?section_info { $delegatedsection = $this->cm->get_delegated_section_info(); if (!$delegatedsection) { // Some restorations can produce a situation where the section is not found. // In that case, we create a new one. $delegatedsection = formatactions::section($this->cm->course)->create_delegated( self::PLUGINNAME, - $this->cm->id, - (object) ['name' => $this->instance->name], + $this->cm->instance, + (object) [ + 'name' => $this->cm->name, + 'visible' => $this->cm->visible, + 'availability' => (!empty($this->cm->availability)) ? $this->cm->availability : null, + ], ); } return $delegatedsection; diff --git a/mod/subsection/tests/courseformat/sectiondelegatemodule_test.php b/mod/subsection/tests/courseformat/sectiondelegatemodule_test.php index 16c1a8aad67..96b7ebfd973 100644 --- a/mod/subsection/tests/courseformat/sectiondelegatemodule_test.php +++ b/mod/subsection/tests/courseformat/sectiondelegatemodule_test.php @@ -109,4 +109,40 @@ final class sectiondelegatemodule_test extends \advanced_testcase { $this->assertInstanceOf(stdClass::class, $delegatedsectioncourse); $this->assertEquals($course->id, $delegatedsectioncourse->id); } + + public function test_instance_plugin_disabled(): void { + $this->resetAfterTest(); + + $manager = \core_plugin_manager::resolve_plugininfo_class('mod'); + $manager::enable_plugin('subsection', 1); + + $course = $this->getDataGenerator()->create_course(['format' => 'topics', 'numsections' => 2]); + $module = $this->getDataGenerator()->create_module( + 'subsection', + (object) ['course' => $course->id, 'section' => 2] + ); + + // Get the section info for the delegated section. + $sectioninfo = get_fast_modinfo($course)->get_section_info_by_component('mod_subsection', $module->id); + + /** @var testsectiondelegatemodule $delegated */ + $delegated = sectiondelegate::instance($sectioninfo); + $this->assertTrue($delegated->is_enabled()); + + // Disabling the plugin should disable the delegate. + $manager::enable_plugin('subsection', 0); + rebuild_course_cache($course->id, true); + + $sectioninfo = get_fast_modinfo($course)->get_section_info_by_component('mod_subsection', $module->id); + + /** @var testsectiondelegatemodule $delegated */ + $delegated = sectiondelegate::instance($sectioninfo); + // Delegated from a disabled plugin are considered orphaned, not delegated. + $this->assertNull($delegated); + + // Section delegate should not be created directly but we do it + // here to validate the is_enabled() method neverthless. + $delegatedinstance = new sectiondelegate($sectioninfo); + $this->assertFalse($delegatedinstance->is_enabled()); + } } diff --git a/mod/subsection/tests/manager_test.php b/mod/subsection/tests/manager_test.php new file mode 100644 index 00000000000..20e1ecd0f95 --- /dev/null +++ b/mod/subsection/tests/manager_test.php @@ -0,0 +1,131 @@ +. + +namespace mod_subsection; + +use availability_date\condition; +use core_availability\tree; +use core_courseformat\formatactions; + +/** + * Tests for Subsection manager class. + * + * @covers \mod_subsection\manager + * @package mod_subsection + * @category test + * @copyright 2024 Ferran Recio + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +final class manager_test extends \advanced_testcase { + /** + * Test get_delegated_section_info. + * + * @covers ::get_delegated_section_info + * @dataProvider provider_test_get_delegated_section_info + * @param bool $hasavailability Whether the module has access restrictions. + * @param bool $visible Whether the module is visible. + * @return void + */ + public function test_get_delegated_section_info( + bool $hasavailability, + bool $visible + ): void { + global $DB; + + $this->resetAfterTest(); + + $pluginmanager = \core_plugin_manager::resolve_plugininfo_class('mod'); + $pluginmanager::enable_plugin('subsection', 1); + + // Set up the availability settings. + $availabilityjson = null; + if ($hasavailability) { + $operation = condition::DIRECTION_FROM; + $availabilityjson = json_encode(tree::get_root_json( + [ + condition::get_json($operation, time() + 3600), + ], + '&', + true + )); + } + + $course = $this->getDataGenerator()->create_course(['format' => 'topics', 'numsections' => 2]); + $module = $this->getDataGenerator()->create_module( + 'subsection', + (object)['course' => $course->id, 'section' => 2], + ['visible' => $visible, 'availability' => $availabilityjson] + ); + + $cm = get_coursemodule_from_id('subsection', $module->cmid, 0, false, MUST_EXIST); + $manager = manager::create_from_coursemodule($cm); + $sectioninfo = $manager->get_delegated_section_info(); + $this->assertInstanceOf(\section_info::class, $sectioninfo); + $this->assertEquals($cm->instance, $sectioninfo->itemid); + $this->assertEquals($cm->name, $sectioninfo->name); + $this->assertEquals($cm->visible, $sectioninfo->visible); + $this->assertEquals($cm->availability, $sectioninfo->availability); + $initialid = $sectioninfo->id; + + // When subsections are disabled, all subsections are considered orphaned + // and can be removed without affecting the course_module. This should regenerate + // the delegated section once the module is re-enabled. + $pluginmanager::enable_plugin('subsection', 0); + formatactions::section($course)->delete($sectioninfo); + rebuild_course_cache($course->id, true); + $pluginmanager::enable_plugin('subsection', 1); + rebuild_course_cache($course->id, true); + + $cm = get_coursemodule_from_id('subsection', $module->cmid, 0, false, MUST_EXIST); + $manager = manager::create_from_coursemodule($cm); + $sectioninfo = $manager->get_delegated_section_info(); + $this->assertInstanceOf(\section_info::class, $sectioninfo); + $this->assertEquals($cm->instance, $sectioninfo->itemid); + $this->assertEquals($cm->name, $sectioninfo->name); + $this->assertEquals($cm->visible, $sectioninfo->visible); + $this->assertEquals($cm->availability, $sectioninfo->availability); + + // The section should be different from the previous one. + $this->assertNotEquals($initialid, $sectioninfo->id); + } + + /** + * Data provider for test_get_delegated_section_info. + * + * @return array + */ + public static function provider_test_get_delegated_section_info(): array { + return [ + 'Module is visible with no restrictions' => [ + 'hasavailability' => false, + 'visible' => true, + ], + 'Module is visible with restrictions' => [ + 'hasavailability' => true, + 'visible' => true, + ], + 'Module is hidden with no restrictions' => [ + 'hasavailability' => false, + 'visible' => false, + ], + 'Module is hidden with restrictions' => [ + 'hasavailability' => true, + 'visible' => false, + ], + + ]; + } +}