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

MDL-82541 core_course: fix orphaned sections logic

Browse Source
This commit is contained in:
ferranrecio
2024-09-12 16:07:42 +02:00
parent ebaf06848c
commit 9c55a02fb1
5 changed files with 235 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
course/format/classes/base.php
+4
View File
@@ -1592,6 +1592,10 @@ abstract class base {
* @return bool;
*/
public function is_section_visible(section_info $section): bool {
// It is unlikely that a section is orphan, but it needs to be checked.
if ($section->is_orphan() && !has_capability('moodle/course:viewhiddensections', $this->get_context())) {
return false;
}
// Previous to Moodle 4.0 thas logic was hardcoded. To prevent errors in the contrib plugins
// the default logic is the same required for topics and weeks format and still uses
// a "hiddensections" format setting.
course/format/tests/base_test.php
+43
View File
@@ -964,6 +964,38 @@ class base_test extends advanced_testcase {
$format = course_get_format($course);
$this->assertTrue($format->can_sections_be_removed_from_navigation());
}
public function test_is_section_visible(): void {
$this->resetAfterTest();
$generator = $this->getDataGenerator();
$course = $generator->create_course(['format' => 'testformatsections'], ['hiddensections' => 1]);
course_create_sections_if_missing($course, [0, 1, 2]);
// Students cannot view hidden sections.
$sectioninfo = get_fast_modinfo($course)->get_section_info(1);
\core_courseformat\formatactions::section($course)->update($sectioninfo, ['visible' => false]);
$format = course_get_format($course);
// Force max sections to 1 to detect section 2 as orphan.
$format->forcemaxsections = 1;
$teacher = $this->getDataGenerator()->create_and_enrol($course, 'editingteacher');
$student = $this->getDataGenerator()->create_and_enrol($course, 'student');
$this->setUser($teacher);
$modinfoteacher = get_fast_modinfo($course, $teacher->id);
$this->assertTrue($format->is_section_visible($modinfoteacher->get_section_info(0)));
$this->assertTrue($format->is_section_visible($modinfoteacher->get_section_info(1)));
$this->assertTrue($format->is_section_visible($modinfoteacher->get_section_info(2)));
$this->setUser($student);
$modinfostudent = get_fast_modinfo($course, $student->id);
$this->assertTrue($format->is_section_visible($modinfostudent->get_section_info(0)));
$this->assertFalse($format->is_section_visible($modinfostudent->get_section_info(1)));
$this->assertFalse($format->is_section_visible($modinfostudent->get_section_info(2)));
}
}
/**
@@ -999,6 +1031,10 @@ class format_testformat extends core_courseformat\base {
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class format_testformatsections extends core_courseformat\base {
/**
* @var int|null $forcemaxsections The maximum number of sections.
*/
public ?int $forcemaxsections = null;
/**
* Returns if this course format uses sections.
*
@@ -1011,6 +1047,13 @@ class format_testformatsections extends core_courseformat\base {
public function can_sections_be_removed_from_navigation(): bool {
return true;
}
public function get_last_section_number(): int {
if ($this->forcemaxsections !== null) {
return $this->forcemaxsections;
}
return parent::get_last_section_number();
}
}
/**
course/tests/modlib_test.php
+11 -7
View File
@@ -18,12 +18,6 @@ namespace core_course;
use core_courseformat\formatactions;
defined('MOODLE_INTERNAL') || die();
global $CFG;
require_once($CFG->dirroot . '/course/lib.php');
require_once($CFG->dirroot . '/course/modlib.php');
/**
* Module lib related unit tests
*
@@ -33,6 +27,16 @@ require_once($CFG->dirroot . '/course/modlib.php');
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class modlib_test extends \advanced_testcase {
/**
* Setup to ensure that fixtures are loaded.
*/
public static function setUpBeforeClass(): void {
global $CFG;
require_once($CFG->dirroot . '/course/lib.php');
require_once($CFG->dirroot . '/course/modlib.php');
require_once($CFG->libdir . '/tests/fixtures/sectiondelegatetest.php');
parent::setUpBeforeClass();
}
/**
* Test prepare_new_moduleinfo_data
@@ -397,7 +401,7 @@ class modlib_test extends \advanced_testcase {
['createsections' => true]
);
$section = formatactions::section($course)->create_delegated('mod_label', 0);
$section = formatactions::section($course)->create_delegated('test_component', 0);
$modinfo = get_fast_modinfo($course);
$this->assertCount(4, $modinfo->get_section_info_all());
lib/modinfolib.php
+45 -6
View File
@@ -347,7 +347,7 @@ class course_modinfo {
}
$sections = [];
foreach ($this->sectioninfobynum as $section) {
if (!$section->is_delegated()) {
if (!$section->get_component_instance()) {
$sections[$section->section] = $section;
}
}
@@ -3226,6 +3226,11 @@ class section_info implements IteratorAggregate {
*/
private ?sectiondelegate $_delegateinstance = null;
/**
* @var bool|null $_isorphan True if the section is orphan for some reason.
*/
private $_isorphan = null;
/**
* Availability conditions for this section based on the completion of
* course-modules (array from course-module id to required completion state
@@ -3564,12 +3569,19 @@ class section_info implements IteratorAggregate {
}
$this->_uservisible = true;
if (!$this->_visible || !$this->get_available()) {
if ($this->is_orphan() || !$this->_visible || !$this->get_available()) {
$coursecontext = context_course::instance($this->get_course());
if (!$this->_visible && !has_capability('moodle/course:viewhiddensections', $coursecontext, $userid) ||
(!$this->get_available() &&
!has_capability('moodle/course:ignoreavailabilityrestrictions', $coursecontext, $userid))) {
if (
($this->_isorphan || !$this->_visible)
&& !has_capability('moodle/course:viewhiddensections', $coursecontext, $userid)
) {
$this->_uservisible = false;
}
if (
$this->_uservisible
&& !$this->get_available()
&& !has_capability('moodle/course:ignoreavailabilityrestrictions', $coursecontext, $userid)
) {
$this->_uservisible = false;
}
}
@@ -3664,6 +3676,33 @@ class section_info implements IteratorAggregate {
return !empty($this->_component);
}
/**
* Returns true if this section is orphan.
*
* @return bool
*/
public function is_orphan(): bool {
if ($this->_isorphan !== null) {
return $this->_isorphan;
}
$courseformat = course_get_format($this->modinfo->get_course());
// There are some cases where a restored course using third-party formats can
// have orphaned sections due to a fixed section number.
if ($this->_sectionnum > $courseformat->get_last_section_number()) {
$this->_isorphan = true;
return $this->_isorphan;
}
// Some delegated sections can belong to a plugin that is disabled or not present.
if ($this->is_delegated() && !$this->get_component_instance()) {
$this->_isorphan = true;
return $this->_isorphan;
}
$this->_isorphan = false;
return $this->_isorphan;
}
/**
* Prepares section data for inclusion in sectioncache cache, removing items
* that are set to defaults, and adding availability data if required.
lib/tests/modinfolib_test.php
+132 -2
View File
@@ -1109,8 +1109,8 @@ class modinfolib_test extends advanced_testcase {
$this->assertCount(4, $listed);
// Generate some delegated sections (not listed).
formatactions::section($course)->create_delegated('mod_label', 0);
formatactions::section($course)->create_delegated('mod_label', 1);
formatactions::section($course)->create_delegated('test_component', 0);
formatactions::section($course)->create_delegated('test_component', 1);
$this->assertCount(6, get_fast_modinfo($course)->get_section_info_all());
@@ -1954,6 +1954,78 @@ class modinfolib_test extends advanced_testcase {
];
}
/**
* Test get_uservisible method when the section is delegated and depending on if the plugin is enabled.
*
* @covers \section_info::get_uservisible
* @dataProvider provider_test_get_uservisible_delegate_enabled
* @param string $role The role to assign to the user.
* @param bool $enabled Whether the plugin is enabled.
* @param bool $expected The expected visibility of the delegated section.
* @return void
*/
public function test_get_uservisible_delegate_enabled(
string $role,
bool $enabled,
bool $expected,
): void {
$this->resetAfterTest();
$manager = \core_plugin_manager::resolve_plugininfo_class('mod');
$manager::enable_plugin('subsection', 1);
$course = $this->getDataGenerator()->create_course(['numsections' => 1]);
$subsection = $this->getDataGenerator()->create_module('subsection', ['course' => $course], ['section' => 1]);
$modinfo = get_fast_modinfo($course);
$delegatedsection = $modinfo->get_cm($subsection->cmid)->get_delegated_section_info();
$user = $this->getDataGenerator()->create_and_enrol($course, $role);
if (!$enabled) {
$manager::enable_plugin('subsection', 0);
rebuild_course_cache($course->id, true);
}
$this->setUser($user);
$modinfo = get_fast_modinfo($course);
$delegatedsection = $modinfo->get_section_info($delegatedsection->section);
// The get_uservisible is a magic getter.
$this->assertEquals($expected, $delegatedsection->uservisible);
}
/**
* Data provider for test_get_uservisible_delegate_enabled.
*
* @return array
*/
public static function provider_test_get_uservisible_delegate_enabled(): array {
return [
'Student with plugin enabled' => [
'role' => 'student',
'enabled' => true,
'expected' => true,
],
'Student with plugin disabled' => [
'role' => 'student',
'enabled' => false,
'expected' => false,
],
'Teacher with plugin enabled' => [
'role' => 'editingteacher',
'enabled' => true,
'expected' => true,
],
'Teacher with plugin disabled' => [
'role' => 'editingteacher',
'enabled' => false,
'expected' => true,
],
];
}
/**
* Test get_available method when the section is delegated.
*
@@ -2093,4 +2165,62 @@ class modinfolib_test extends advanced_testcase {
],
];
}
/**
* Test when a section is considered orphan.
*
* @covers \section_info::is_orphan
* @return void
*/
public function test_is_orphan(): void {
global $DB;
$this->resetAfterTest();
$manager = \core_plugin_manager::resolve_plugininfo_class('mod');
$manager::enable_plugin('subsection', 1);
$course = $this->getDataGenerator()->create_course(['numsections' => 1]);
$subsection = $this->getDataGenerator()->create_module('subsection', ['course' => $course], ['section' => 1]);
$modinfo = get_fast_modinfo($course);
$delegatedsection = $modinfo->get_cm($subsection->cmid)->get_delegated_section_info();
// If mod_subsection is enabled, a subsection is not orphan.
$modinfo = get_fast_modinfo($course);
$this->assertFalse($delegatedsection->is_orphan());
// Delegated sections without a component instance (disabled mod_subsection) is considered orphan.
$manager::enable_plugin('subsection', 0);
rebuild_course_cache($course->id, true);
$modinfo = get_fast_modinfo($course);
$delegatedsection = $modinfo->get_section_info($delegatedsection->section);
$this->assertTrue($delegatedsection->is_orphan());
// Check enabling the plugin restore the previous state.
$manager::enable_plugin('subsection', 1);
rebuild_course_cache($course->id, true);
$modinfo = get_fast_modinfo($course);
$delegatedsection = $modinfo->get_section_info($delegatedsection->section);
$this->assertFalse($delegatedsection->is_orphan());
// Force section limit in the course format instance.
rebuild_course_cache($course->id, true);
$modinfo = get_fast_modinfo($course);
// Core formats does not use numsections anymore. We need to use reflection to change the value.
$format = course_get_format($course);
// Add a fake numsections format data (Force loading format data first).
$format->get_course();
$reflection = new \ReflectionObject($format);
$property = $reflection->getProperty('course');
$courseobject = $property->getValue($format);
$courseobject->numsections = 1;
$property->setValue($format, $courseobject);
$delegatedsection = $modinfo->get_section_info($delegatedsection->section);
$this->assertTrue($delegatedsection->is_orphan());
}
}