diff --git a/admin/roles/allow.php b/admin/roles/allow.php index 916e6c2b0c2..71e9bcb3d0a 100644 --- a/admin/roles/allow.php +++ b/admin/roles/allow.php @@ -29,7 +29,8 @@ $mode = required_param('mode', PARAM_ALPHANUMEXT); $classformode = array( 'assign' => 'core_role_allow_assign_page', 'override' => 'core_role_allow_override_page', - 'switch' => 'core_role_allow_switch_page' + 'switch' => 'core_role_allow_switch_page', + 'view' => 'core_role_allow_view_page' ); if (!isset($classformode[$mode])) { print_error('invalidmode', '', '', $mode); @@ -58,6 +59,9 @@ if (optional_param('submit', false, PARAM_BOOL) && data_submitted() && confirm_s case 'switch': $event = \core\event\role_allow_switch_updated::create(array('context' => $syscontext)); break; + case 'view': + $event = \core\event\role_allow_view_updated::create(array('context' => $syscontext)); + break; } if ($event) { $event->trigger(); diff --git a/admin/roles/classes/allow_view_page.php b/admin/roles/classes/allow_view_page.php new file mode 100644 index 00000000000..2899fc631b5 --- /dev/null +++ b/admin/roles/classes/allow_view_page.php @@ -0,0 +1,77 @@ +. + +/** + * Role view matrix. + * + * @package core_role + * @copyright 2016 onwards Andrew Hancox + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Subclass of role_allow_role_page for the Allow views tab. + * + * @package core_role + * @copyright 2016 onwards Andrew Hancox + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_role_allow_view_page extends core_role_allow_role_page { + /** @var array */ + protected $allowedtargetroles; + + /** + * core_role_allow_view_page constructor. + */ + public function __construct() { + parent::__construct('role_allow_view', 'allowview'); + } + + + /** + * Allow from role to view target role. + * @param int $fromroleid + * @param int $targetroleid + */ + protected function set_allow($fromroleid, $targetroleid) { + allow_view($fromroleid, $targetroleid); + } + + /** + * Get tool tip for cell. + * @param string $fromrole + * @param string $targetrole + * @return string + * @throws \coding_exception + */ + protected function get_cell_tooltip($fromrole, $targetrole) { + $a = new stdClass; + $a->fromrole = $fromrole->localname; + $a->targetrole = $targetrole->localname; + return get_string('allowroletoview', 'core_role', $a); + } + + /** + * Get intro text for role allow view page. + * @return string + * @throws \coding_exception + */ + public function get_intro_text() { + return get_string('configallowview', 'core_admin'); + } +} diff --git a/admin/roles/classes/define_role_table_advanced.php b/admin/roles/classes/define_role_table_advanced.php index bcf6a495d1f..36cce176d4f 100644 --- a/admin/roles/classes/define_role_table_advanced.php +++ b/admin/roles/classes/define_role_table_advanced.php @@ -42,6 +42,7 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi protected $allowassign; protected $allowoverride; protected $allowswitch; + protected $allowview; public function __construct($context, $roleid) { $this->roleid = $roleid; @@ -72,6 +73,7 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi $this->allowassign = array_keys($this->get_allow_roles_list('assign')); $this->allowoverride = array_keys($this->get_allow_roles_list('override')); $this->allowswitch = array_keys($this->get_allow_roles_list('switch')); + $this->allowview = array_keys($this->get_allow_roles_list('view')); } else { $this->role = new stdClass; @@ -83,6 +85,7 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi $this->allowassign = array(); $this->allowoverride = array(); $this->allowswitch = array(); + $this->allowview = array(); } parent::load_current_permissions(); } @@ -162,6 +165,10 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi if (!is_null($allow)) { $this->allowswitch = $allow; } + $allow = optional_param_array('allowview', null, PARAM_INT); + if (!is_null($allow)) { + $this->allowview = $allow; + } // Now read the permissions for each capability. parent::read_submitted_permissions(); @@ -178,7 +185,8 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi * @param int $roleid role id or 0 for no role * @param array $options array with following keys: * 'name', 'shortname', 'description', 'permissions', 'archetype', - * 'contextlevels', 'allowassign', 'allowoverride', 'allowswitch' + * 'contextlevels', 'allowassign', 'allowoverride', 'allowswitch', + * 'allowview' */ public function force_duplicate($roleid, array $options) { global $DB; @@ -215,6 +223,9 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi if ($options['allowswitch']) { $this->allowswitch = array(); } + if ($options['allowview']) { + $this->allowview = array(); + } if ($options['permissions']) { foreach ($this->capabilities as $capid => $cap) { @@ -260,6 +271,9 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi if ($options['allowswitch']) { $this->allowswitch = array_keys($this->get_allow_roles_list('switch', $roleid)); } + if ($options['allowview']) { + $this->allowview = array_keys($this->get_allow_roles_list('view', $roleid)); + } if ($options['permissions']) { $this->permissions = $DB->get_records_menu('role_capabilities', @@ -280,7 +294,8 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi * @param string $archetype * @param array $options array with following keys: * 'name', 'shortname', 'description', 'permissions', 'archetype', - * 'contextlevels', 'allowassign', 'allowoverride', 'allowswitch' + * 'contextlevels', 'allowassign', 'allowoverride', 'allowswitch', + * 'allowview' */ public function force_archetype($archetype, array $options) { $archetypes = get_role_archetypes(); @@ -321,6 +336,9 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi if ($options['allowswitch']) { $this->allowswitch = get_default_role_archetype_allows('switch', $archetype); } + if ($options['allowview']) { + $this->allowview = get_default_role_archetype_allows('view', $archetype); + } if ($options['permissions']) { $defaultpermissions = get_default_capabilities($archetype); @@ -340,7 +358,8 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi * @param string $xml * @param array $options array with following keys: * 'name', 'shortname', 'description', 'permissions', 'archetype', - * 'contextlevels', 'allowassign', 'allowoverride', 'allowswitch' + * 'contextlevels', 'allowassign', 'allowoverride', 'allowswitch', + * 'allowview' */ public function force_preset($xml, array $options) { if (!$info = core_role_preset::parse_preset($xml)) { @@ -377,7 +396,7 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi } } - foreach (array('assign', 'override', 'switch') as $type) { + foreach (array('assign', 'override', 'switch', 'view') as $type) { if ($options['allow'.$type]) { if (isset($info['allow'.$type])) { $this->{'allow'.$type} = $info['allow'.$type]; @@ -438,6 +457,7 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi $this->save_allow('assign'); $this->save_allow('override'); $this->save_allow('switch'); + $this->save_allow('view'); // Permissions. parent::save_changes(); @@ -523,7 +543,7 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi protected function get_allow_roles_list($type, $roleid = null) { global $DB; - if ($type !== 'assign' and $type !== 'switch' and $type !== 'override') { + if ($type !== 'assign' and $type !== 'switch' and $type !== 'override' and $type !== 'view') { debugging('Invalid role allowed type specified', DEBUG_DEVELOPER); return array(); } @@ -547,11 +567,11 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi /** * Returns an array of roles with the allowed type. * - * @param string $type Must be one of: assign, switch, or override. + * @param string $type Must be one of: assign, switch, override or view. * @return array Am array of role names with the allowed type */ protected function get_allow_role_control($type) { - if ($type !== 'assign' and $type !== 'switch' and $type !== 'override') { + if ($type !== 'assign' and $type !== 'switch' and $type !== 'override' and $type !== 'view') { debugging('Invalid role allowed type specified', DEBUG_DEVELOPER); return ''; } @@ -641,6 +661,7 @@ class core_role_define_role_table_advanced extends core_role_capability_table_wi $this->print_field('menuallowassign', get_string('allowassign', 'core_role'), $this->get_allow_role_control('assign')); $this->print_field('menuallowoverride', get_string('allowoverride', 'core_role'), $this->get_allow_role_control('override')); $this->print_field('menuallowswitch', get_string('allowswitch', 'core_role'), $this->get_allow_role_control('switch')); + $this->print_field('menuallowview', get_string('allowview', 'core_role'), $this->get_allow_role_control('view')); if ($risks = $this->get_role_risks_info()) { $this->print_field('', get_string('rolerisks', 'core_role'), $risks); } diff --git a/admin/roles/classes/preset.php b/admin/roles/classes/preset.php index 256f635d5ba..b99434255ec 100644 --- a/admin/roles/classes/preset.php +++ b/admin/roles/classes/preset.php @@ -84,7 +84,7 @@ class core_role_preset { $contextlevels->appendChild($dom->createElement('level', $name)); } - foreach (array('assign', 'override', 'switch') as $type) { + foreach (array('assign', 'override', 'switch', 'view') as $type) { $allows = $dom->createElement('allow'.$type); $top->appendChild($allows); $records = $DB->get_records('role_allow_'.$type, array('roleid'=>$roleid), "allow$type ASC"); @@ -205,7 +205,7 @@ class core_role_preset { } } - foreach (array('assign', 'override', 'switch') as $type) { + foreach (array('assign', 'override', 'switch', 'view') as $type) { $values = self::get_node_children_values($dom, '/role/allow'.$type, 'shortname'); if (!isset($values)) { $info['allow'.$type] = null; diff --git a/admin/roles/classes/preset_form.php b/admin/roles/classes/preset_form.php index ff2024d7d9c..426c3d8d38f 100644 --- a/admin/roles/classes/preset_form.php +++ b/admin/roles/classes/preset_form.php @@ -79,6 +79,7 @@ class core_role_preset_form extends moodleform { $mform->addElement('advcheckbox', 'allowassign', get_string('allowassign', 'core_role')); $mform->addElement('advcheckbox', 'allowoverride', get_string('allowoverride', 'core_role')); $mform->addElement('advcheckbox', 'allowswitch', get_string('allowswitch', 'core_role')); + $mform->addElement('advcheckbox', 'allowview', get_string('allowview', 'core_role')); $mform->addElement('advcheckbox', 'permissions', get_string('permissions', 'core_role')); } diff --git a/admin/roles/define.php b/admin/roles/define.php index d670bae45d8..de3eac1b1c4 100644 --- a/admin/roles/define.php +++ b/admin/roles/define.php @@ -103,7 +103,8 @@ if ($action === 'add' and $resettype !== 'none') { 'contextlevels' => 1, 'allowassign' => 1, 'allowoverride' => 1, - 'allowswitch' => 1); + 'allowswitch' => 1, + 'allowview' => 1); if ($showadvanced) { $definitiontable = new core_role_define_role_table_advanced($systemcontext, 0); } else { @@ -150,7 +151,8 @@ if ($action === 'add' and $resettype !== 'none') { 'contextlevels' => $data->contextlevels, 'allowassign' => $data->allowassign, 'allowoverride' => $data->allowoverride, - 'allowswitch' => $data->allowswitch); + 'allowswitch' => $data->allowswitch, + 'allowview' => $data->allowview); if ($showadvanced) { $definitiontable = new core_role_define_role_table_advanced($systemcontext, $roleid); } else { diff --git a/admin/roles/managetabs.php b/admin/roles/managetabs.php index 78f9f7532db..a9a8385a13d 100644 --- a/admin/roles/managetabs.php +++ b/admin/roles/managetabs.php @@ -29,6 +29,7 @@ $toprow[] = new tabobject('manage', new moodle_url('/admin/roles/manage.php'), g $toprow[] = new tabobject('assign', new moodle_url('/admin/roles/allow.php', array('mode'=>'assign')), get_string('allowassign', 'core_role')); $toprow[] = new tabobject('override', new moodle_url('/admin/roles/allow.php', array('mode'=>'override')), get_string('allowoverride', 'core_role')); $toprow[] = new tabobject('switch', new moodle_url('/admin/roles/allow.php', array('mode'=>'switch')), get_string('allowswitch', 'core_role')); +$toprow[] = new tabobject('view', new moodle_url('/admin/roles/allow.php', ['mode' => 'view']), get_string('allowview', 'core_role')); echo $OUTPUT->tabtree($toprow, $currenttab); diff --git a/admin/roles/role_schema.xml b/admin/roles/role_schema.xml index f0a33c9ecc0..bb3de8976b7 100644 --- a/admin/roles/role_schema.xml +++ b/admin/roles/role_schema.xml @@ -11,6 +11,7 @@ + @@ -45,6 +46,13 @@ + + + + + + + diff --git a/admin/roles/tests/preset_test.php b/admin/roles/tests/preset_test.php index 08994800314..4ab0ebc35ed 100644 --- a/admin/roles/tests/preset_test.php +++ b/admin/roles/tests/preset_test.php @@ -44,7 +44,7 @@ class core_role_preset_testcase extends advanced_testcase { $contextlevels = get_role_contextlevels($role->id); $this->assertEquals(array_values($contextlevels), array_values($info['contextlevels'])); - foreach (array('assign', 'override', 'switch') as $type) { + foreach (array('assign', 'override', 'switch', 'view') as $type) { $records = $DB->get_records('role_allow_'.$type, array('roleid'=>$role->id), "allow$type ASC"); $allows = array(); foreach ($records as $record) { diff --git a/badges/criteria/award_criteria_manual.php b/badges/criteria/award_criteria_manual.php index 83fde27038a..ff1e613d6d9 100644 --- a/badges/criteria/award_criteria_manual.php +++ b/badges/criteria/award_criteria_manual.php @@ -65,6 +65,7 @@ class award_criteria_manual extends award_criteria { $none = true; $roles = get_roles_with_capability('moodle/badges:awardbadge', CAP_ALLOW, $PAGE->context); + $visibleroles = get_viewable_roles($PAGE->context); $roleids = array_map(function($o) { return $o->id; }, $roles); @@ -89,6 +90,9 @@ class award_criteria_manual extends award_criteria { $mform->addElement('header', 'first_header', $this->get_title()); $mform->addHelpButton('first_header', 'criteria_' . $this->criteriatype, 'badges'); foreach ($roleids as $rid) { + if (!key_exists($rid, $visibleroles)) { + continue; + } $checked = false; if (in_array($rid, $existing)) { $checked = true; diff --git a/badges/tests/behat/role_visibility.feature b/badges/tests/behat/role_visibility.feature new file mode 100644 index 00000000000..5bb0a73543b --- /dev/null +++ b/badges/tests/behat/role_visibility.feature @@ -0,0 +1,51 @@ +@core @core_badges @role_visibility +Feature: Test role visibility + In order to control access + As an admin + I need to control which roles can see each other + + Background: Add a bunch of users + Given the following "courses" exist: + | fullname | shortname | + | Course 1 | C1 | + And the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | manager1 | Manager | 1 | manager1@example.com | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | manager1 | C1 | manager | + + @javascript @_file_upload + Scenario: Check the default roles are visible + Given I log in as "manager1" + And I am on "Course 1" course homepage + And I navigate to "Add a new badge" node in "Course administration > Badges" + And I follow "Add a new badge" + And I set the following fields to these values: + | Name | Course Badge | + | Description | Course badge description | + | issuername | Tester of course badge | + And I upload "badges/tests/behat/badge.png" file to "Image" filemanager + And I press "Create badge" + And I set the field "type" to "Manual issue by role" + Then I should see "Teacher" + And I should see "Manager" + + @javascript @_file_upload + Scenario: Check hidden roles are not visible + Given I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "Add a new badge" node in "Course administration > Badges" + And I follow "Add a new badge" + And I set the following fields to these values: + | Name | Course Badge | + | Description | Course badge description | + | issuername | Tester of course badge | + And I upload "badges/tests/behat/badge.png" file to "Image" filemanager + And I press "Create badge" + And I set the following fields to these values: + | Add badge criteria | Manual issue by role | + Then I should see "Teacher" + And I should not see "Manager" \ No newline at end of file diff --git a/enrol/locallib.php b/enrol/locallib.php index 75564193163..0447951ff4f 100644 --- a/enrol/locallib.php +++ b/enrol/locallib.php @@ -117,6 +117,7 @@ class course_enrolment_manager { private $_plugins = null; private $_allplugins = null; private $_roles = null; + private $_visibleroles = null; private $_assignableroles = null; private $_assignablerolesothers = null; private $_groups = null; @@ -593,6 +594,18 @@ class course_enrolment_manager { return $this->_roles; } + /** + * Gets all of the roles this course can contain. + * + * @return array + */ + public function get_viewable_roles() { + if ($this->_visibleroles === null) { + $this->_visibleroles = get_viewable_roles($this->context); + } + return $this->_visibleroles; + } + /** * Gets all of the assignable roles for this course. * @@ -1032,7 +1045,7 @@ class course_enrolment_manager { $strunenrol = get_string('unenrol', 'enrol'); $stredit = get_string('edit'); - $allroles = $this->get_all_roles(); + $visibleroles = $this->get_viewable_roles(); $assignable = $this->get_assignable_roles(); $allgroups = $this->get_all_groups(); $context = $this->get_context(); @@ -1054,7 +1067,15 @@ class course_enrolment_manager { if (!is_siteadmin() and !isset($assignable[$rid])) { $unchangeable = true; } - $details['roles'][$rid] = array('text'=>$allroles[$rid]->localname, 'unchangeable'=>$unchangeable); + + if (isset($visibleroles[$rid])) { + $label = $visibleroles[$rid]; + } else { + $label = get_string('novisibleroles', 'role'); + $unchangeable = true; + } + + $details['roles'][$rid] = array('text' => $label, 'unchangeable' => $unchangeable); } // Users diff --git a/enrol/tests/behat/role_visibility.feature b/enrol/tests/behat/role_visibility.feature new file mode 100644 index 00000000000..8a482b7443c --- /dev/null +++ b/enrol/tests/behat/role_visibility.feature @@ -0,0 +1,38 @@ +@core @core_enrol @role_visibility +Feature: Test role visibility + In order to control access + As an admin + I need to control which roles can see each other + + Background: Add a bunch of users + Given the following "courses" exist: + | fullname | shortname | + | Course 1 | C1 | + And the following "users" exist: + | username | firstname | lastname | email | + | learner1 | Learner | 1 | learner1@example.com | + | teacher1 | Teacher | 1 | teacher1@example.com | + | manager1 | Manager | 1 | manager1@example.com | + And the following "course enrolments" exist: + | user | course | role | + | learner1 | C1 | student | + | teacher1 | C1 | editingteacher | + | manager1 | C1 | manager | + + Scenario: Check the default roles are visible + Given I log in as "manager1" + And I follow "Course 1" + When I navigate to "Enrolled users" node in "Course administration > Users" + Then "Learner 1" row "Roles" column of "participants" table should contain "Student" + And "Teacher 1" row "Roles" column of "participants" table should contain "Teacher" + And "Manager 1" row "Roles" column of "participants" table should contain "Manager" + And I should not see "No Roles" in the "table#participants" "css_element" + + Scenario: Do not allow managers to view any roles but manager and check they are hidden + Given I log in as "teacher1" + And I am on "Course 1" course homepage + When I navigate to "Enrolled users" node in "Course administration > Users" + Then "Learner 1" row "Roles" column of "participants" table should contain "Student" + And "Teacher 1" row "Roles" column of "participants" table should contain "Teacher" + And "Manager 1" row "Roles" column of "participants" table should not contain "Manager" + And "Manager 1" row "Roles" column of "participants" table should contain "No roles" \ No newline at end of file diff --git a/enrol/users_forms.php b/enrol/users_forms.php index 97b62b73655..aa3ada707b9 100644 --- a/enrol/users_forms.php +++ b/enrol/users_forms.php @@ -156,10 +156,10 @@ class enrol_users_filter_form extends moodleform { // names if applied. The reason for not restricting to roles that can // be assigned at course level is that upper-level roles display in the // enrolments table so it makes sense to let users filter by them. - $allroles = $manager->get_all_roles(); + $visibleroles = $manager->get_viewable_roles(); $rolenames = array(); - foreach ($allroles as $id => $role) { - $rolenames[$id] = $role->localname; + foreach ($visibleroles as $id => $role) { + $rolenames[$id] = $role; } $mform->addElement('select', 'role', get_string('role'), array(0 => get_string('all')) + $rolenames); diff --git a/group/lib.php b/group/lib.php index 57bd61610da..ca774c1704c 100644 --- a/group/lib.php +++ b/group/lib.php @@ -978,6 +978,7 @@ function groups_calculate_role_people($rs, $context) { } $allroles = role_fix_names(get_all_roles($context), $context); + $visibleroles = get_viewable_roles($context); // Array of all involved roles $roles = array(); @@ -1036,14 +1037,15 @@ function groups_calculate_role_people($rs, $context) { // Now we rearrange the data to store users by role foreach ($users as $userid=>$userdata) { - $rolecount = count($userdata->roles); + $visibleuserroles = array_intersect_key($userdata->roles, $visibleroles); + $rolecount = count($visibleuserroles); if ($rolecount == 0) { // does not have any roles $roleid = 0; } else if($rolecount > 1) { $roleid = '*'; } else { - $userrole = reset($userdata->roles); + $userrole = reset($visibleuserroles); $roleid = $userrole->id; } $roles[$roleid]->users[$userid] = $userdata; diff --git a/group/tests/behat/role_visibility.feature b/group/tests/behat/role_visibility.feature new file mode 100644 index 00000000000..f21d2acd27b --- /dev/null +++ b/group/tests/behat/role_visibility.feature @@ -0,0 +1,53 @@ +@core @core_group @role_visibility +Feature: Test role visibility + In order to control access + As an admin + I need to control which roles can see each other + + Background: Set up some groups + Given the following "courses" exist: + | fullname | shortname | + | Course 1 | C1 | + And the following "users" exist: + | username | firstname | lastname | email | + | learner1 | Learner | 1 | learner1@example.com | + | teacher1 | Teacher | 1 | teacher1@example.com | + | manager1 | Manager | 1 | manager1@example.com | + And the following "course enrolments" exist: + | user | course | role | + | learner1 | C1 | student | + | teacher1 | C1 | editingteacher | + | manager1 | C1 | manager | + And the following "groups" exist: + | name | course | idnumber | + | Group 1 | C1 | G1 | + And the following "group members" exist: + | user | group | + | learner1 | G1 | + | teacher1 | G1 | + | manager1 | G1 | + + @javascript + Scenario: Check the default roles are visible + Given I log in as "manager1" + And I am on "Course 1" course homepage + And I navigate to "Groups" node in "Course administration > Users" + When I set the field "groups" to "Group 1 (3)" + And I press "Show members for group" + Then "optgroup[label='No roles']" "css_element" should not exist in the "#members" "css_element" + And "optgroup[label='Student']" "css_element" should exist in the "#members" "css_element" + And "optgroup[label='Teacher']" "css_element" should exist in the "#members" "css_element" + And "optgroup[label='Manager']" "css_element" should exist in the "#members" "css_element" + And I log out + + @javascript + Scenario: Do not allow managers to view any roles and check they are hidden + Given I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "Groups" node in "Course administration > Users" + When I set the field "groups" to "Group 1 (3)" + Then "optgroup[label='No roles']" "css_element" should exist in the "#members" "css_element" + And "optgroup[label='Student']" "css_element" should exist in the "#members" "css_element" + And "optgroup[label='Teacher']" "css_element" should exist in the "#members" "css_element" + And "optgroup[label='Manager']" "css_element" should not exist in the "#members" "css_element" + And I log out \ No newline at end of file diff --git a/lang/en/admin.php b/lang/en/admin.php index 69291ba0621..01ee8b7e728 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -154,6 +154,7 @@ $string['configallowusermailcharset'] = 'If enabled, users can choose an email c $string['configallowuserswitchrolestheycantassign'] = 'By default, moodle/role:assign is required for users to switch roles. Enabling this setting removes this requirement, and results in the roles available in the "Switch role to" dropdown menu being determined by settings in the "Allow role assignments" table only. It is recommended that the settings in the "Allow role assignments" table do not allow users to switch to a role with more capabilities than their existing role.'; $string['configallowuserthemes'] = 'If you enable this, then users will be allowed to set their own themes. User themes override site themes (but not course themes)'; +$string['configallowview'] = 'Select which roles a user will see, be able to filter by etc. based on which roles they already have.'; $string['configallusersaresitestudents'] = 'For activities on the front page of the site, should ALL users be considered as students? If you answer "Yes", then any confirmed user account will be allowed to participate as a student in those activities. If you answer "No", then only users who are already a participant in at least one course will be able to take part in those front page activities. Only admins and specially assigned teachers can act as teachers for these front page activities.'; $string['configauthenticationplugins'] = 'Please choose the authentication plugins you wish to use and arrange them in order of failthrough.'; $string['configautolang'] = 'Detect default language from browser setting, if disabled site default is used.'; diff --git a/lang/en/role.php b/lang/en/role.php index f1052255f48..8c8072ec82e 100644 --- a/lang/en/role.php +++ b/lang/en/role.php @@ -32,7 +32,9 @@ $string['allowoverride'] = 'Allow role overrides'; $string['allowroletoassign'] = 'Allow users with role {$a->fromrole} to assign the role {$a->targetrole}'; $string['allowroletooverride'] = 'Allow users with role {$a->fromrole} to override the role {$a->targetrole}'; $string['allowroletoswitch'] = 'Allow users with role {$a->fromrole} to switch roles to the role {$a->targetrole}'; +$string['allowroletoview'] = 'Allow users with role {$a->fromrole} to view the role {$a->targetrole}'; $string['allowswitch'] = 'Allow role switches'; +$string['allowview'] = 'Allow role to view'; $string['allsiteusers'] = 'All site users'; $string['analytics:listinsights'] = 'List insights'; $string['analytics:managemodels'] = 'Manage models'; @@ -219,6 +221,7 @@ $string['errorexistsroleshortname'] = 'Role name already exists'; $string['eventroleallowassignupdated'] = 'Allow role assignment'; $string['eventroleallowoverrideupdated'] = 'Allow role override'; $string['eventroleallowswitchupdated'] = 'Allow role switch'; +$string['eventroleallowviewupdated'] = 'Allow role view'; $string['eventroleassigned'] = 'Role assigned'; $string['eventrolecapabilitiesupdated'] = 'Role capabilities updated'; $string['eventroledeleted'] = 'Role deleted'; @@ -292,6 +295,7 @@ $string['noneinthisxmatching'] = 'No users matching \'{$a->search}\' in this {$a $string['norole'] = 'No role'; $string['noroles'] = 'No roles'; $string['noroleassignments'] = 'This user does not have any role assignments anywhere in this site.'; +$string['novisibleroles'] = 'No roles'; $string['notabletoassignroleshere'] = 'Assigning of roles in this context has not been enabled by an administrator.'; $string['notabletooverrideroleshere'] = 'You are not able to override the permissions on any roles here'; $string['notes:manage'] = 'Manage notes'; diff --git a/lib/accesslib.php b/lib/accesslib.php index 856a80924b9..a9f1d19cfe2 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -1984,7 +1984,7 @@ function get_default_capabilities($archetype) { * Return default roles that can be assigned, overridden or switched * by give role archetype. * - * @param string $type assign|override|switch + * @param string $type assign|override|switch|view * @param string $archetype * @return array of role ids */ @@ -2034,6 +2034,16 @@ function get_default_role_archetype_allows($type, $archetype) { 'user' => array(), 'frontpage' => array(), ), + 'view' => array( + 'manager' => array('manager', 'coursecreator', 'editingteacher', 'teacher', 'student', 'guest', 'user', 'frontpage'), + 'coursecreator' => array('coursecreator', 'editingteacher', 'teacher', 'student'), + 'editingteacher' => array('coursecreator', 'editingteacher', 'teacher', 'student'), + 'teacher' => array('coursecreator', 'editingteacher', 'teacher', 'student'), + 'student' => array('coursecreator', 'editingteacher', 'teacher', 'student'), + 'guest' => array(), + 'user' => array(), + 'frontpage' => array(), + ), ); if (!isset($defaults[$type][$archetype])) { @@ -2602,10 +2612,14 @@ function get_user_roles_in_course($userid, $courseid) { $rolestring = ''; if ($roles = $DB->get_records_sql($sql, $params)) { - $rolenames = role_fix_names($roles, $context, ROLENAME_ALIAS, true); // Substitute aliases + $viewableroles = get_viewable_roles($context, $userid); - foreach ($rolenames as $roleid => $rolename) { - $rolenames[$roleid] = ''.$rolename.''; + $rolenames = array(); + foreach ($roles as $roleid => $unused) { + if (isset($viewableroles[$roleid])) { + $url = new moodle_url('/user/index.php', ['contextid' => $context->id, 'roleid' => $roleid]); + $rolenames[] = '' . $viewableroles[$roleid] . ''; + } } $rolestring = implode(',', $rolenames); } @@ -2883,6 +2897,22 @@ function allow_switch($fromroleid, $targetroleid) { $DB->insert_record('role_allow_switch', $record); } +/** + * Creates a record in the role_allow_view table + * + * @param int $fromroleid source roleid + * @param int $targetroleid target roleid + * @return void + */ +function core_role_set_view_allowed($fromroleid, $targetroleid) { + global $DB; + + $record = new stdClass(); + $record->roleid = $fromroleid; + $record->allowview = $targetroleid; + $DB->insert_record('role_allow_view', $record); +} + /** * Gets a list of roles that this user can assign in this context * @@ -3023,6 +3053,58 @@ function get_switchable_roles(context $context) { return role_fix_names($roles, $context, ROLENAME_ALIAS, true); } +/** + * Gets a list of roles that this user can view in a context + * + * @param context $context a context. + * @param int $userid id of user. + * @return array an array $roleid => $rolename. + */ +function get_viewable_roles(context $context, $userid = null) { + global $USER, $DB; + + if ($userid == null) { + $userid = $USER->id; + } + + $params = array(); + $extrajoins = ''; + $extrawhere = ''; + if (!is_siteadmin()) { + // Admins are allowed to view any role. + // Others are subject to the additional constraint that the view role must be allowed by + // 'role_allow_view' for some role they have assigned in this context or any parent. + $contexts = $context->get_parent_context_ids(true); + list($insql, $inparams) = $DB->get_in_or_equal($contexts, SQL_PARAMS_NAMED); + + $extrajoins = "JOIN {role_allow_view} ras ON ras.allowview = r.id + JOIN {role_assignments} ra ON ra.roleid = ras.roleid"; + $extrawhere = "WHERE ra.userid = :userid AND ra.contextid $insql"; + + $params += $inparams; + $params['userid'] = $userid; + } + + if ($coursecontext = $context->get_course_context(false)) { + $params['coursecontext'] = $coursecontext->id; + } else { + $params['coursecontext'] = 0; // No course aliases. + $coursecontext = null; + } + + $query = " + SELECT r.id, r.name, r.shortname, rn.name AS coursealias + FROM {role} r + $extrajoins + LEFT JOIN {role_names} rn ON (rn.contextid = :coursecontext AND rn.roleid = r.id) + $extrawhere + GROUP BY r.id, r.name, r.shortname, rn.name + ORDER BY r.sortorder"; + $roles = $DB->get_records_sql($query, $params); + + return role_fix_names($roles, $context, ROLENAME_ALIAS, true); +} + /** * Gets a list of roles that this user can override in this context. * diff --git a/lib/classes/event/role_allow_view_updated.php b/lib/classes/event/role_allow_view_updated.php new file mode 100644 index 00000000000..1d08df97eb8 --- /dev/null +++ b/lib/classes/event/role_allow_view_updated.php @@ -0,0 +1,82 @@ +. + +/** + * Role allow view updated event. + * + * @package core + * @since Moodle 3.4 + * @copyright 2017 Andrew Hancox + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core\event; + +defined('MOODLE_INTERNAL') || die(); + +/** + * Role allow view updated event class. + * + * @package core + * @since Moodle 2.6 + * @copyright 2013 Rajesh Taneja + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class role_allow_view_updated extends base { + /** + * Initialise event parameters. + */ + protected function init() { + $this->data['crud'] = 'u'; + $this->data['edulevel'] = self::LEVEL_OTHER; + } + + /** + * Returns localised event name. + * + * @return string + */ + public static function get_name() { + return get_string('eventroleallowviewupdated', 'role'); + } + + /** + * Returns non-localised event description with id's for admin use only. + * + * @return string + */ + public function get_description() { + return "The user with id '$this->userid' updated Allow role views."; + } + + /** + * Returns relevant URL. + * + * @return \moodle_url + */ + public function get_url() { + return new \moodle_url('/admin/roles/allow.php', array('mode' => 'view')); + } + + /** + * Returns array of parameters to be passed to legacy add_to_log() function. + * + * @return array + */ + protected function get_legacy_logdata() { + return array(SITEID, 'role', 'edit allow view', 'admin/roles/allow.php?mode=view'); + } +} diff --git a/lib/db/install.php b/lib/db/install.php index 22816a158cf..2c2e576accc 100644 --- a/lib/db/install.php +++ b/lib/db/install.php @@ -266,7 +266,7 @@ function xmldb_main_install() { // Default allow role matrices. foreach ($DB->get_records('role') as $role) { - foreach (array('assign', 'override', 'switch') as $type) { + foreach (array('assign', 'override', 'switch', 'view') as $type) { $function = 'allow_'.$type; $allows = get_default_role_archetype_allows($type, $role->archetype); foreach ($allows as $allowid) { diff --git a/lib/db/install.xml b/lib/db/install.xml index 821cf37726b..f95a699618c 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1077,6 +1077,20 @@ + + + + + + + + + + + + + +
diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 31019f526b7..e2e68c1526b 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -1859,5 +1859,48 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2017121200.00); } + if ($oldversion < 2017121900.00) { + + // Define table role_allow_view to be created. + $table = new xmldb_table('role_allow_view'); + + // Adding fields to table role_allow_view. + $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null); + $table->add_field('roleid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null); + $table->add_field('allowview', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null); + + // Adding keys to table role_allow_view. + $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id')); + $table->add_key('roleid', XMLDB_KEY_FOREIGN, array('roleid'), 'role', array('id')); + $table->add_key('allowview', XMLDB_KEY_FOREIGN, array('allowview'), 'role', array('id')); + + // Conditionally launch create table for role_allow_view. + if (!$dbman->table_exists($table)) { + $dbman->create_table($table); + } + + $index = new xmldb_index('roleid-allowview', XMLDB_INDEX_UNIQUE, array('roleid', 'allowview')); + + // Conditionally launch add index roleid. + if (!$dbman->index_exists($table, $index)) { + $dbman->add_index($table, $index); + } + + $roles = $DB->get_records('role', array(), 'sortorder ASC'); + + $DB->delete_records('role_allow_view'); + foreach ($roles as $role) { + foreach ($roles as $allowedrole) { + $record = new stdClass(); + $record->roleid = $role->id; + $record->allowview = $allowedrole->id; + $DB->insert_record('role_allow_view', $record); + } + } + + // Main savepoint reached. + upgrade_main_savepoint(true, 2017121900.00); + } + return true; } diff --git a/lib/statslib.php b/lib/statslib.php index 9dd95ca0378..9dfd7f05f74 100644 --- a/lib/statslib.php +++ b/lib/statslib.php @@ -1404,6 +1404,7 @@ function stats_get_report_options($courseid,$mode) { $sql = 'SELECT r.id, r.name, r.shortname FROM {role} r JOIN {stats_daily} s ON s.roleid = r.id WHERE s.courseid = :courseid GROUP BY r.id, r.name, r.shortname'; if ($roles = $DB->get_records_sql($sql, array('courseid' => $courseid))) { + $roles = array_intersect_key($roles, get_viewable_roles($context)); foreach ($roles as $role) { $reportoptions[STATS_REPORT_ACTIVITYBYROLE.$role->id] = get_string('statsreport'.STATS_REPORT_ACTIVITYBYROLE). ' ' . role_get_name($role, $context); diff --git a/lib/testing/generator/data_generator.php b/lib/testing/generator/data_generator.php index 4bf2e8b42bf..6d422506a12 100644 --- a/lib/testing/generator/data_generator.php +++ b/lib/testing/generator/data_generator.php @@ -789,9 +789,9 @@ EOD; if ($record['archetype']) { - // We copy all the roles the archetype can assign, override and switch to. + // We copy all the roles the archetype can assign, override, switch to and view. if ($record['archetype']) { - $types = array('assign', 'override', 'switch'); + $types = array('assign', 'override', 'switch', 'view'); foreach ($types as $type) { $rolestocopy = get_default_role_archetype_allows($type, $record['archetype']); foreach ($rolestocopy as $tocopy) { diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php index a4d16e5d60b..d18cb52b23b 100644 --- a/lib/tests/accesslib_test.php +++ b/lib/tests/accesslib_test.php @@ -918,6 +918,9 @@ class core_accesslib_testcase extends advanced_testcase { $result = get_default_role_archetype_allows('switch', $archetype); $this->assertInternalType('array', $result); + + $result = get_default_role_archetype_allows('view', $archetype); + $this->assertInternalType('array', $result); } $result = get_default_role_archetype_allows('assign', ''); @@ -929,6 +932,9 @@ class core_accesslib_testcase extends advanced_testcase { $result = get_default_role_archetype_allows('switch', ''); $this->assertSame(array(), $result); + $result = get_default_role_archetype_allows('view', ''); + $this->assertSame(array(), $result); + $result = get_default_role_archetype_allows('assign', 'wrongarchetype'); $this->assertSame(array(), $result); $this->assertDebuggingCalled(); @@ -940,6 +946,10 @@ class core_accesslib_testcase extends advanced_testcase { $result = get_default_role_archetype_allows('switch', 'wrongarchetype'); $this->assertSame(array(), $result); $this->assertDebuggingCalled(); + + $result = get_default_role_archetype_allows('view', 'wrongarchetype'); + $this->assertSame(array(), $result); + $this->assertDebuggingCalled(); } /** @@ -1029,6 +1039,35 @@ class core_accesslib_testcase extends advanced_testcase { $this->assertEventLegacyLogData($expectedlegacylog, $event); } + /** + * Test allowing of role switching. + */ + public function test_allow_view() { + global $DB, $CFG; + + $this->resetAfterTest(); + + $otherid = create_role('Other role', 'other', 'Some other role', ''); + $student = $DB->get_record('role', array('shortname' => 'student'), '*', MUST_EXIST); + + $this->assertFalse($DB->record_exists('role_allow_view', array('roleid' => $otherid, 'allowview' => $student->id))); + allow_view($otherid, $student->id); + $this->assertTrue($DB->record_exists('role_allow_view', array('roleid' => $otherid, 'allowview' => $student->id))); + + // Test event trigger. + $allowroleassignevent = \core\event\role_allow_view_updated::create(array('context' => context_system::instance())); + $sink = $this->redirectEvents(); + $allowroleassignevent->trigger(); + $events = $sink->get_events(); + $sink->close(); + $event = array_pop($events); + $this->assertInstanceOf('\core\event\role_allow_view_updated', $event); + $mode = 'view'; + $baseurl = new moodle_url('/admin/roles/allow.php', array('mode' => $mode)); + $expectedlegacylog = array(SITEID, 'role', 'edit allow ' . $mode, str_replace($CFG->wwwroot . '/', '', $baseurl)); + $this->assertEventLegacyLogData($expectedlegacylog, $event); + } + /** * Test returning of assignable roles in context. */ @@ -1287,6 +1326,74 @@ class core_accesslib_testcase extends advanced_testcase { } } + /** + * Test getting of all overridable roles. + */ + public function test_get_viewable_roles_course() { + global $DB; + + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $coursecontext = context_course::instance($course->id); + + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'), '*', MUST_EXIST); + $teacher = $this->getDataGenerator()->create_user(); + role_assign($teacherrole->id, $teacher->id, $coursecontext); + + $studentrole = $DB->get_record('role', array('shortname' => 'student'), '*', MUST_EXIST); + $studentrolerename = (object) array('roleid' => $studentrole->id, 'name' => 'Učitel', 'contextid' => $coursecontext->id); + $DB->insert_record('role_names', $studentrolerename); + + // By default teacher can see student. + $this->setUser($teacher); + $viewableroles = get_viewable_roles($coursecontext); + $this->assertContains($studentrolerename->name, array_values($viewableroles)); + // Remove view permission. + $DB->delete_records('role_allow_view', array('roleid' => $teacherrole->id, 'allowview' => $studentrole->id)); + $viewableroles = get_viewable_roles($coursecontext); + // Teacher can no longer see student role. + $this->assertNotContains($studentrolerename->name, array_values($viewableroles)); + // Allow again teacher to view student. + allow_view($teacherrole->id, $studentrole->id); + // Teacher can now see student role. + $viewableroles = get_viewable_roles($coursecontext); + $this->assertContains($studentrolerename->name, array_values($viewableroles)); + } + + /** + * Test getting of all overridable roles. + */ + public function test_get_viewable_roles_system() { + global $DB; + + $this->resetAfterTest(); + + $context = context_system::instance(); + + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'), '*', MUST_EXIST); + $teacher = $this->getDataGenerator()->create_user(); + role_assign($teacherrole->id, $teacher->id, $context); + + $studentrole = $DB->get_record('role', array('shortname' => 'student'), '*', MUST_EXIST); + $studentrolename = role_get_name($studentrole, $context); + + // By default teacher can see student. + $this->setUser($teacher); + $viewableroles = get_viewable_roles($context); + $this->assertContains($studentrolename, array_values($viewableroles)); + // Remove view permission. + $DB->delete_records('role_allow_view', array('roleid' => $teacherrole->id, 'allowview' => $studentrole->id)); + $viewableroles = get_viewable_roles($context); + // Teacher can no longer see student role. + $this->assertNotContains($studentrolename, array_values($viewableroles)); + // Allow again teacher to view student. + allow_view($teacherrole->id, $studentrole->id); + // Teacher can now see student role. + $viewableroles = get_viewable_roles($context); + $this->assertContains($studentrolename, array_values($viewableroles)); + } + /** * Test we have context level defaults. */ @@ -1553,6 +1660,8 @@ class core_accesslib_testcase extends advanced_testcase { $user4 = $this->getDataGenerator()->create_user(); role_assign($managerrole->id, $user4->id, $coursecontext->id); + $this->setAdminUser(); + $roles = get_user_roles_in_course($user1->id, $course->id); $this->assertEquals(1, preg_match_all('/,/', $roles, $matches)); $this->assertTrue(strpos($roles, role_get_name($teacherrole, $coursecontext)) !== false); diff --git a/lib/upgrade.txt b/lib/upgrade.txt index ccc2381f8e6..924e5af9725 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -58,6 +58,9 @@ information provided here is intended especially for developers. * $CFG->loginhttps is now deprecated, do not use it. * $PAGE->https_required and $PAGE->verify_https_required() are now deprecated. They are no longer used and will throw a coding_exception. * $CFG->httpswwwroot is now deprecated and will always result in the same value as wwwroot. +* Added function core_role_set_view_allowed() to check if a user should be able to see a given role. + This should be checked whenever displaying a list of roles to a user, however, core_role_set_assign_allowed may need to override it + in some cases. === 3.3.1 === diff --git a/report/stats/locallib.php b/report/stats/locallib.php index 75b91b680f7..65389046c75 100644 --- a/report/stats/locallib.php +++ b/report/stats/locallib.php @@ -287,14 +287,14 @@ function report_stats_report($course, $report, $mode, $user, $roleid, $time) { $times = array(); $missedlines = array(); $coursecontext = context_course::instance($course->id); - $rolenames = role_fix_names(get_all_roles($coursecontext), $coursecontext, ROLENAME_ALIAS, true); + $rolenames = get_viewable_roles($coursecontext); foreach ($stats as $stat) { if (!empty($stat->zerofixed)) { $missedlines[] = $stat->timeend; } $data[$stat->timeend][$stat->roleid] = $stat->line1; if ($stat->roleid != 0) { - if (!array_key_exists($stat->roleid,$roles)) { + if (!array_key_exists($stat->roleid, $roles) && array_key_exists($stat->roleid, $rolenames)) { $roles[$stat->roleid] = $rolenames[$stat->roleid]; } } else { @@ -419,7 +419,7 @@ function report_stats_print_chart($courseid, $report, $time, $mode, $userid = 0, $times = array(); $roles = array(); $missedlines = array(); - $rolenames = role_fix_names(get_all_roles($coursecontext), $coursecontext, ROLENAME_ALIAS, true); + $rolenames = get_viewable_roles($coursecontext); foreach ($stats as $stat) { $data[$stat->roleid][$stat->timeend] = $stat->line1; @@ -427,7 +427,7 @@ function report_stats_print_chart($courseid, $report, $time, $mode, $userid = 0, $missedlines[] = $stat->timeend; } if ($stat->roleid != 0) { - if (!array_key_exists($stat->roleid, $roles)) { + if (!array_key_exists($stat->roleid, $roles) && array_key_exists($stat->roleid, $rolenames)) { $roles[$stat->roleid] = $rolenames[$stat->roleid]; } } else { diff --git a/user/classes/output/user_roles_editable.php b/user/classes/output/user_roles_editable.php index 56c8aec0803..12fc5c51e7b 100644 --- a/user/classes/output/user_roles_editable.php +++ b/user/classes/output/user_roles_editable.php @@ -49,6 +49,12 @@ class user_roles_editable extends \core\output\inplace_editable { /** @var \stdClass[] $profileroles */ private $profileroles; + /** @var \stdClass[] $viewableroles */ + private $viewableroles; + + /** @var \stdClass[] $assignableroles */ + private $assignableroles; + /** * Constructor. * @@ -60,7 +66,11 @@ class user_roles_editable extends \core\output\inplace_editable { * @param \stdClass[] $profileroles The list of roles that should be visible in a users profile. * @param \stdClass[] $userroles The list of user roles. */ - public function __construct($course, $context, $user, $courseroles, $assignableroles, $profileroles, $userroles) { + public function __construct($course, $context, $user, $courseroles, $assignableroles, $profileroles, $userroles, $viewableroles = null) { + if ($viewableroles === null) { + debugging('Constructor for user_roles_editable now needs the result of get_viewable_roles passed as viewableroles'); + } + // Check capabilities to get editable value. $editable = has_capability('moodle/role:assign', $context); @@ -77,6 +87,8 @@ class user_roles_editable extends \core\output\inplace_editable { // Remember these for the display value. $this->courseroles = $courseroles; $this->profileroles = $profileroles; + $this->viewableroles = array_keys($viewableroles); + $this->assignableroles = array_keys($assignableroles); $this->context = $context; parent::__construct('core_user', 'user_roles', $itemid, $editable, $value, $value); @@ -106,8 +118,9 @@ class user_roles_editable extends \core\output\inplace_editable { public function export_for_template(\renderer_base $output) { $listofroles = []; $roleids = json_decode($this->value); + $viewableroleids = array_intersect($roleids, array_merge($this->viewableroles, $this->assignableroles)); - foreach ($roleids as $id) { + foreach ($viewableroleids as $id) { // If this is a student, we only show a subset of the roles. if ($this->editable || array_key_exists($id, $this->profileroles)) { $listofroles[] = format_string($this->courseroles[$id]->localname, true, ['context' => $this->context]); @@ -116,6 +129,8 @@ class user_roles_editable extends \core\output\inplace_editable { if (!empty($listofroles)) { $this->displayvalue = implode($listofroles, ', '); + } else if (!empty($roleids) && empty($viewableroleids)) { + $this->displayvalue = get_string('novisibleroles', 'role'); } else { $this->displayvalue = get_string('noroles', 'role'); } @@ -160,6 +175,7 @@ class user_roles_editable extends \core\output\inplace_editable { // Check that all the groups belong to the course. $allroles = role_fix_names(get_all_roles($context), $context); $assignableroles = get_assignable_roles($context, ROLENAME_ALIAS, false); + $viewableroles = get_viewable_roles($context); $userrolesbyid = get_user_roles($context, $userid, true, 'c.contextlevel DESC, r.sortorder ASC'); $profileroles = get_profile_roles($context); @@ -224,6 +240,6 @@ class user_roles_editable extends \core\output\inplace_editable { $course = get_course($courseid); $user = core_user::get_user($userid); - return new self($course, $context, $user, $allroles, $assignableroles, $profileroles, $userroles); + return new self($course, $context, $user, $allroles, $assignableroles, $profileroles, $userroles, $viewableroles); } } diff --git a/user/classes/participants_table.php b/user/classes/participants_table.php index bc66a9efbe1..09f92468e71 100644 --- a/user/classes/participants_table.php +++ b/user/classes/participants_table.php @@ -129,6 +129,9 @@ class participants_table extends \table_sql { */ protected $profileroles; + /** @var \stdClass[] $viewableroles */ + private $viewableroles; + /** * Sets up the table. * @@ -236,6 +239,7 @@ class participants_table extends \table_sql { $this->allroleassignments = get_users_roles($this->context, [], true, 'c.contextlevel DESC, r.sortorder ASC'); $this->assignableroles = get_assignable_roles($this->context, ROLENAME_ALIAS, false); $this->profileroles = get_profile_roles($this->context); + $this->viewableroles = get_viewable_roles($this->context); } /** @@ -299,7 +303,8 @@ class participants_table extends \table_sql { $this->allroles, $this->assignableroles, $this->profileroles, - $roles); + $roles, + $this->viewableroles); return $OUTPUT->render_from_template('core/inplace_editable', $editable->export_for_template($OUTPUT)); } diff --git a/user/renderer.php b/user/renderer.php index ca0fe04d407..d3af40a1c8b 100644 --- a/user/renderer.php +++ b/user/renderer.php @@ -127,7 +127,12 @@ class core_user_renderer extends plugin_renderer_base { $filteroptions = []; // Filter options for role. - $roles = role_fix_names(get_profile_roles($context), $context, ROLENAME_ALIAS, true); + $roleseditable = has_capability('moodle/role:assign', $context); + $roles = get_viewable_roles($context); + if ($roleseditable) { + $roles += get_assignable_roles($context, ROLENAME_ALIAS); + } + $criteria = get_string('role'); $roleoptions = []; foreach ($roles as $id => $role) { diff --git a/version.php b/version.php index 454b481422d..fd60f44e0b9 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2017121400.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2017121900.00; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.