diff --git a/admin/tool/usertours/classes/local/filter/role.php b/admin/tool/usertours/classes/local/filter/role.php index 96bc6ea29dc..fec6b605919 100644 --- a/admin/tool/usertours/classes/local/filter/role.php +++ b/admin/tool/usertours/classes/local/filter/role.php @@ -36,6 +36,13 @@ use context; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class role extends base { + /** + * The Site Admin pseudo-role. + * + * @var ROLE_SITEADMIN int + */ + const ROLE_SITEADMIN = -1; + /** * The name of the filter. * @@ -52,7 +59,24 @@ class role extends base { * And whose values are the values to display */ public static function get_filter_options() { - return role_get_names(null, ROLENAME_ALIAS, true); + $allroles = role_get_names(null, ROLENAME_ALIAS); + + $roles = []; + foreach ($allroles as $role) { + if ($role->archetype === 'guest') { + // No point in including the 'guest' role as it isn't possible to show tours to a guest. + continue; + } + $roles[$role->shortname] = $role->localname; + } + + // Add the Site Administrator pseudo-role. + $roles[self::ROLE_SITEADMIN] = get_string('administrator', 'core'); + + // Sort alphabetically too. + \core_collator::asort($roles); + + return $roles; } /** @@ -73,13 +97,14 @@ class role extends base { return true; } - if (is_siteadmin()) { - return true; - } - // Presence within the array is sufficient. Ignore any value. $values = array_flip($values); + if (isset($values[self::ROLE_SITEADMIN]) && is_siteadmin()) { + // This tour has been restricted to a role including site admin, and this user is a site admin. + return true; + } + $cache = \cache::make_from_params(\cache_store::MODE_REQUEST, 'tool_usertours', 'filter_role'); $cachekey = "{$USER->id}_{$context->id}"; $userroles = $cache->get($cachekey); diff --git a/admin/tool/usertours/tests/role_filter_test.php b/admin/tool/usertours/tests/role_filter_test.php index ade49dda885..3f6fed48c42 100644 --- a/admin/tool/usertours/tests/role_filter_test.php +++ b/admin/tool/usertours/tests/role_filter_test.php @@ -119,9 +119,9 @@ class tool_usertours_role_filter_testcase extends advanced_testcase { } } - // The admin should always be able to view too. + // The admin can't view this one either. $this->setAdminUser(); - $this->assertTrue(\tool_usertours\local\filter\role::filter_matches($tour, $context)); + $this->assertFalse(\tool_usertours\local\filter\role::filter_matches($tour, $context)); } /** @@ -149,9 +149,9 @@ class tool_usertours_role_filter_testcase extends advanced_testcase { } } - // The admin should always be able to view too. + // The admin can't view this one either. $this->setAdminUser(); - $this->assertTrue(\tool_usertours\local\filter\role::filter_matches($tour, $context)); + $this->assertFalse(\tool_usertours\local\filter\role::filter_matches($tour, $context)); } /** @@ -181,8 +181,109 @@ class tool_usertours_role_filter_testcase extends advanced_testcase { } } - // The admin should always be able to view too. + // The admin can't view this one either. + $this->setAdminUser(); + $this->assertFalse(\tool_usertours\local\filter\role::filter_matches($tour, $context)); + } + + /** + * Test the filter_matches function when it is targetted at an admin. + */ + public function test_filter_matches_multiple_role_only_admin() { + $context = \context_course::instance($this->course->id); + + $roles = [ + \tool_usertours\local\filter\role::ROLE_SITEADMIN, + ]; + + $this->getDataGenerator()->enrol_user($this->student->id, $this->course->id, $this->roles['teacher']); + + // Note: No need to persist this tour. + $tour = new \tool_usertours\tour(); + $tour->set_filter_values('role', $roles); + + + // Note: The role filter does not use the context. + foreach ($this->testroles as $role) { + $this->setUser($this->$role); + $this->assertFalse(\tool_usertours\local\filter\role::filter_matches($tour, $context)); + } + + // The admin can view this one because it's only aimed at them. $this->setAdminUser(); $this->assertTrue(\tool_usertours\local\filter\role::filter_matches($tour, $context)); } + + /** + * Test the filter_matches function when multiple roles are set, including an admin user. + */ + public function test_filter_matches_multiple_role_including_admin() { + $context = \context_course::instance($this->course->id); + + $roles = [ + \tool_usertours\local\filter\role::ROLE_SITEADMIN, + $this->roles['teacher'], + $this->roles['editingteacher'], + ]; + + // Note: No need to persist this tour. + $tour = new \tool_usertours\tour(); + $tour->set_filter_values('role', $roles); + + // Note: The role filter does not use the context. + foreach ($this->testroles as $role) { + $this->setUser($this->$role); + if ($role === 'student') { + $this->assertFalse(\tool_usertours\local\filter\role::filter_matches($tour, $context)); + } else { + $this->assertTrue(\tool_usertours\local\filter\role::filter_matches($tour, $context)); + } + } + + // The admin can view this one because it's only aimed at them. + $this->setAdminUser(); + $this->assertTrue(\tool_usertours\local\filter\role::filter_matches($tour, $context)); + } + + /** + * Test the filter_matches function when an admin user has multiple roles. + */ + public function test_filter_matches_multiple_role_admin_user() { + global $USER; + + $context = \context_course::instance($this->course->id); + + $roles = [ + \tool_usertours\local\filter\role::ROLE_SITEADMIN, + ]; + + $this->setAdminUser(); + $this->getDataGenerator()->enrol_user($USER->id, $this->course->id, $this->roles['student']); + + // Note: No need to persist this tour. + $tour = new \tool_usertours\tour(); + $tour->set_filter_values('role', $roles); + + // The admin can view this one because it's only aimed at them. + $this->assertTrue(\tool_usertours\local\filter\role::filter_matches($tour, $context)); + } + + /** + * Test that the get_filter_options function does not include the guest roles. + */ + public function test_get_filter_options_no_guest_roles() { + create_role('Test Role', 'testrole', 'This is a test role', 'guest'); + + $allroles = role_get_names(null, ROLENAME_ALIAS); + $options = \tool_usertours\local\filter\role::get_filter_options(); + + foreach ($allroles as $role) { + $hasrole = isset($options[$role->shortname]); + if ($role->archetype === 'guest') { + $this->assertFalse($hasrole); + } else { + $this->assertTrue($hasrole); + } + } + } }