MDL-51833 tool_monitor: Improve access check for monitor.

admin/tool/monitor/index.php

@@ -41,7 +41,7 @@ if (empty($courseid)) {
     // check system level capability.
     if (!has_capability('tool/monitor:subscribe', $context)) {
         // If not system level then check to see if they have access to any course level rules.
-        if (tool_monitor_get_user_courses()){
+        if (tool_monitor_can_subscribe()) {
             // Make them choose a course.
             $choose = true;
         } else {

admin/tool/monitor/lib.php

@@ -75,12 +75,20 @@ function tool_monitor_extend_navigation_frontpage($navigation, $course, $context
  * @param context         $coursecontext     The context of the course
  */
 function tool_monitor_extend_navigation_user_settings($navigation, $user, $usercontext, $course, $coursecontext) {
-    global $USER, $SITE;
+    global $USER, $PAGE;
+
+    // Don't bother doing needless calculations unless we are on the relevant pages.
+    $onpreferencepage = $PAGE->url->compare(new moodle_url('/user/preferences.php'), URL_MATCH_BASE);
+    $onmonitorpage = $PAGE->url->compare(new moodle_url('/admin/tool/monitor/index.php'), URL_MATCH_BASE);
+    if (!$onpreferencepage && !$onmonitorpage) {
+        return null;
+    }
 
     // Don't show the setting if the event monitor isn't turned on. No access to other peoples subscriptions.
     if (get_config('tool_monitor', 'enablemonitor') && $USER->id == $user->id) {
         // Now let's check to see if the user has any courses / site rules that they can subscribe to.
-        if ($courses = tool_monitor_get_user_courses()) {
+        // We skip doing a check here if we are on the event monitor page as the check is done internally on that page.
+        if ($onmonitorpage || tool_monitor_can_subscribe()) {
             $url = new moodle_url('/admin/tool/monitor/index.php');
             $subsnode = navigation_node::create(get_string('managesubscriptions', 'tool_monitor'), $url,
                     navigation_node::TYPE_SETTING, null, 'monitor', new pix_icon('i/settings', ''));
@@ -92,6 +100,19 @@ function tool_monitor_extend_navigation_user_settings($navigation, $user, $userc
     }
 }
 
+/**
+ * Check if the user has the capacity to subscribe to an event monitor anywhere.
+ *
+ * @return bool True if a capability in a course is found. False otherwise.
+ */
+function tool_monitor_can_subscribe() {
+    if (has_capability('tool/monitor:subscribe', context_system::instance())) {
+        return true;
+    }
+    $courses = get_user_capability_course('tool/monitor:subscribe', null, true, '', '', 1);
+    return empty($courses) ? false : true;
+}
+
 /**
  * Get a list of courses and also include 'Site' for site wide rules.
  *
@@ -103,8 +124,11 @@ function tool_monitor_get_user_courses() {
     if (has_capability('tool/monitor:subscribe', context_system::instance())) {
         $options[0] = get_string('site');
     }
-    if ($courses = get_user_capability_course('tool/monitor:subscribe', null, true, 'fullname, visible', $orderby)) {
+
+    $fields = 'fullname, visible, ctxid, ctxpath, ctxdepth, ctxlevel, ctxinstance';
+    if ($courses = get_user_capability_course('tool/monitor:subscribe', null, true, $fields, $orderby)) {
         foreach ($courses as $course) {
+            context_helper::preload_from_record($course);
             $coursectx = context_course::instance($course->id);
             if ($course->visible || has_capability('moodle/course:viewhiddencourses', $coursectx)) {
                 $options[$course->id] = format_string($course->fullname, true, array('context' => $coursectx));

lib/accesslib.php

@@ -3986,20 +3986,26 @@ function count_role_users($roleid, context $context, $parent = false) {
  * @param int $userid User ID or null for current user
  * @param bool $doanything True if 'doanything' is permitted (default)
  * @param string $fieldsexceptid Leave blank if you only need 'id' in the course records;
- *   otherwise use a comma-separated list of the fields you require, not including id
+ *   otherwise use a comma-separated list of the fields you require, not including id.
+ *   Add ctxid, ctxpath, ctxdepth etc to return course context information for preloading.
  * @param string $orderby If set, use a comma-separated list of fields from course
  *   table with sql modifiers (DESC) if needed
+ * @param int $limit Limit the number of courses to return on success. Zero equals all entries.
  * @return array|bool Array of courses, if none found false is returned.
  */
-function get_user_capability_course($capability, $userid = null, $doanything = true, $fieldsexceptid = '', $orderby = '') {
+function get_user_capability_course($capability, $userid = null, $doanything = true, $fieldsexceptid = '', $orderby = '',
+        $limit = 0) {
     global $DB;
 
     // Convert fields list and ordering
     $fieldlist = '';
     if ($fieldsexceptid) {
-        $fields = explode(',', $fieldsexceptid);
+        $fields = array_map('trim', explode(',', $fieldsexceptid));
         foreach($fields as $field) {
-            $fieldlist .= ',c.'.$field;
+            // Context fields have a different alias and are added further down.
+            if (strpos($field, 'ctx') !== 0) {
+                $fieldlist .= ',c.'.$field;
+            }
         }
     }
     if ($orderby) {
@@ -4020,6 +4026,13 @@ function get_user_capability_course($capability, $userid = null, $doanything = t
 
     $contextpreload = context_helper::get_preload_record_columns_sql('x');
 
+    $contextlist = ['ctxid', 'ctxpath', 'ctxdepth', 'ctxlevel', 'ctxinstance'];
+    $unsetitems = $contextlist;
+    if ($fieldsexceptid) {
+        $coursefields = array_map('trim', explode(',', $fieldsexceptid));
+        $unsetitems = array_diff($contextlist, $coursefields);
+    }
+
     $courses = array();
     $rs = $DB->get_recordset_sql("SELECT c.id $fieldlist, $contextpreload
                                     FROM {course} c
@@ -4027,12 +4040,23 @@ function get_user_capability_course($capability, $userid = null, $doanything = t
                                 $orderby");
     // Check capability for each course in turn
     foreach ($rs as $course) {
-        context_helper::preload_from_record($course);
+        // The preload_from_record() unsets the context related fields, but it is possible that our caller may
+        // want them returned too (so they can use them for their own context preloading). For that reason we
+        // pass a clone.
+        context_helper::preload_from_record(clone($course));
         $context = context_course::instance($course->id);
         if (has_capability($capability, $context, $userid, $doanything)) {
+            // Unset context fields if they were not asked for.
+            foreach ($unsetitems as $item) {
+                unset($course->$item);
+            }
             // We've got the capability. Make the record look like a course record
             // and store it
             $courses[] = $course;
+            $limit--;
+            if ($limit == 0) {
+                break;
+            }
         }
     }
     $rs->close();

lib/upgrade.txt

@@ -50,6 +50,8 @@ information provided here is intended especially for developers.
     - "<element_string>" "<selector_string>" should exists
     - "<element_string>" "<selector_string>" should not exists
     - the following "<element_string>" exists:
+* get_user_capability_course() now has an additional parameter 'limit'. This can be used to return a set number of records with
+  the submitted capability. The parameter 'fieldsexceptid' will now accept context fields which can be used for preloading.
 
 === 3.2 ===