Index: group/index.php
===================================================================
RCS file: /cvsroot/moodle/moodle/group/index.php,v
retrieving revision 1.38
diff -u -r1.38 index.php
--- group/index.php	19 Nov 2007 20:31:58 -0000	1.38
+++ group/index.php	14 Jan 2008 16:30:06 -0000
@@ -46,18 +46,23 @@
         break;
 
     case 'ajax_getmembersingroup':
-        $members = array();
-        if ($members = groups_get_members($groupid)) {
-            $member_names = array();
-            foreach($members as $member) {
-                $user = new object();
-                $user->id   = $member->id;
-                $user->name = fullname($member, true);
-                $member_names[] = $user;
+        $roles = array();
+        if ($groupmemberroles = groups_get_members_by_course_role($groupid,$courseid,'u.id,u.firstname,u.lastname')) {
+            foreach($groupmemberroles as $roleid=>$roledata) {
+                $shortroledata=new StdClass;
+                $shortroledata->name=$roledata->name;
+                $shortroledata->users=array();                
+                foreach($roledata->users as $member) {
+                    $shortmember=new StdClass;
+                    $shortmember->id=$member->id;
+                    $shortmember->name=fullname($member, true);
+                    $shortroledata->users[]=$shortmember;
+                }
+                $roles[]=$shortroledata;        
             }
-            $json = new Services_JSON();
-            echo $json->encode($member_names);
         }
+        $json = new Services_JSON();
+        echo $json->encode($roles);
         die;  // Client side JavaScript takes it from here.
 
     case 'deletegroup':
@@ -194,21 +199,22 @@
 
 $member_names = array();
 
+$atleastonemember = false;
 if ($sel_groupid) {
-    if ($members = groups_get_members($groupid)) {
-        foreach($members as $member) {
-            $member_names[$member->id] = fullname($member, true);
+
+    if ($groupmemberroles = groups_get_members_by_course_role($groupid,$courseid,'u.id,u.firstname,u.lastname')) {
+        foreach($groupmemberroles as $roleid=>$roledata) {
+            echo '<optgroup label="'.htmlspecialchars($roledata->name).'">';
+            foreach($roledata->users as $member) {
+                echo '<option value="'.$member->id.'">'.fullname($member, true).'</option>';
+                $atleastonemember = true;
+            }
+            echo '</optgroup>';        
         }
-    }
+    } 
 }
-
-if ($member_names) {
-    // Put the groupings into a hash and sort them
-    foreach ($member_names as $userid=>$username) {
-        echo "<option value=\"{$userid}\" title=\"{$username}\">{$username}</option>\n";
-    }
-
-} else {
+    
+if (!$atleastonemember) {
     // Print an empty option to avoid the XHTML error of having an empty select element
     echo '<option>&nbsp;</option>';
 }
Index: group/clientlib.js
===================================================================
RCS file: /cvsroot/moodle/moodle/group/clientlib.js,v
retrieving revision 1.1
diff -u -r1.1 clientlib.js
--- group/clientlib.js	16 Aug 2007 11:06:51 -0000	1.1
+++ group/clientlib.js	14 Jan 2008 16:30:06 -0000
@@ -21,7 +21,7 @@
                 var membersComboEl = document.getElementById("members");
 
                 if (membersComboEl) {
-                    // Clear the members combo box.
+                    // Clear the members list box.
                     while (membersComboEl.firstChild) {
                         membersComboEl.removeChild(membersComboEl.firstChild);
                     }
@@ -30,7 +30,7 @@
         	    if (groupsComboEl && o.responseText) {
         	        var groups = eval("("+o.responseText+")");
 
-        	        // Populate the groups combo box.
+        	        // Populate the groups list box.
                     for (var i=0; i<groups.length; i++) {
                         var optionEl = document.createElement("option");
                         optionEl.setAttribute("value", groups[i].id);
@@ -51,7 +51,7 @@
 
     };
 
-    // Add onchange event to groups combo box.
+    // Add onchange event to groups list box.
     // Okay, this is not working in IE. The onchange is never fired...
     // I'm hard coding the onchange in ../index.php. Not ideal, but it works
     // then. vyshane AT moodle DOT com.
@@ -80,21 +80,27 @@
         	if (o.responseText !== undefined) {
                 var selectEl = document.getElementById("members");
         	    if (selectEl && o.responseText) {
-        	        var members = eval("("+o.responseText+")");
+                    var roles = eval("("+o.responseText+")");
 
-                    // Clear the members combo box.
+                    // Clear the members list box.
                     if (selectEl) {
                         while (selectEl.firstChild) {
                             selectEl.removeChild(selectEl.firstChild);
                         }
                     }
-        	        // Populate the members combo box.
-                    for (var i=0; i<members.length; i++) {
-                        var optionEl = document.createElement("option");
-                        optionEl.setAttribute("value", members[i].id);
-                        optionEl.title = members[i].name;
-                        optionEl.innerHTML = members[i].name;
-                        selectEl.appendChild(optionEl);
+                    // Populate the members list box.
+                    for (var i=0; i<roles.length; i++) {
+                        var optgroupEl = document.createElement("optgroup");
+                        optgroupEl.setAttribute("label",roles[i].name);
+
+                        for(var j=0; j<roles[i].users.length; j++) {
+                            var optionEl = document.createElement("option");
+                            optionEl.setAttribute("value", roles[i].users[j].id);
+                            optionEl.title = roles[i].users[j].name;
+                            optionEl.innerHTML = roles[i].users[j].name;
+                            optgroupEl.appendChild(optionEl);
+                        }
+                        selectEl.appendChild(optgroupEl);
                     }
                 }
         	}
@@ -128,7 +134,7 @@
         spanEl.innerHTML = selectEl.options[selectEl.selectedIndex].title;
     }
 
-    // Clear the members combo box.
+    // Clear the members list box.
     selectEl = document.getElementById("members");
     if (selectEl) {
         while (selectEl.firstChild) {
Index: group/lib.php
===================================================================
RCS file: /cvsroot/moodle/moodle/group/lib.php,v
retrieving revision 1.11
diff -u -r1.11 lib.php
--- group/lib.php	27 Dec 2007 13:40:27 -0000	1.11
+++ group/lib.php	14 Jan 2008 16:30:06 -0000
@@ -299,6 +299,8 @@
 /* various functions used by groups UI */
 /* =================================== */
 
+
+
 /**
  * Gets the users for a course who are not in a specified group
  * @param int $groupid The id of the group
@@ -311,9 +313,55 @@
 function groups_get_users_not_in_group($courseid, $groupid, $searchtext='', $sort = 'u.lastname ASC') {
 
     global $CFG;
-
     $context = get_context_instance(CONTEXT_COURSE, $courseid);
+    
+    if ($searchtext !== '') {   // Search for a subset of remaining users
+        $LIKE      = sql_ilike();
+        $FULLNAME  = sql_fullname();
+        $wheresearch = " AND u.id IN (SELECT id FROM {$CFG->prefix}user WHERE $FULLNAME $LIKE '%$searchtext%' OR email $LIKE '%$searchtext%' )";
+    } else {
+        $wheresearch = '';
+    }
+
+/// Get list of allowed roles     
+    if(!($validroleids=groups_get_possible_roles($context))) {
+        return;
+    }
+    $roleids = '('.implode(',', $validroleids).')';
+
+/// Construct the main SQL
+    $select = " SELECT u.id, u.firstname, u.lastname";
+    $from   = " FROM {$CFG->prefix}user u
+                INNER JOIN {$CFG->prefix}role_assignments ra ON ra.userid = u.id
+                INNER JOIN {$CFG->prefix}role r ON r.id = ra.roleid";
+
+    $where  = " WHERE ra.contextid ".get_related_contexts_string($context)."
+                  AND u.deleted = 0
+                  AND ra.roleid in $roleids
+                  AND u.id NOT IN (SELECT userid
+                                   FROM {$CFG->prefix}groups_members
+                                   WHERE groupid = $groupid)
+                  $wheresearch";
+    $groupby = " GROUP BY u.id, u.firstname, u.lastname ";
+    $orderby = " ORDER BY $sort";
+
+    return get_records_sql($select.$from.$where.$groupby.$orderby);
+}
+
 
+/**
+ * Gets the users for a course who are not in a specified group
+ * @param int $groupid The id of the group
+ * @param string searchtext similar to searchtext in role assign, search
+ * @return array An array of the userids of the non-group members,  or false if
+ * an error occurred.
+ * This function was changed to get_users_by_capability style
+ * mostly because of the searchtext requirement
+ */
+function groups_get_users_not_in_group_by_course_role($courseid, $groupid, $searchtext='', $sort = 'u.lastname ASC') {
+    global $CFG;
+    $context = get_context_instance(CONTEXT_COURSE, $courseid);
+    
     if ($searchtext !== '') {   // Search for a subset of remaining users
         $LIKE      = sql_ilike();
         $FULLNAME  = sql_fullname();
@@ -322,6 +370,47 @@
         $wheresearch = '';
     }
 
+/// Get list of allowed roles     
+    if(!($validroleids=groups_get_possible_roles($context))) {
+        return;
+    }
+    $roleids = '('.implode(',', $validroleids).')';
+    
+    // Note that the role which permits inclusion may not be on the course, but
+    // the second role (r2) on the course is the one used for categorisation.
+    // There might not be a course role because the user might have a role at
+    // higher level, hence the LEFT JOINs.
+    $from   = " FROM {$CFG->prefix}user u
+                INNER JOIN {$CFG->prefix}role_assignments ra ON ra.userid = u.id
+                INNER JOIN {$CFG->prefix}role r ON r.id = ra.roleid
+                LEFT JOIN {$CFG->prefix}role_assignments ra2 ON ra2.userid = u.id AND ra2.contextid='{$context->id}'
+                LEFT JOIN {$CFG->prefix}role r2 ON r2.id = ra2.roleid";
+    
+    $where  = " WHERE ra.contextid ".get_related_contexts_string($context)."
+                  AND u.deleted = 0
+                  AND ra.roleid in $roleids
+                  AND u.id NOT IN (SELECT userid
+                                   FROM {$CFG->prefix}groups_members
+                                   WHERE groupid = $groupid)
+                  $wheresearch";
+
+    $orderby = " ORDER BY r.sortorder,$sort";       
+                  
+    return groups_calculate_course_role_people(get_recordset_sql(
+        $crap="SELECT r2.id AS roleid,r2.shortname AS roleshortname,r2.name AS rolename,
+        u.id AS userid,u.firstname,u.lastname 
+        $from $where $orderby"));        
+}
+
+
+/**
+ * Obtains a list of the possible roles that group members might come from,
+ * on a course. Generally this includes all the roles who would have 
+ * course:view on that course, except the doanything roles.
+ * @param object $context Context of course
+ * @return Array of role ID integers, or false if error/none.
+ */
+function groups_get_possible_roles($context) {
     $capability = 'moodle/course:view';
     $doanything = false;
 
@@ -350,28 +439,10 @@
         if (empty($validroleids)) {
             return false;
         }
-        $roleids =  '('.implode(',', $validroleids).')';
+        return $validroleids;
     } else {
         return false;  // No need to continue, since no roles have this capability set
-    }
-
-/// Construct the main SQL
-    $select = " SELECT u.id, u.firstname, u.lastname";
-    $from   = " FROM {$CFG->prefix}user u
-                INNER JOIN {$CFG->prefix}role_assignments ra ON ra.userid = u.id
-                INNER JOIN {$CFG->prefix}role r ON r.id = ra.roleid";
-
-    $where  = " WHERE ra.contextid ".get_related_contexts_string($context)."
-                  AND u.deleted = 0
-                  AND ra.roleid in $roleids
-                  AND u.id NOT IN (SELECT userid
-                                   FROM {$CFG->prefix}groups_members
-                                   WHERE groupid = $groupid)
-                  $wheresearch";
-    $groupby = " GROUP BY u.id, u.firstname, u.lastname ";
-    $orderby = " ORDER BY $sort";
-
-    return get_records_sql($select.$from.$where.$groupby.$orderby);
+    }    
 }
 
 
@@ -491,4 +562,130 @@
     return delete_records('groupings_groups', 'groupingid', $groupingid, 'groupid', $groupid);
 }
 
+/**
+ * Lists users in a group based on their role on the course.
+ * Returns false if there's an error or there are no users in the group. 
+ * Otherwise returns an array of role ID => role data, where role data includes:
+ * (role) $id, $shortname, $name
+ * $users: array of objects for each user which include the specified fields
+ * Users who do not have a role are stored in the returned array with key '-'
+ * and pseudo-role details (including a name, 'No role'). Users with multiple
+ * roles, same deal with key '*' and name 'Multiple roles'. You can find out 
+ * which roles each has by looking in the $roles array of the user object.
+ * @param int $groupid
+ * @param int $courseid Course ID (should match the group's course)
+ * @param string $fields List of fields from user table prefixed with u, default 'u.*'
+ * @param string $sort SQL ORDER BY clause, default 'u.lastname ASC'
+ * @return array Complex array as described above
+ */
+function groups_get_members_by_course_role($groupid, $courseid, $fields='u.*', $sort='u.lastname ASC') {
+    global $CFG;
+    
+    // Retrieve information about all users and their roles on the course 
+    // (Note that some users in group may not have a role on the course which 
+    // is why the left joins.) 
+    $context=get_context_instance(CONTEXT_COURSE,$courseid);
+    $rs=get_recordset_sql($crap="SELECT r.id AS roleid,r.shortname AS roleshortname,r.name AS rolename,
+                                        u.id AS userid,$fields
+                                  FROM {$CFG->prefix}groups_members gm
+                            INNER JOIN {$CFG->prefix}user u ON u.id = gm.userid
+                             LEFT JOIN {$CFG->prefix}role_assignments ra ON ra.userid = u.id AND ra.contextid='{$context->id}'
+                             LEFT JOIN {$CFG->prefix}role r ON r.id = ra.roleid
+                                 WHERE gm.groupid='$groupid' 
+                              ORDER BY r.sortorder,$sort");
+    
+    return groups_calculate_course_role_people($rs);
+}
+
+/**
+ * Internal function used by groups_get_members_by_course_role to handle the
+ * results of a database query that includes a list of users and possible 
+ * roles on a course.
+ *
+ * @param object $rs The record set (may be false)
+ * @return array As described in groups_get_members_by_course_role 
+ */
+function groups_calculate_course_role_people($rs) {
+    global $CFG;
+    if(!$rs) {
+        return false;
+    }
+    
+    // Array of all involved roles
+    $roles=array();
+    // Array of all retrieved users    
+    $users=array();    
+    // Fill arrays
+    while($rec=rs_fetch_next_record($rs)) {
+        // Create information about user if this is a new one
+        if(!array_key_exists($rec->userid,$users)) {
+            // User data includes all the optional fields, but not any of the
+            // stuff we added to get the role details
+            $userdata=clone($rec);
+            unset($userdata->roleid);
+            unset($userdata->roleshortname);
+            unset($userdata->rolename);
+            unset($userdata->userid);
+            $userdata->id=$rec->userid;
+            
+            // Make an array to hold the list of roles for this user            
+            $userdata->roles=array();
+            $users[$rec->userid]=$userdata;
+        }
+        // If user has a role...
+        if(!is_null($rec->roleid)) {
+            // Create information about role if this is a new one 
+            if(!array_key_exists($rec->roleid,$roles)) {
+                $roledata=new StdClass;
+                $roledata->id=$rec->roleid;
+                $roledata->shortname=$rec->roleshortname;
+                $roledata->name=$rec->rolename;
+                $roledata->users=array();
+                $roles[$roledata->id]=$roledata;
+            }
+            // Record that user has role
+            $users[$rec->userid]->roles[] = $roles[$rec->roleid];
+        }
+    }
+    
+    // Return false if there weren't any users
+    if(count($users)==0) {
+        return false;
+    }
+    
+    // Add pseudo-roles for multiple roles and no roles
+    $roledata=new StdClass;
+    $roledata->name=get_string('multipleroles','role');
+    $roledata->users=array();
+    $roles['*']=$roledata;
+    $roledata=new StdClass;
+    $roledata->name=get_string('noroles','role');
+    $roledata->users=array();
+    $roles['-']=$roledata;
+    
+    // Now we rearrange the data to store users by role
+    foreach($users as $userid=>$userdata) {
+        $rolecount=count($userdata->roles);
+        if($rolecount==0) {
+            $roleid='-';
+        } else if($rolecount>1) {
+            $roleid='*';
+        } else {
+            $roleid=$userdata->roles[0]->id;
+        }        
+        $roles[$roleid]->users[$userid]=$userdata;
+    }
+    
+    // Delete pseudo-roles if not used
+    if(count($roles['*']->users)==0) {
+        unset($roles['*']);
+    } 
+    if(count($roles['-']->users)==0) {
+        unset($roles['-']);
+    } 
+    
+    // Return list of roles containing their users
+    return $roles;
+}
+
 ?>
Index: group/members.php
===================================================================
RCS file: /cvsroot/moodle/moodle/group/members.php,v
retrieving revision 1.5
diff -u -r1.5 members.php
--- group/members.php	27 Dec 2007 13:40:27 -0000	1.5
+++ group/members.php	14 Jan 2008 16:30:06 -0000
@@ -77,12 +77,16 @@
 $groupmembersoptions = '';
 $groupmemberscount = 0;
 
-if ($groupmembers = groups_get_members($groupid)) {
-    foreach($groupmembers as $member) {
-        $groupmembersoptions .= '<option value="'.$member->id.'">'.fullname($member, true).'</option>';
-        $groupmemberscount ++;
+// Get members, organised by role, and display
+if ($groupmemberroles = groups_get_members_by_course_role($groupid,$courseid,'u.id,u.firstname,u.lastname')) {
+    foreach($groupmemberroles as $roleid=>$roledata) {
+        $groupmembersoptions .= '<optgroup label="'.htmlspecialchars($roledata->name).'">';
+        foreach($roledata->users as $member) {
+            $groupmembersoptions .= '<option value="'.$member->id.'">'.fullname($member, true).'</option>';
+            $groupmemberscount ++;
+        }
+        $groupmembersoptions .= '</optgroup>';        
     }
-
 } else {
     $groupmembersoptions .= '<option>&nbsp;</option>';
 }
@@ -91,35 +95,42 @@
 $potentialmembersoptions = '';
 $potentialmemberscount = 0;
 
-$potentialmembers = groups_get_users_not_in_group($courseid, $groupid, $searchtext);
-if (!empty($potentialmembers)) {
-    $potentialmemberscount = count($potentialmembers);
-} else {
-    $potentialmemberscount = 0;
+// Get potential members, organised by role, and count them
+$potentialmembersbyrole = groups_get_users_not_in_group_by_course_role($courseid, $groupid, $searchtext);
+$potentialmemberscount=0;
+$potentialmembersids=array();
+if (!empty($potentialmembersbyrole)) {
+    foreach($potentialmembersbyrole as $roledata) {
+        $potentialmemberscount+=count($roledata->users);
+        $potentialmembersids=array_merge($potentialmembersids,array_keys($roledata->users));
+    }
 }
+
 if ($potentialmemberscount <=  MAX_USERS_PER_PAGE) {
 
-    if ($potentialmembers != false) {
+    if ($potentialmemberscount != 0) {
         // Get other groups user already belongs to
         $sql = "SELECT u.id AS userid, g.* FROM {$CFG->prefix}user u " .
                     "INNER JOIN {$CFG->prefix}groups_members gm ON u.id = gm.userid " .
                     "INNER JOIN {$CFG->prefix}groups g ON gm.groupid = g.id " .
-               "WHERE u.id IN (".implode(',',array_keys($potentialmembers)).") AND g.courseid = {$course->id} ";
+               "WHERE u.id IN (".implode(',',$potentialmembersids).") AND g.courseid = {$course->id} ";
         $rs = get_recordset_sql($sql);
         $groups = array();
         $usergroups = array();
         while ($usergroup =  rs_fetch_next_record($rs)) {
             $usergroups[$usergroup->userid][$usergroup->id] = $usergroup;
         }
-
-        // Put the groupings into a hash and sorts them
-        foreach ($potentialmembers as $userid => $user) {
-            $nonmembers[$userid] = fullname($user)." (".@count($usergroups[$userid]).")";
-        }
-
-        // Print out the HTML
-        foreach($nonmembers as $id => $name) {
-            $potentialmembersoptions .= "<option value=\"$id\">$name</option>\n";
+        rs_close($rs);
+        
+        foreach($potentialmembersbyrole as $roleid=>$roledata) {
+            $potentialmembersoptions .= '<optgroup label="'.htmlspecialchars($roledata->name).'">';
+            foreach($roledata->users as $member) {
+                $name=htmlspecialchars(fullname($member, true));
+                $potentialmembersoptions .= '<option value="'.$member->id.
+                    '" title="'.$name.'">'.$name.
+                    ' ('.@count($usergroups[$member->id]).')</option>';
+            }
+            $potentialmembersoptions .= '</optgroup>';        
         }
     } else {
         $potentialmembersoptions .= '<option>&nbsp;</option>';
Index: lang/en_utf8/role.php
===================================================================
RCS file: /cvsroot/moodle/moodle/lang/en_utf8/role.php,v
retrieving revision 1.55
diff -u -r1.55 role.php
--- lang/en_utf8/role.php	9 Jan 2008 07:36:29 -0000	1.55
+++ lang/en_utf8/role.php	14 Jan 2008 16:30:06 -0000
@@ -90,6 +90,8 @@
 $string['manageroles'] = 'Manage roles';
 $string['metaassignerror'] = 'Can not assign this role to user \"$a\" because Manage metacourse capability is needed.';
 $string['metaunassignerror'] = 'Role of user \"$a\" was automatically reassigned, please unassign the role in child courses instead.';
+$string['multipleroles'] = 'Multiple roles';
+$string['noroles'] = 'No roles';
 $string['overridepermissions'] = 'Override permissions';
 $string['overridepermissionsin'] = 'Override permissions in $a';
 $string['morethan'] = 'More than $a';