diff --git a/CHANGES.md b/CHANGES.md index 1d6ec9600c8..a68bbca9024 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,7 @@ Changes ### Unreleased +* 2024-05-11 - Improvement: Enhance smart menu restrictions for authenticated user role, guest roles and visitor role, resolves #571 * 2024-05-11 - Improvement: Smart menu "locations" must be filled with a value, resolves #404 * 2024-05-10 - Bugfix: Do not show empty smart menus to users, resolves #405 * 2024-05-09 - Bugfix: Smart menu menubar overlaid course index, resolves #607 diff --git a/smartmenus/menulib.php b/smartmenus/menulib.php index 51c79884751..3eef818e154 100644 --- a/smartmenus/menulib.php +++ b/smartmenus/menulib.php @@ -140,24 +140,31 @@ public function restriction_byroles(&$query) { global $DB, $CFG; $roles = $this->data->roles; - // Roles not mentioned then stop the role check. + // If no role restrictions are set. if ($roles == '' || empty($roles)) { + // Return directly. return true; } - // Verify the default user role is set to view the menus. + // If the user is logged in and the default user role is allowed to view the menu. $defaultuserroleid = isset($CFG->defaultuserroleid) ? $CFG->defaultuserroleid : 0; if ($defaultuserroleid && in_array($defaultuserroleid, $roles) && !empty($this->userid) && !isguestuser($this->userid)) { + // Return directly. return true; } - // Verify the guest user have any menus or items to view. - if (isguestuser()) { - $guestroles = get_archetype_roles('guest'); - $guestroleid = array_column($guestroles, 'id'); - if (array_intersect($guestroleid, $roles)) { - return true; - } + // If the user is a guest and the guest role is allowed to view the menu. + $guestroleid = isset($CFG->guestroleid) ? $CFG->guestroleid : 0; + if ($guestroleid && in_array($guestroleid, $roles) && isguestuser()) { + // Return directly. + return true; + } + + // If the user is a visitor and the visitor role is allowed to view the menu. + $visitorroleid = isset($CFG->notloggedinroleid) ? $CFG->notloggedinroleid : 0; + if ($visitorroleid && in_array($visitorroleid, $roles) && !isloggedin() && !isguestuser()) { + // Return directly. + return true; } list($insql, $inparam) = $DB->get_in_or_equal($roles, SQL_PARAMS_NAMED, 'rl'); diff --git a/tests/behat/theme_boost_union_smartmenusettings_menuitems_rules.feature b/tests/behat/theme_boost_union_smartmenusettings_menuitems_rules.feature index 4d2b3da48b0..9abd82f8a89 100644 --- a/tests/behat/theme_boost_union_smartmenusettings_menuitems_rules.feature +++ b/tests/behat/theme_boost_union_smartmenusettings_menuitems_rules.feature @@ -58,6 +58,12 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, app And the following "system role assigns" exist: | user | course | role | | systemmanager | Acceptance test site | manager | + And the following "roles" exist: + | name | shortname | description | + | Visitor | visitor | My visitor role | + And I navigate to "Users > Permissions > User policies" in site administration + And I set the field "Role for visitors" to "Visitor (visitor)" + And I press "Save changes" When I navigate to smart menus And I should see "Quick links" in the "smartmenus" "table" And I should see smart menu "Quick links" item "Resources" in location "Main, Menu, User, Bottom" @@ -83,16 +89,17 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, app And I log in as "guest" Then I see smart menu "Quick links" item "Resources" in location "Main, Menu, Bottom" And I log out - And I should not see smart menu "Quick links" item "Resources" in location "Main, Menu, Bottom" + And I see smart menu "Quick links" item "Resources" in location "Main, Menu, Bottom" Examples: - | byrole | context | student1shouldorshouldnot | teachershouldorshouldnot | managershouldorshouldnot | guestshouldorshouldnot | adminshouldorshouldnot | systemshouldorshouldnot | - | Manager | Any | should not | should not | should | should not | should not | should | - | Manager, Student | Any | should | should not | should | should not | should not | should | - | Manager, Student, Teacher | Any | should | should | should | should not | should not | should | - | Manager, Student, Teacher | System | should not | should not | should not | should not | should not | should | - | Authenticated user | Any | should | should | should | should not | should | should | - | Guest | Any | should not | should not | should not | should | should not | should not | + | byrole | context | student1shouldorshouldnot | teachershouldorshouldnot | managershouldorshouldnot | guestshouldorshouldnot | adminshouldorshouldnot | systemshouldorshouldnot | visitorshouldorshouldnot | + | Manager | Any | should not | should not | should | should not | should not | should | should not | + | Manager, Student | Any | should | should not | should | should not | should not | should | should not | + | Manager, Student, Teacher | Any | should | should | should | should not | should not | should | should not | + | Manager, Student, Teacher | System | should not | should not | should not | should not | should not | should | should not | + | Authenticated user | Any | should | should | should | should not | should | should | should not | + | Guest | Any | should not | should not | should not | should | should not | should not | should not | + | Visitor | Any | should not | should not | should not | should not | should not | should not | should | @javascript Scenario Outline: Smartmenu: Menu items: Rules - Show smart menu item based on the user assignment in single cohorts diff --git a/tests/behat/theme_boost_union_smartmenusettings_menus_rules.feature b/tests/behat/theme_boost_union_smartmenusettings_menus_rules.feature index eb8607c8cfd..30dc4a17b88 100644 --- a/tests/behat/theme_boost_union_smartmenusettings_menus_rules.feature +++ b/tests/behat/theme_boost_union_smartmenusettings_menus_rules.feature @@ -53,6 +53,12 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, app And the following "system role assigns" exist: | user | course | role | | systemmanager | Acceptance test site | manager | + And the following "roles" exist: + | name | shortname | description | + | Visitor | visitor | My visitor role | + And I navigate to "Users > Permissions > User policies" in site administration + And I set the field "Role for visitors" to "Visitor (visitor)" + And I press "Save changes" When I navigate to smart menus And I should see "Quick links" in the "smartmenus" "table" And I should see smart menu "Quick links" in location "Main, Menu, User, Bottom" @@ -78,16 +84,17 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, app And I log in as "guest" Then I see smart menu "Quick links" in location "Main, Menu, Bottom" And I log out - And I should not see smart menu "Quick links" in location "Main, Menu, Bottom" + And I see smart menu "Quick links" in location "Main, Menu, Bottom" Examples: - | byrole | context | student1shouldorshouldnot | teachershouldorshouldnot | managershouldorshouldnot | guestshouldorshouldnot | adminshouldorshouldnot | systemshouldorshouldnot | - | Manager | Any | should not | should not | should | should not | should not | should | - | Manager, Student | Any | should | should not | should | should not | should not | should | - | Manager, Student, Teacher | Any | should | should | should | should not | should not | should | - | Manager, Student, Teacher | System | should not | should not | should not | should not | should not | should | - | Authenticated user | Any | should | should | should | should not | should | should | - | Guest | Any | should not | should not | should not | should | should not | should not | + | byrole | context | student1shouldorshouldnot | teachershouldorshouldnot | managershouldorshouldnot | guestshouldorshouldnot | adminshouldorshouldnot | systemshouldorshouldnot | visitorshouldorshouldnot | + | Manager | Any | should not | should not | should | should not | should not | should | should not | + | Manager, Student | Any | should | should not | should | should not | should not | should | should not | + | Manager, Student, Teacher | Any | should | should | should | should not | should not | should | should not | + | Manager, Student, Teacher | System | should not | should not | should not | should not | should not | should | should not | + | Authenticated user | Any | should | should | should | should not | should | should | should not | + | Guest | Any | should not | should not | should not | should | should not | should not | should not | + | Visitor | Any | should not | should not | should not | should not | should not | should not | should | @javascript Scenario Outline: Smartmenu: Menus: Rules - Show smart menu based on the user assignment in single cohorts