From: martinlanghoff <martinlanghoff>
Date: Sun, 6 Jan 2008 23:24:14 +0000 (+0000)
Subject: accesslib: get_user_by_capability() - Move hidden RA checks to subselect
X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=d2c5b7a9fa599d2d4fd47044a1aea0d69000070f;p=moodle.git

accesslib: get_user_by_capability() - Move hidden RA checks to subselect

we don't deal with RAs in the main SELECT -- we deal with _capabilities_
which is an entirely different matter ;-) -- so push the ra.hidden check
into the subselect.

Also, remove ra.hidden from the default list of fields. Hopefully no
callers are using ra.hidden -- if they are, they should be calling
something else, as this function deals with capabilities. So we might
need an audit of callers, to check that noone is expecting ra.hidden
to be there.

MDL-12452
---

diff --git a/lib/accesslib.php b/lib/accesslib.php
index 1f1cc33604..018f0ea751 100755
--- a/lib/accesslib.php
+++ b/lib/accesslib.php
@@ -4281,7 +4281,9 @@ function get_users_by_capability($context, $capability, $fields='', $sort='',
 
     /// Set up hidden role-assignments sql
     if ($view && !has_capability('moodle/role:viewhiddenassigns', $context)) {
-        $wherecond['hiddenra'] = ' ra.hidden = 0 ';
+        $condhiddenra = 'AND ra.hidden = 0 ';
+    } else {
+        $condhiddenra = '';
     }
 
     // Collect WHERE conditions
@@ -4293,9 +4295,9 @@ function get_users_by_capability($context, $capability, $fields='', $sort='',
     /// Set up default fields
     if (empty($fields)) {
         if ($iscoursepage) {
-            $fields = 'u.*, ul.timeaccess as lastaccess, ra.hidden';
+            $fields = 'u.*, ul.timeaccess as lastaccess';
         } else {
-            $fields = 'u.*, ra.hidden';
+            $fields = 'u.*';
         }
     }
 
@@ -4352,6 +4354,7 @@ function get_users_by_capability($context, $capability, $fields='', $sort='',
                           FROM {$CFG->prefix}role_assignments ssra
                           WHERE ssra.contextid IN ($ctxids)
                                 AND ssra.roleid IN (".implode(',',$roleids) .")
+                                $condhiddenra
                           ) ra ON ra.userid = u.id
                     $uljoin ";
         $where  = " WHERE u.deleted = 0 ";
@@ -4407,15 +4410,14 @@ function get_users_by_capability($context, $capability, $fields='', $sort='',
         // with a SELECT FROM user LEFT OUTER JOIN against ra -
         // This is expensive on the SQL and PHP sides -
         // moves a ton of data across the wire.
-
-        // TODO -- test!
         $ss = "SELECT u.id as userid, ra.roleid,
                       ctx.depth
                FROM {$CFG->prefix}user u
                LEFT OUTER JOIN {$CFG->prefix}role_assignments ra 
                  ON (ra.userid = u.id
                      AND ra.contextid IN ($ctxids)
-                     AND ra.roleid IN (".implode(',',$roleids) ."))
+                     AND ra.roleid IN (".implode(',',$roleids) .")
+                     $condhiddenra)
                LEFT OUTER JOIN {$CFG->prefix}context ctx
                  ON ra.contextid=ctx.id
                WHERE u.deleted=0";
@@ -4428,6 +4430,7 @@ function get_users_by_capability($context, $capability, $fields='', $sort='',
                JOIN {$CFG->prefix}context ctx
                  ON ra.contextid=ctx.id
                WHERE ra.contextid IN ($ctxids)
+                     $condhiddenra
                      AND ra.roleid IN (".implode(',',$roleids) .")";
     }