accesslib: get_user_by_capability() - Simple cases now handle multiple RAs

The "simple" case SQL did not handle multiple enrolments for the same
user correctly -- it would generate multiple rows for those users,
incorrectly.

With this patch we move the join to RA to a subselect where DISTINCT
takes care of things.

MDL-12452

diff --git a/lib/accesslib.php b/lib/accesslib.php
index 7514239aee28c8b73fc166a360e74ce07f016bfc..85d6c289d581d437d62c7df5373f5bc3e50973e0 100755 (executable)
--- a/lib/accesslib.php
+++ b/lib/accesslib.php
@@ -4330,14 +4330,19 @@ function get_users_by_capability($context, $capability, $fields='', $sort='',
             return get_records_sql($sql, $limitfrom, $limitnum);
         }
 
-        /// Simple SQL assuming no negative rolecaps
+        /// Simple SQL assuming no negative rolecaps.
+        /// We use a subselect to grab the role assignments
+        /// ensuring only one row per user -- even if they
+        /// have many "relevant" role assignments.
         $select = " SELECT $fields";
         $from   = " FROM {$CFG->prefix}user u
-                    JOIN {$CFG->prefix}role_assignments ra ON ra.userid = u.id
+                    JOIN (SELECT DISTINCT ssra.userid
+                          FROM {$CFG->prefix}role_assignments ssra
+                          WHERE ssra.contextid IN ($ctxids)
+                                AND ssra.roleid IN (".implode(',',$roleids) .")
+                          ) ra ON ra.userid = u.id
                     $uljoin ";
-        $where  = " WHERE ra.contextid IN ($ctxids)
-                          AND u.deleted = 0
-                          AND ra.roleid IN (".implode(',',$roleids) .")";
+        $where  = " WHERE u.deleted = 0 ";
         if (count(array_keys($wherecond))) {
             $where .= ' AND ' . implode(' AND ', array_values($wherecond));
         }