mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-01-18 05:58:34 +01:00
Code Issues Projects Releases Wiki Activity

MDL-73839 tool_uploadcourse: Proper enrol instance updates.

Browse Source 
We should use update_instance() method instead of direct query.
Fixed validation role method to validate roleids and role names.
Fiexed bug in unit test - course creator role shouldn't be assigned for
self enrolment method.
This commit is contained in:
Ilya Tregubov 2022-09-09 11:58:54 +04:00
parent 8dbb6183ff
commit 491a784813
No known key found for this signature in database
GPG Key ID: 0F58186F748E55C1
2 changed files with 32 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
admin/tool/uploadcourse/classes/course.php
View File

@ -932,17 +932,22 @@ class tool_uploadcourse_course {
foreach ($enrolmentdata as $method => $options) {
if (isset($options['role'])) {
$role = $options['role'];
if (isset($options['role']) || isset($options['roleid'])) {
if (isset($options['role'])) {
$role = $options['role'];
$roleid = $DB->get_field('role', 'id', ['shortname' => $role], MUST_EXIST);
} else {
$roleid = $options['roleid'];
$role = $DB->get_field('role', 'shortname', ['id' => $roleid], MUST_EXIST);
}
if ($courseid) {
if (!$this->validate_role_context($courseid, $role)) {
if (!$this->validate_role_context($courseid, $roleid)) {
$errors['contextrolenotallowed'] = new lang_string('contextrolenotallowed', 'core_role', $role);
break;
}
} else {
// We can at least check that context level is correct while actual context not exist.
$roleid = $DB->get_field('role', 'id', ['shortname' => $role], MUST_EXIST);
if (!$this->validate_role_context_level($roleid)) {
$errors['contextrolenotallowed'] = new lang_string('contextrolenotallowed', 'core_role', $role);
@ -1079,13 +1084,11 @@ class tool_uploadcourse_course {
}
// Now update values.
foreach ($method as $k => $v) {
$instance->{$k} = $v;
}
$modifiedinstance = $instance;
// Sort out the start, end and date.
$instance->enrolstartdate = (isset($method['startdate']) ? strtotime($method['startdate']) : 0);
$instance->enrolenddate = (isset($method['enddate']) ? strtotime($method['enddate']) : 0);
$modifiedinstance->enrolstartdate = (isset($method['startdate']) ? strtotime($method['startdate']) : 0);
$modifiedinstance->enrolenddate = (isset($method['enddate']) ? strtotime($method['enddate']) : 0);
// Is the enrolment period set?
if (isset($method['enrolperiod']) && ! empty($method['enrolperiod'])) {
@ -1095,35 +1098,40 @@ class tool_uploadcourse_course {
// Try and convert period to seconds.
$method['enrolperiod'] = strtotime('1970-01-01 GMT + ' . $method['enrolperiod']);
}
$instance->enrolperiod = $method['enrolperiod'];
$modifiedinstance->enrolperiod = $method['enrolperiod'];
}
if ($instance->enrolstartdate > 0 && isset($method['enrolperiod'])) {
$instance->enrolenddate = $instance->enrolstartdate + $method['enrolperiod'];
$modifiedinstance->enrolenddate = $instance->enrolstartdate + $method['enrolperiod'];
}
if ($instance->enrolenddate > 0) {
$instance->enrolperiod = $instance->enrolenddate - $instance->enrolstartdate;
$modifiedinstance->enrolperiod = $instance->enrolenddate - $instance->enrolstartdate;
}
if ($instance->enrolenddate < $instance->enrolstartdate) {
$instance->enrolenddate = $instance->enrolstartdate;
$modifiedinstance->enrolenddate = $instance->enrolstartdate;
}
// Sort out the given role.
if (isset($method['role'])) {
$role = $method['role'];
if (!$this->validate_role_context($course->id, $role)) {
if (isset($method['role']) || isset($method['roleid'])) {
if (isset($method['role'])) {
$role = $method['role'];
$roleid = $DB->get_field('role', 'id', ['shortname' => $role], MUST_EXIST);
} else {
$roleid = $method['roleid'];
$role = $DB->get_field('role', 'shortname', ['id' => $roleid], MUST_EXIST);
}
if (!$this->validate_role_context($course->id, $roleid)) {
$this->error('contextrolenotallowed',
new lang_string('contextrolenotallowed', 'core_role', $role));
break;
}
$roleids = tool_uploadcourse_helper::get_role_ids();
if (isset($roleids[$method['role']])) {
$instance->roleid = $roleids[$method['role']];
if (in_array($roleid, $roleids)) {
$modifiedinstance->roleid = $roleid;
}
}
$instance->timemodified = time();
$DB->update_record('enrol', $instance);
$plugin->update_instance($instance, $modifiedinstance);
}
}
}
@ -1132,15 +1140,15 @@ class tool_uploadcourse_course {
* Check if role is allowed in course context
*
* @param int $courseid course context.
* @param string $role Role.
* @param int $roleid Role ID.
* @return bool
*/
protected function validate_role_context(int $courseid, string $role) : bool {
protected function validate_role_context(int $courseid, int $roleid) : bool {
if (empty($this->assignableroles[$courseid])) {
$coursecontext = \context_course::instance($courseid);
$this->assignableroles[$courseid] = get_assignable_roles($coursecontext, ROLENAME_SHORT);
}
if (!in_array($role, $this->assignableroles[$courseid])) {
if (!array_key_exists($roleid, $this->assignableroles[$courseid])) {
return false;
}
return true;

2
admin/tool/uploadcourse/tests/course_test.php
View File

@ -537,7 +537,7 @@ class course_test extends \advanced_testcase {
'enrolment_1' => 'guest',
'enrolment_1_disable' => '1',
'enrolment_2' => 'self',
'enrolment_2_roleid' => '2',
'enrolment_2_roleid' => '5',
'enrolment_3' => 'manual',
'enrolment_3_delete' => '1',
);