MDL-46803 course: Prevent unenrol users with multiple roles during reset

This commit is contained in:
Julien Boulen 2019-03-18 11:20:08 +01:00
parent ec819146cc
commit b1d1369dff
4 changed files with 123 additions and 2 deletions

View File

@ -3426,7 +3426,9 @@ class core_course_courselib_testcase extends advanced_testcase {
}
/**
* Test reset_course_userdata() with reset_roles_overrides enabled.
* Test reset_course_userdata()
* - with reset_roles_overrides enabled
* - with selective role unenrolments
*/
public function test_course_roles_reset() {
global $DB;
@ -3441,6 +3443,7 @@ class core_course_courselib_testcase extends advanced_testcase {
$roleid = $DB->get_field('role', 'id', array('shortname' => 'student'), MUST_EXIST);
$generator->enrol_user($user->id, $course->id, $roleid);
// Test case with reset_roles_overrides enabled.
// Override course so it does NOT allow students 'mod/forum:viewdiscussion'.
$coursecontext = context_course::instance($course->id);
assign_capability('mod/forum:viewdiscussion', CAP_PREVENT, $roleid, $coursecontext->id);
@ -3456,6 +3459,39 @@ class core_course_courselib_testcase extends advanced_testcase {
// Check new expected capabilities - override at the course level should be reset.
$this->assertTrue(has_capability('mod/forum:viewdiscussion', $coursecontext, $user));
// Test case with selective role unenrolments.
$roles = array();
$roles['student'] = $DB->get_field('role', 'id', array('shortname' => 'student'), MUST_EXIST);
$roles['teacher'] = $DB->get_field('role', 'id', array('shortname' => 'teacher'), MUST_EXIST);
// We enrol a user with student and teacher roles.
$generator->enrol_user($user->id, $course->id, $roles['student']);
$generator->enrol_user($user->id, $course->id, $roles['teacher']);
// When we reset only student role, we expect to keep teacher role.
$resetdata = new stdClass();
$resetdata->id = $course->id;
$resetdata->unenrol_users = array($roles['student']);
reset_course_userdata($resetdata);
$usersroles = enrol_get_course_users_roles($course->id);
$this->assertArrayHasKey($user->id, $usersroles);
$this->assertArrayHasKey($roles['teacher'], $usersroles[$user->id]);
$this->assertArrayNotHasKey($roles['student'], $usersroles[$user->id]);
$this->assertCount(1, $usersroles[$user->id]);
// We reenrol user as student.
$generator->enrol_user($user->id, $course->id, $roles['student']);
// When we reset student and teacher roles, we expect no roles left.
$resetdata = new stdClass();
$resetdata->id = $course->id;
$resetdata->unenrol_users = array($roles['student'], $roles['teacher']);
reset_course_userdata($resetdata);
$usersroles = enrol_get_course_users_roles($course->id);
$this->assertEmpty($usersroles);
}
public function test_course_check_module_updates_since() {

View File

@ -1028,4 +1028,56 @@ class core_enrollib_testcase extends advanced_testcase {
$this->assertEquals($expectedcourses, $actual);
}
/**
* Test enrol_get_course_users_roles function.
*
* @return void
*/
public function test_enrol_get_course_users_roles() {
global $DB;
$this->resetAfterTest();
$user1 = $this->getDataGenerator()->create_user();
$user2 = $this->getDataGenerator()->create_user();
$course = $this->getDataGenerator()->create_course();
$context = context_course::instance($course->id);
$roles = array();
$roles['student'] = $DB->get_field('role', 'id', array('shortname' => 'student'), MUST_EXIST);
$roles['teacher'] = $DB->get_field('role', 'id', array('shortname' => 'teacher'), MUST_EXIST);
$manual = enrol_get_plugin('manual');
$this->assertNotEmpty($manual);
$enrol = $DB->get_record('enrol', array('courseid' => $course->id, 'enrol' => 'manual'), '*', MUST_EXIST);
// Test without enrolments.
$this->assertEmpty(enrol_get_course_users_roles($course->id));
// Test with 1 user, 1 role.
$manual->enrol_user($enrol, $user1->id, $roles['student']);
$return = enrol_get_course_users_roles($course->id);
$this->assertArrayHasKey($user1->id, $return);
$this->assertArrayHasKey($roles['student'], $return[$user1->id]);
$this->assertArrayNotHasKey($roles['teacher'], $return[$user1->id]);
// Test with 1 user, 2 role.
$manual->enrol_user($enrol, $user1->id, $roles['teacher']);
$return = enrol_get_course_users_roles($course->id);
$this->assertArrayHasKey($user1->id, $return);
$this->assertArrayHasKey($roles['student'], $return[$user1->id]);
$this->assertArrayHasKey($roles['teacher'], $return[$user1->id]);
// Test with another user, 1 role.
$manual->enrol_user($enrol, $user2->id, $roles['student']);
$return = enrol_get_course_users_roles($course->id);
$this->assertArrayHasKey($user1->id, $return);
$this->assertArrayHasKey($roles['student'], $return[$user1->id]);
$this->assertArrayHasKey($roles['teacher'], $return[$user1->id]);
$this->assertArrayHasKey($user2->id, $return);
$this->assertArrayHasKey($roles['student'], $return[$user2->id]);
$this->assertArrayNotHasKey($roles['teacher'], $return[$user2->id]);
}
}

View File

@ -865,7 +865,31 @@ function enrol_get_users_courses($userid, $onlyactive = false, $fields = null, $
}
return $courses;
}
/**
* Returns list of roles per users into course.
*
* @param int $courseid Course id.
* @return array Array[$userid][$roleid] = role_assignment.
*/
function enrol_get_course_users_roles(int $courseid) : array {
global $DB;
$context = context_course::instance($courseid);
$roles = array();
$records = $DB->get_recordset('role_assignments', array('contextid' => $context->id));
foreach ($records as $record) {
if (isset($roles[$record->userid]) === false) {
$roles[$record->userid] = array();
}
$roles[$record->userid][$record->roleid] = $record;
}
$records->close();
return $roles;
}
/**

View File

@ -5461,6 +5461,7 @@ function reset_course_userdata($data) {
}
}
$usersroles = enrol_get_course_users_roles($data->courseid);
foreach ($data->unenrol_users as $withroleid) {
if ($withroleid) {
$sql = "SELECT ue.*
@ -5492,7 +5493,15 @@ function reset_course_userdata($data) {
continue;
}
$plugin->unenrol_user($instance, $ue->userid);
if ($withroleid && count($usersroles[$ue->userid]) > 1) {
// If we don't remove all roles and user has more than one role, just remove this role.
role_unassign($withroleid, $ue->userid, $context->id);
unset($usersroles[$ue->userid][$withroleid]);
} else {
// If we remove all roles or user has only one role, unenrol user from course.
$plugin->unenrol_user($instance, $ue->userid);
}
$data->unenrolled[$ue->userid] = $ue->userid;
}
$rs->close();