mirror of
https://github.com/moodle/moodle.git
synced 2025-01-17 21:49:15 +01:00
MDL-80190 core_courseformat: fix activity creation on delegated sections
The method for checking if the user can create an activity executed create_if_missing wihtout checking the section exists or not. This is a problem for delegated sections because create_if_missing will create a regular section and push down all delegate ones. With the patch the method first checks if the section exists.
This commit is contained in:
Ferran Recio
parent
97a7958aee
commit
15db7d9d98
@ -172,6 +172,10 @@ class sectionactions extends baseactions {
|
||||
|
||||
/**
|
||||
* Create course sections if they are not created yet.
|
||||
*
|
||||
* The calculations will ignore sections delegated to components.
|
||||
* If the section is created, all delegated sections will be pushed down.
|
||||
*
|
||||
* @param int[] $sectionnums the section numbers to create
|
||||
* @return bool whether any section was created
|
||||
*/
|
||||
|
||||
@ -28,6 +28,7 @@
|
||||
defined('MOODLE_INTERNAL') || die;
|
||||
|
||||
use \core_grades\component_gradeitems;
|
||||
use core_courseformat\formatactions;
|
||||
|
||||
require_once($CFG->dirroot.'/course/lib.php');
|
||||
|
||||
@ -497,27 +498,33 @@ function set_moduleinfo_defaults($moduleinfo) {
|
||||
* The fucntion create the course section if it doesn't exist.
|
||||
*
|
||||
* @param object $course the course of the module
|
||||
* @param object $modulename the module name
|
||||
* @param object $section the section of the module
|
||||
* @param string $modulename the module name
|
||||
* @param int $sectionnum the section of the module
|
||||
* @return array list containing module, context, course section.
|
||||
* @throws moodle_exception if user is not allowed to perform the action or module is not allowed in this course
|
||||
*/
|
||||
function can_add_moduleinfo($course, $modulename, $section) {
|
||||
function can_add_moduleinfo($course, $modulename, $sectionnum) {
|
||||
global $DB;
|
||||
|
||||
$module = $DB->get_record('modules', array('name'=>$modulename), '*', MUST_EXIST);
|
||||
$module = $DB->get_record('modules', ['name' => $modulename], '*', MUST_EXIST);
|
||||
|
||||
$context = context_course::instance($course->id);
|
||||
require_capability('moodle/course:manageactivities', $context);
|
||||
|
||||
course_create_sections_if_missing($course, $section);
|
||||
$cw = get_fast_modinfo($course)->get_section_info($section);
|
||||
// If the $sectionnum is a delegated section, we cannot execute create_if_missing
|
||||
// because it only works to create regular sections. To prevent that from happening, we
|
||||
// check if the section is already there, no matter if it is delegated or not.
|
||||
$sectioninfo = get_fast_modinfo($course)->get_section_info($sectionnum);
|
||||
if (!$sectioninfo) {
|
||||
formatactions::section($course)->create_if_missing([$sectionnum]);
|
||||
$sectioninfo = get_fast_modinfo($course)->get_section_info($sectionnum);
|
||||
}
|
||||
|
||||
if (!course_allowed_module($course, $module->name)) {
|
||||
throw new \moodle_exception('moduledisable');
|
||||
}
|
||||
|
||||
return array($module, $context, $cw);
|
||||
return [$module, $context, $sectioninfo];
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -16,6 +16,8 @@
|
||||
|
||||
namespace core_course;
|
||||
|
||||
use core_courseformat\formatactions;
|
||||
|
||||
defined('MOODLE_INTERNAL') || die();
|
||||
|
||||
global $CFG;
|
||||
@ -276,4 +278,188 @@ class modlib_test extends \advanced_testcase {
|
||||
$this->assertEquals($expectedorder, $modinfo->get_sections()[$sectionnumber]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for can_add_moduleinfo on a non-existing module.
|
||||
*
|
||||
* @covers \can_add_moduleinfo
|
||||
*/
|
||||
public function test_can_add_moduleinfo_invalid_module(): void {
|
||||
global $DB;
|
||||
$this->resetAfterTest(true);
|
||||
|
||||
$course = $this->getDataGenerator()->create_course(
|
||||
['numsections' => 2, 'enablecompletion' => 1],
|
||||
['createsections' => true]
|
||||
);
|
||||
|
||||
$modinfo = get_fast_modinfo($course);
|
||||
$section = $modinfo->get_section_info(2);
|
||||
|
||||
$this->setAdminUser();
|
||||
|
||||
$this->expectException(\dml_missing_record_exception::class);
|
||||
|
||||
can_add_moduleinfo($course, 'non-existent-module!', $section->section);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for can_add_moduleinfo when the user does not have addinstance capability.
|
||||
*
|
||||
* @covers \can_add_moduleinfo
|
||||
*/
|
||||
public function test_can_add_moduleinfo_deny_add_instance(): void {
|
||||
global $DB;
|
||||
$this->resetAfterTest(true);
|
||||
|
||||
$course = $this->getDataGenerator()->create_course(
|
||||
['numsections' => 2, 'enablecompletion' => 1],
|
||||
['createsections' => true]
|
||||
);
|
||||
|
||||
$modinfo = get_fast_modinfo($course);
|
||||
$section = $modinfo->get_section_info(2);
|
||||
|
||||
// The can_add_moduleinfo uses course_allowed_module to check if the module is allowed in the course.
|
||||
// This method uses capabilities like the specific module addinstance capability.
|
||||
$teacherrole = $DB->get_record('role', ['shortname' => 'editingteacher'], '*', MUST_EXIST);
|
||||
role_change_permission(
|
||||
$teacherrole->id,
|
||||
\context_course::instance($course->id),
|
||||
'mod/label:addinstance',
|
||||
CAP_PROHIBIT
|
||||
);
|
||||
|
||||
$user = $this->getDataGenerator()->create_user();
|
||||
$this->getDataGenerator()->enrol_user($user->id, $course->id, 'editingteacher');
|
||||
$this->setUser($user);
|
||||
|
||||
$this->expectException(\moodle_exception::class);
|
||||
|
||||
can_add_moduleinfo($course, 'label', $section->section);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for can_add_moduleinfo.
|
||||
*
|
||||
* @dataProvider provider_can_add_moduleinfo
|
||||
* @covers \can_add_moduleinfo
|
||||
* @param string $rolename
|
||||
* @param bool $hascapability
|
||||
*/
|
||||
public function test_can_add_moduleinfo_capability(string $rolename, bool $hascapability): void {
|
||||
global $DB;
|
||||
$this->resetAfterTest(true);
|
||||
|
||||
$module = $DB->get_record('modules', ['name' => 'label'], '*', MUST_EXIST);
|
||||
|
||||
$course = $this->getDataGenerator()->create_course(
|
||||
['numsections' => 2, 'enablecompletion' => 1],
|
||||
['createsections' => true]
|
||||
);
|
||||
|
||||
$modinfo = get_fast_modinfo($course);
|
||||
$section = $modinfo->get_section_info(2);
|
||||
|
||||
$user = $this->getDataGenerator()->create_user();
|
||||
$this->getDataGenerator()->enrol_user($user->id, $course->id, $rolename);
|
||||
$this->setUser($user);
|
||||
|
||||
if (!$hascapability) {
|
||||
$this->expectException(\required_capability_exception::class);
|
||||
}
|
||||
|
||||
$result = can_add_moduleinfo($course, 'label', $section->section);
|
||||
|
||||
$this->assertEquals($module, $result[0]);
|
||||
$this->assertEquals(
|
||||
\context_course::instance($course->id),
|
||||
$result[1]
|
||||
);
|
||||
$this->assertEquals($section->id, $result[2]->id);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for can_add_moduleinfo returns true on a delegate section.
|
||||
*
|
||||
* @dataProvider provider_can_add_moduleinfo
|
||||
* @covers \can_add_moduleinfo
|
||||
* @param string $rolename
|
||||
* @param bool $hascapability
|
||||
*/
|
||||
public function test_can_add_moduleinfo_delegate_section(string $rolename, bool $hascapability): void {
|
||||
global $DB;
|
||||
$this->resetAfterTest(true);
|
||||
|
||||
$module = $DB->get_record('modules', ['name' => 'label'], '*', MUST_EXIST);
|
||||
|
||||
$course = $this->getDataGenerator()->create_course(
|
||||
['numsections' => 2, 'enablecompletion' => 1],
|
||||
['createsections' => true]
|
||||
);
|
||||
|
||||
$section = formatactions::section($course)->create_delegated('mod_label', 0);
|
||||
|
||||
$modinfo = get_fast_modinfo($course);
|
||||
$this->assertCount(4, $modinfo->get_section_info_all());
|
||||
$this->assertCount(3, $modinfo->get_listed_section_info_all());
|
||||
|
||||
$user = $this->getDataGenerator()->create_user();
|
||||
$this->getDataGenerator()->enrol_user($user->id, $course->id, $rolename);
|
||||
$this->setUser($user);
|
||||
|
||||
if (!$hascapability) {
|
||||
$this->expectException(\required_capability_exception::class);
|
||||
}
|
||||
|
||||
$result = can_add_moduleinfo($course, 'label', $section->section);
|
||||
|
||||
$this->assertEquals($module, $result[0]);
|
||||
$this->assertEquals(
|
||||
\context_course::instance($course->id),
|
||||
$result[1]
|
||||
);
|
||||
$this->assertEquals($section->id, $result[2]->id);
|
||||
|
||||
// Validate no section has been created.
|
||||
$modinfo = get_fast_modinfo($course);
|
||||
$this->assertCount(4, $modinfo->get_section_info_all());
|
||||
$this->assertCount(3, $modinfo->get_listed_section_info_all());
|
||||
$this->assertEquals(
|
||||
$section->section,
|
||||
$modinfo->get_section_info_by_id($section->id)->section
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Data provider for test_can_add_moduleinfo.
|
||||
* @return array
|
||||
*/
|
||||
public static function provider_can_add_moduleinfo(): array {
|
||||
return [
|
||||
'Editing teacher' => [
|
||||
'rolename' => 'editingteacher',
|
||||
'hascapability' => true,
|
||||
],
|
||||
'Manager' => [
|
||||
'rolename' => 'manager',
|
||||
'hascapability' => true,
|
||||
],
|
||||
'Course creator' => [
|
||||
'rolename' => 'coursecreator',
|
||||
'hascapability' => false,
|
||||
],
|
||||
'Non-editing teacher' => [
|
||||
'rolename' => 'teacher',
|
||||
'hascapability' => false,
|
||||
],
|
||||
'Student' => [
|
||||
'rolename' => 'student',
|
||||
'hascapability' => false,
|
||||
],
|
||||
'Guest' => [
|
||||
'rolename' => 'guest',
|
||||
'hascapability' => false,
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user