diff --git a/admin/tool/usertours/classes/local/filter/role.php b/admin/tool/usertours/classes/local/filter/role.php index fec6b605919..a64ee83585c 100644 --- a/admin/tool/usertours/classes/local/filter/role.php +++ b/admin/tool/usertours/classes/local/filter/role.php @@ -105,7 +105,11 @@ class role extends base { return true; } + // Use a request cache to save on DB queries. + // We may be checking multiple tours and they'll all be for the same userid, and contextid $cache = \cache::make_from_params(\cache_store::MODE_REQUEST, 'tool_usertours', 'filter_role'); + + // Get all of the roles used in this context, including special roles such as user, and frontpageuser. $cachekey = "{$USER->id}_{$context->id}"; $userroles = $cache->get($cachekey); if ($userroles === false) { @@ -113,8 +117,20 @@ class role extends base { $cache->set($cachekey, $userroles); } + // Some special roles do not include the shortname. + // Therefore we must fetch all roles too. Thankfully these don't actually change based on context. + // They do require a DB call, so let's cache it. + $cachekey = "allroles"; + $allroles = $cache->get($cachekey); + if ($allroles === false) { + $allroles = get_all_roles(); + $cache->set($cachekey, $allroles); + } + + // Now we can check whether any of the user roles are in the list of allowed roles for this filter. foreach ($userroles as $role) { - if (isset($values[$role->roleid])) { + $shortname = $allroles[$role->roleid]->shortname; + if (isset($values[$shortname])) { return true; } } diff --git a/admin/tool/usertours/tests/role_filter_test.php b/admin/tool/usertours/tests/role_filter_test.php index 3f6fed48c42..940e517fc3e 100644 --- a/admin/tool/usertours/tests/role_filter_test.php +++ b/admin/tool/usertours/tests/role_filter_test.php @@ -102,7 +102,7 @@ class tool_usertours_role_filter_testcase extends advanced_testcase { $context = \context_course::instance($this->course->id); $roles = [ - $this->roles['student'], + 'student', ]; // Note: No need to persist this tour. @@ -131,8 +131,8 @@ class tool_usertours_role_filter_testcase extends advanced_testcase { $context = \context_course::instance($this->course->id); $roles = [ - $this->roles['teacher'], - $this->roles['editingteacher'], + 'teacher', + 'editingteacher', ]; // Note: No need to persist this tour. @@ -161,7 +161,7 @@ class tool_usertours_role_filter_testcase extends advanced_testcase { $context = \context_course::instance($this->course->id); $roles = [ - $this->roles['student'], + 'student', ]; $this->getDataGenerator()->enrol_user($this->student->id, $this->course->id, $this->roles['teacher']); @@ -222,8 +222,8 @@ class tool_usertours_role_filter_testcase extends advanced_testcase { $roles = [ \tool_usertours\local\filter\role::ROLE_SITEADMIN, - $this->roles['teacher'], - $this->roles['editingteacher'], + 'teacher', + 'editingteacher', ]; // Note: No need to persist this tour.