Skip to content

Commit

Permalink
Review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@abias
abias committed May 11, 2024
1 parent f541779 commit 4335db3
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 25 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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-09 - Bugfix: Smart menu menubar overlaid course index, resolves #607
* 2024-04-27 - Improvement: Add navigation to policy overview page, resolves #633

Expand Down
25 changes: 16 additions & 9 deletions smartmenus/menulib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
23 changes: 15 additions & 8 deletions tests/behat/theme_boost_union_smartmenusettings_menuitems_rules.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" item "Resources" in location "Main, Menu, User, Bottom"
Expand All @@ -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 <guestshouldorshouldnot> 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 <visitorshouldorshouldnot> 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
Expand Down
23 changes: 15 additions & 8 deletions tests/behat/theme_boost_union_smartmenusettings_menus_rules.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 <guestshouldorshouldnot> 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 <visitorshouldorshouldnot> 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
Expand Down

0 comments on commit 4335db3

Please sign in to comment.