Merge branch 'w02_MDL-36898_m25_ccontact' of git://github.com/skodak/moodle

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;

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)

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);
     }
 
     /**