diff --git a/course/renderer.php b/course/renderer.php index 75141fc72bc..7ced2f26463 100644 --- a/course/renderer.php +++ b/course/renderer.php @@ -70,7 +70,7 @@ class core_course_renderer extends plugin_renderer_base { if (!empty($CFG->coursecontact)) { $coursecontactroles = explode(',', $CFG->coursecontact); foreach ($coursecontactroles as $roleid) { - if ($users = get_role_users($roleid, $context, true)) { + if ($users = get_role_users($roleid, $context, true, '', null, false)) { foreach ($users as $teacher) { $role = new stdClass(); $role->id = $teacher->roleid; diff --git a/lib/accesslib.php b/lib/accesslib.php index b69a4a50a8f..c6d7f2c976b 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -3862,7 +3862,7 @@ function sort_by_roleassignment_authority($users, context $context, $roles = arr * @param string $fields fields from user (u.) , role assignment (ra) or role (r.) * @param string $sort sort from user (u.) , role assignment (ra.) or role (r.). * null => use default sort from users_order_by_sql. - * @param bool $gethidden_ignored use enrolments instead + * @param bool $all true means all, false means limit to enrolled users * @param string $group defaults to '' * @param mixed $limitfrom defaults to '' * @param mixed $limitnum defaults to '' @@ -3871,7 +3871,7 @@ function sort_by_roleassignment_authority($users, context $context, $roles = arr * @return array */ function get_role_users($roleid, context $context, $parent = false, $fields = '', - $sort = null, $gethidden_ignored = null, $group = '', + $sort = null, $all = true, $group = '', $limitfrom = '', $limitnum = '', $extrawheretest = '', $whereorsortparams = array()) { global $DB; @@ -3922,7 +3922,7 @@ function get_role_users($roleid, context $context, $parent = false, $fields = '' } if ($whereorsortparams) { - $params = array_merge($params, $whereparams); + $params = array_merge($params, $whereorsortparams); } if (!$sort) { @@ -3930,10 +3930,24 @@ function get_role_users($roleid, context $context, $parent = false, $fields = '' $params = array_merge($params, $sortparams); } + if ($all === null) { + // Previously null was used to indicate that parameter was not used. + $all = true; + } + if (!$all and $coursecontext) { + // Do not use get_enrolled_sql() here for performance reasons. + $ejoin = "JOIN {user_enrolments} ue ON ue.userid = u.id + JOIN {enrol} e ON (e.id = ue.enrolid AND e.courseid = :ecourseid)"; + $params['ecourseid'] = $coursecontext->instanceid; + } else { + $ejoin = ""; + } + $sql = "SELECT DISTINCT $fields, ra.roleid FROM {role_assignments} ra JOIN {user} u ON u.id = ra.userid JOIN {role} r ON ra.roleid = r.id + $ejoin LEFT JOIN {role_names} rn ON (rn.contextid = :coursecontext AND rn.roleid = r.id) $groupjoin WHERE (ra.contextid = :contextid $parentcontexts) diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php index becf2443a96..f0e9ae18bed 100644 --- a/lib/tests/accesslib_test.php +++ b/lib/tests/accesslib_test.php @@ -1222,11 +1222,13 @@ class accesslib_testcase extends advanced_testcase { * @return void */ public function test_get_role_users() { - global $DB; + global $DB, $CFG; + require_once("$CFG->dirroot/group/lib.php"); $this->resetAfterTest(); $systemcontext = context_system::instance(); + $studentrole = $DB->get_record('role', array('shortname'=>'student'), '*', MUST_EXIST); $teacherrole = $DB->get_record('role', array('shortname'=>'editingteacher'), '*', MUST_EXIST); $course = $this->getDataGenerator()->create_course(); $coursecontext = context_course::instance($course->id); @@ -1240,21 +1242,33 @@ class accesslib_testcase extends advanced_testcase { role_assign($teacherrole->id, $user1->id, $coursecontext->id); $user2 = $this->getDataGenerator()->create_user(); role_assign($teacherrole->id, $user2->id, $systemcontext->id); + $user3 = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($user3->id, $course->id, $teacherrole->id); + $user4 = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($user4->id, $course->id, $studentrole->id); + + $group = $this->getDataGenerator()->create_group(array('courseid'=>$course->id)); + groups_add_member($group->id, $user3->id); $users = get_role_users($teacherrole->id, $coursecontext); - $this->assertCount(1, $users); - $user = reset($users); - $userid = key($users); - $this->assertEquals($userid, $user->id); + $this->assertEquals(array($user1->id, $user3->id), array_keys($users), '', 0, 10, true); + $user = $users[$user1->id]; $this->assertEquals($teacherrole->id, $user->roleid); $this->assertEquals($teacherrole->name, $user->rolename); $this->assertEquals($teacherrole->shortname, $user->roleshortname); $this->assertEquals($teacherrename->name, $user->rolecoursealias); $users = get_role_users($teacherrole->id, $coursecontext, true); + $this->assertEquals(array($user1->id, $user2->id, $user3->id), array_keys($users), '', 0, 10, true); + + $users = get_role_users($teacherrole->id, $coursecontext, false, '', null, false); + $this->assertEquals(array($user3->id), array_keys($users), '', 0, 10, true); + + $users = get_role_users($teacherrole->id, $coursecontext, false, '', null, null); $this->assertCount(2, $users); - $users = get_role_users($teacherrole->id, $coursecontext, false, 'u.id, u.email, u.idnumber', 'u.idnumber', null, 1, 0, 10, 'u.deleted = 0'); + $users = get_role_users($teacherrole->id, $coursecontext, false, 'u.id, u.email, u.idnumber', 'u.idnumber', true, $group->id, 0, 10, 'u.deleted = 0'); + $this->assertEquals(array($user3->id), array_keys($users), '', 0, 10, true); } /**