Skip to content

Commit 50c3e49

Browse files
prasanna-lmsaceabias
authored andcommitted
Improvement: Enhance smart menu restrictions for authenticated and guest roles, resolves #571 (#640)
1 parent 120574b commit 50c3e49

File tree

6 files changed

+72
-19
lines changed

6 files changed

+72
-19
lines changed

CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ Changes
66

77
### Unreleased
88

9+
* 2024-05-11 - Improvement: Enhance smart menu restrictions for authenticated user role, guest roles and visitor role, resolves #571
910
* 2024-05-11 - Improvement: Smart menu "locations" must be filled with a value, resolves #404
1011
* 2024-05-10 - Bugfix: Do not show empty smart menus to users, resolves #405
1112
* 2024-05-09 - Bugfix: Smart menu menubar overlaid course index, resolves #607

classes/form/smartmenu_edit_form.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,9 @@ public function definition() {
189189
$rolelist = role_get_names(\context_system::instance());
190190
$roleoptions = [];
191191
foreach ($rolelist as $role) {
192-
$roleoptions[$role->id] = $role->localname;
192+
if ($role->archetype !== 'frontpage') { // Frontpage roles are not supported in the menus restriction.
193+
$roleoptions[$role->id] = $role->localname;
194+
}
193195
}
194196
$byroleswidget = $mform->addElement('autocomplete', 'roles', get_string('smartmenusbyrole', 'theme_boost_union'),
195197
$roleoptions);

classes/form/smartmenu_item_edit_form.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,9 @@ public function definition() {
358358
$rolelist = role_get_names(\context_system::instance());
359359
$roleoptions = [];
360360
foreach ($rolelist as $role) {
361-
$roleoptions[$role->id] = $role->localname;
361+
if ($role->archetype !== 'frontpage') { // Frontpage roles are not supported in the items restriction.
362+
$roleoptions[$role->id] = $role->localname;
363+
}
362364
}
363365
$byroleswidget = $mform->addElement('autocomplete', 'roles', get_string('smartmenusbyrole', 'theme_boost_union'),
364366
$roleoptions);

smartmenus/menulib.php

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,33 @@ public function verify_access_restrictions() {
137137
* @return void
138138
*/
139139
public function restriction_byroles(&$query) {
140-
global $DB;
140+
global $DB, $CFG;
141141

142142
$roles = $this->data->roles;
143-
// Roles not mentioned then stop the role check.
143+
// If no role restrictions are set.
144144
if ($roles == '' || empty($roles)) {
145+
// Return directly.
146+
return true;
147+
}
148+
149+
// If the user is logged in and the default user role is allowed to view the menu.
150+
$defaultuserroleid = isset($CFG->defaultuserroleid) ? $CFG->defaultuserroleid : 0;
151+
if ($defaultuserroleid && in_array($defaultuserroleid, $roles) && !empty($this->userid) && !isguestuser($this->userid)) {
152+
// Return directly.
153+
return true;
154+
}
155+
156+
// If the user is a guest and the guest role is allowed to view the menu.
157+
$guestroleid = isset($CFG->guestroleid) ? $CFG->guestroleid : 0;
158+
if ($guestroleid && in_array($guestroleid, $roles) && isguestuser()) {
159+
// Return directly.
160+
return true;
161+
}
162+
163+
// If the user is a visitor and the visitor role is allowed to view the menu.
164+
$visitorroleid = isset($CFG->notloggedinroleid) ? $CFG->notloggedinroleid : 0;
165+
if ($visitorroleid && in_array($visitorroleid, $roles) && !isloggedin() && !isguestuser()) {
166+
// Return directly.
145167
return true;
146168
}
147169

@@ -156,7 +178,6 @@ public function restriction_byroles(&$query) {
156178
'systemcontext' => context_system::instance()->id,
157179
];
158180
$query->params += array_merge($params, $inparam);
159-
160181
}
161182

162183
/**

tests/behat/theme_boost_union_smartmenusettings_menuitems_rules.feature

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, app
5858
And the following "system role assigns" exist:
5959
| user | course | role |
6060
| systemmanager | Acceptance test site | manager |
61+
And the following "roles" exist:
62+
| name | shortname | description |
63+
| Visitor | visitor | My visitor role |
64+
And I navigate to "Users > Permissions > User policies" in site administration
65+
And I set the field "Role for visitors" to "Visitor (visitor)"
66+
And I press "Save changes"
6167
When I navigate to smart menus
6268
And I should see "Quick links" in the "smartmenus" "table"
6369
And I should see smart menu "Quick links" item "Resources" in location "Main, Menu, User, Bottom"
@@ -67,7 +73,7 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, app
6773
And I set the field "By role" to "<byrole>"
6874
And I set the field "Context" to "<context>"
6975
And I click on "Save changes" "button"
70-
And I should not see smart menu "Quick links" item "Resources" in location "Main, Menu, User, Bottom"
76+
And I <adminshouldorshouldnot> see smart menu "Quick links" item "Resources" in location "Main, Menu, User, Bottom"
7177
And I log out
7278
And I log in as "coursemanager"
7379
Then I <managershouldorshouldnot> see smart menu "Quick links" item "Resources" in location "Main, Menu, User, Bottom"
@@ -79,14 +85,21 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, app
7985
Then I <teachershouldorshouldnot> see smart menu "Quick links" item "Resources" in location "Main, Menu, User, Bottom"
8086
And I log out
8187
And I log in as "systemmanager"
82-
Then I should see smart menu "Quick links" item "Resources" in location "Main, Menu, User, Bottom"
88+
Then I <systemshouldorshouldnot> see smart menu "Quick links" item "Resources" in location "Main, Menu, User, Bottom"
89+
And I log in as "guest"
90+
Then I <guestshouldorshouldnot> see smart menu "Quick links" item "Resources" in location "Main, Menu, Bottom"
91+
And I log out
92+
And I <visitorshouldorshouldnot> see smart menu "Quick links" item "Resources" in location "Main, Menu, Bottom"
8393

8494
Examples:
85-
| byrole | context | student1shouldorshouldnot | teachershouldorshouldnot | managershouldorshouldnot |
86-
| Manager | Any | should not | should not | should |
87-
| Manager, Student | Any | should | should not | should |
88-
| Manager, Student, Teacher | Any | should | should | should |
89-
| Manager, Student, Teacher | System | should not | should not | should not |
95+
| byrole | context | student1shouldorshouldnot | teachershouldorshouldnot | managershouldorshouldnot | guestshouldorshouldnot | adminshouldorshouldnot | systemshouldorshouldnot | visitorshouldorshouldnot |
96+
| Manager | Any | should not | should not | should | should not | should not | should | should not |
97+
| Manager, Student | Any | should | should not | should | should not | should not | should | should not |
98+
| Manager, Student, Teacher | Any | should | should | should | should not | should not | should | should not |
99+
| Manager, Student, Teacher | System | should not | should not | should not | should not | should not | should | should not |
100+
| Authenticated user | Any | should | should | should | should not | should | should | should not |
101+
| Guest | Any | should not | should not | should not | should | should not | should not | should not |
102+
| Visitor | Any | should not | should not | should not | should not | should not | should not | should |
90103

91104
@javascript
92105
Scenario Outline: Smartmenu: Menu items: Rules - Show smart menu item based on the user assignment in single cohorts

tests/behat/theme_boost_union_smartmenusettings_menus_rules.feature

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, app
5353
And the following "system role assigns" exist:
5454
| user | course | role |
5555
| systemmanager | Acceptance test site | manager |
56+
And the following "roles" exist:
57+
| name | shortname | description |
58+
| Visitor | visitor | My visitor role |
59+
And I navigate to "Users > Permissions > User policies" in site administration
60+
And I set the field "Role for visitors" to "Visitor (visitor)"
61+
And I press "Save changes"
5662
When I navigate to smart menus
5763
And I should see "Quick links" in the "smartmenus" "table"
5864
And I should see smart menu "Quick links" in location "Main, Menu, User, Bottom"
@@ -61,7 +67,7 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, app
6167
And I set the field "By role" to "<byrole>"
6268
And I set the field "Context" to "<context>"
6369
And I click on "Save and return" "button"
64-
And I should not see smart menu "Quick links" in location "Main, Menu, User, Bottom"
70+
And I <adminshouldorshouldnot> see smart menu "Quick links" in location "Main, Menu, User, Bottom"
6571
And I log out
6672
And I log in as "coursemanager"
6773
Then I <managershouldorshouldnot> see smart menu "Quick links" in location "Main, Menu, User, Bottom"
@@ -73,14 +79,22 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, app
7379
Then I <teachershouldorshouldnot> see smart menu "Quick links" in location "Main, Menu, User, Bottom"
7480
And I log out
7581
And I log in as "systemmanager"
76-
Then I should see smart menu "Quick links" in location "Main, Menu, User, Bottom"
82+
Then I <systemshouldorshouldnot> see smart menu "Quick links" in location "Main, Menu, User, Bottom"
83+
And I log out
84+
And I log in as "guest"
85+
Then I <guestshouldorshouldnot> see smart menu "Quick links" in location "Main, Menu, Bottom"
86+
And I log out
87+
And I <visitorshouldorshouldnot> see smart menu "Quick links" in location "Main, Menu, Bottom"
7788

7889
Examples:
79-
| byrole | context | student1shouldorshouldnot | teachershouldorshouldnot | managershouldorshouldnot |
80-
| Manager | Any | should not | should not | should |
81-
| Manager, Student | Any | should | should not | should |
82-
| Manager, Student, Teacher | Any | should | should | should |
83-
| Manager, Student, Teacher | System | should not | should not | should not |
90+
| byrole | context | student1shouldorshouldnot | teachershouldorshouldnot | managershouldorshouldnot | guestshouldorshouldnot | adminshouldorshouldnot | systemshouldorshouldnot | visitorshouldorshouldnot |
91+
| Manager | Any | should not | should not | should | should not | should not | should | should not |
92+
| Manager, Student | Any | should | should not | should | should not | should not | should | should not |
93+
| Manager, Student, Teacher | Any | should | should | should | should not | should not | should | should not |
94+
| Manager, Student, Teacher | System | should not | should not | should not | should not | should not | should | should not |
95+
| Authenticated user | Any | should | should | should | should not | should | should | should not |
96+
| Guest | Any | should not | should not | should not | should | should not | should not | should not |
97+
| Visitor | Any | should not | should not | should not | should not | should not | should not | should |
8498

8599
@javascript
86100
Scenario Outline: Smartmenu: Menus: Rules - Show smart menu based on the user assignment in single cohorts

0 commit comments

Comments
 (0)