MDL-41042 coursecat: reset course contacts cache more often

enrol/manual/locallib.php

@@ -270,6 +270,8 @@ class enrol_manual_editselectedusers_operation extends enrol_bulk_enrolment_oper
                     $event->trigger();
                 }
             }
+            // Delete cached course contacts for this course because they may be affected.
+            cache::make('core', 'coursecontacts')->delete($manager->get_context()->instanceid);
             return true;
         }
 

enrol/upgrade.txt

@@ -6,6 +6,7 @@ information provided here is intended especially for developers.
 * Added new events enrol_instance_created, enrol_instance_updated and
   enrol_instance_deleted . Always trigger them when changing records in the
   DB table 'enrol'.
+* Constant CACHE_COURSE_CONTACTS_TTL was deleted.
 
 === 2.9 ===
 

lib/accesslib.php

@@ -1673,7 +1673,7 @@ function get_roles_with_capability($capability, $permission = null, $context = n
  * @return int new/existing id of the assignment
  */
 function role_assign($roleid, $userid, $contextid, $component = '', $itemid = 0, $timemodified = '') {
-    global $USER, $DB;
+    global $USER, $DB, $CFG;
 
     // first of all detect if somebody is using old style parameters
     if ($contextid === 0 or is_numeric($component)) {
@@ -1754,6 +1754,9 @@ function role_assign($roleid, $userid, $contextid, $component = '', $itemid = 0,
         reload_all_capabilities();
     }
 
+    require_once($CFG->libdir . '/coursecatlib.php');
+    coursecat::role_assignment_changed($roleid, $context);
+
     $event = \core\event\role_assigned::create(array(
         'context' => $context,
         'objectid' => $ra->roleid,
@@ -1811,6 +1814,7 @@ function role_unassign($roleid, $userid, $contextid, $component = '', $itemid =
  */
 function role_unassign_all(array $params, $subcontexts = false, $includemanual = false) {
     global $USER, $CFG, $DB;
+    require_once($CFG->libdir . '/coursecatlib.php');
 
     if (!$params) {
         throw new coding_exception('Missing parameters in role_unsassign_all() call');
@@ -1861,6 +1865,7 @@ function role_unassign_all(array $params, $subcontexts = false, $includemanual =
             ));
             $event->add_record_snapshot('role_assignments', $ra);
             $event->trigger();
+            coursecat::role_assignment_changed($ra->roleid, $context);
         }
     }
     unset($ras);
@@ -1892,6 +1897,7 @@ function role_unassign_all(array $params, $subcontexts = false, $includemanual =
                             'other'=>array('id'=>$ra->id, 'component'=>$ra->component, 'itemid'=>$ra->itemid)));
                     $event->add_record_snapshot('role_assignments', $ra);
                     $event->trigger();
+                    coursecat::role_assignment_changed($ra->roleid, $context);
                 }
             }
         }

lib/coursecatlib.php

@@ -52,9 +52,6 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate {
     /** @var coursecat stores pseudo category with id=0. Use coursecat::get(0) to retrieve */
     protected static $coursecat0;
 
-    /** Do not fetch course contacts more often than once per hour. */
-    const CACHE_COURSE_CONTACTS_TTL = 3600;
-
     /** @var array list of all fields and their short name and default value for caching */
     protected static $coursecatfields = array(
         'id' => array('id', 0),
@@ -673,6 +670,85 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate {
                 array('contextcoursecat' => CONTEXT_COURSECAT) + $params);
     }
 
+    /**
+     * Resets course contact caches when role assignments were changed
+     *
+     * @param int $roleid role id that was given or taken away
+     * @param context $context context where role assignment has been changed
+     */
+    public static function role_assignment_changed($roleid, $context) {
+        global $CFG, $DB;
+
+        if ($context->contextlevel > CONTEXT_COURSE) {
+            // No changes to course contacts if role was assigned on the module/block level.
+            return;
+        }
+
+        if (!$CFG->coursecontact || !in_array($roleid, explode(',', $CFG->coursecontact))) {
+            // The role is not one of course contact roles.
+            return;
+        }
+
+        // Remove from cache course contacts of all affected courses.
+        $cache = cache::make('core', 'coursecontacts');
+        if ($context->contextlevel == CONTEXT_COURSE) {
+            $cache->delete($context->instanceid);
+        } else if ($context->contextlevel == CONTEXT_SYSTEM) {
+            $cache->purge();
+        } else {
+            $sql = "SELECT ctx.instanceid
+                    FROM {context} ctx
+                    WHERE ctx.path LIKE ? AND ctx.contextlevel = ?";
+            $params = array($context->path . '/%', CONTEXT_COURSE);
+            if ($courses = $DB->get_fieldset_sql($sql, $params)) {
+                $cache->delete_many($courses);
+            }
+        }
+    }
+
+    /**
+     * Executed when user enrolment was changed to check if course
+     * contacts cache needs to be cleared
+     *
+     * @param int $courseid course id
+     * @param int $userid user id
+     * @param int $status new enrolment status (0 - active, 1 - suspended)
+     * @param int $timestart new enrolment time start
+     * @param int $timeend new enrolment time end
+     */
+    public static function user_enrolment_changed($courseid, $userid,
+            $status, $timestart = null, $timeend = null) {
+        $cache = cache::make('core', 'coursecontacts');
+        $contacts = $cache->get($courseid);
+        if ($contacts === false) {
+            // The contacts for the affected course were not cached anyway.
+            return;
+        }
+        $enrolmentactive = ($status == 0) &&
+                (!$timestart || $timestart < time()) &&
+                (!$timeend || $timeend > time());
+        if (!$enrolmentactive) {
+            $isincontacts = false;
+            foreach ($contacts as $contact) {
+                if ($contact->id == $userid) {
+                    $isincontacts = true;
+                }
+            }
+            if (!$isincontacts) {
+                // Changed user's enrolment does not exist or is not active,
+                // and he is not in cached course contacts, no changes to be made.
+                return;
+            }
+        }
+        // Either enrolment of manager was deleted/suspended
+        // or user enrolment was added or activated.
+        // In order to see if the course contacts for this course need
+        // changing we would need to make additional queries, they will
+        // slow down bulk enrolment changes. It is better just to remove
+        // course contacts cache for this course.
+        $cache->delete($courseid);
+    }
+
     /**
      * Given list of DB records from table course populates each record with list of users with course contact roles
      *

lib/enrollib.php

@@ -1330,6 +1330,10 @@ abstract class enrol_plugin {
                         )
                     );
             $event->trigger();
+            // Check if course contacts cache needs to be cleared.
+            require_once($CFG->libdir . '/coursecatlib.php');
+            coursecat::user_enrolment_changed($courseid, $ue->userid,
+                    $ue->status, $ue->timestart, $ue->timeend);
         }
 
         if ($roleid) {
@@ -1370,7 +1374,7 @@ abstract class enrol_plugin {
      * @return void
      */
     public function update_user_enrol(stdClass $instance, $userid, $status = NULL, $timestart = NULL, $timeend = NULL) {
-        global $DB, $USER;
+        global $DB, $USER, $CFG;
 
         $name = $this->get_name();
 
@@ -1420,6 +1424,10 @@ abstract class enrol_plugin {
                     )
                 );
         $event->trigger();
+
+        require_once($CFG->libdir . '/coursecatlib.php');
+        coursecat::user_enrolment_changed($instance->courseid, $ue->userid,
+                $ue->status, $ue->timestart, $ue->timeend);
     }
 
     /**
@@ -1501,6 +1509,10 @@ abstract class enrol_plugin {
         // reset all enrol caches
         $context->mark_dirty();
 
+        // Check if courrse contacts cache needs to be cleared.
+        require_once($CFG->libdir . '/coursecatlib.php');
+        coursecat::user_enrolment_changed($courseid, $ue->userid, ENROL_USER_SUSPENDED);
+
         // reset current user enrolment caching
         if ($userid == $USER->id) {
             if (isset($USER->enrol['enrolled'][$courseid])) {

lib/tests/coursecatlib_test.php

@@ -561,6 +561,12 @@ class core_coursecatlib_testcase extends advanced_testcase {
 
         $manual = enrol_get_plugin('manual');
 
+        // Nobody is enrolled now and course contacts are empty.
+        $allcourses = coursecat::get(0)->get_courses(array('recursive' => true, 'coursecontacts' => true, 'sort' => array('idnumber' => 1)));
+        foreach ($allcourses as $onecourse) {
+            $this->assertEmpty($onecourse->get_course_contacts());
+        }
+
         // Cat1 (user2 has teacher role)
         role_assign($teacherrole->id, $user[2], context_coursecat::instance($category[1]));
         // course21 (user2 is enrolled as manager)
@@ -616,6 +622,14 @@ class core_coursecatlib_testcase extends advanced_testcase {
         //   -- course12 (user1 has teacher role)         |
         $this->assertSame('', $contacts[1][2]);
 
+        // Suspend user 4 and make sure he is no longer in contacts of course 1 in category 4.
+        $manual->enrol_user($enrol[4][1], $user[4], $teacherrole->id, 0, 0, ENROL_USER_SUSPENDED);
+        $allcourses = coursecat::get(0)->get_courses(array('recursive' => true, 'coursecontacts' => true, 'sort' => array('idnumber' => 1)));
+        $contacts = $allcourses[$course[4][1]]->get_course_contacts();
+        $this->assertCount(1, $contacts);
+        $contact = reset($contacts);
+        $this->assertEquals('F5 L5', $contact['username']);
+
         $CFG->coursecontact = $oldcoursecontact;
     }