diff --git a/lib/accesslib.php b/lib/accesslib.php index c60077d5f84..0e15bf9db66 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -2475,20 +2475,25 @@ function get_component_string($component, $contextlevel) { /** * Gets the list of roles assigned to this context and up (parents) - * from the list of roles that are visible on user profile page - * and participants page. + * from the aggregation of: + * a) the list of roles that are visible on user profile page and participants page (profileroles setting) and; + * b) if applicable, those roles the current user can assign in the context. * * @param context $context * @return array */ function get_profile_roles(context $context) { global $CFG, $DB; - - if (empty($CFG->profileroles)) { - return array(); + // If the current user can assign roles, then they can also see those assignable roles on the profile and participants page, + // provided the roles are assigned to at least 1 user in the context. + $policyroles = empty($CFG->profileroles) ? [] : array_map('trim', explode(',', $CFG->profileroles)); + $assignableroles = array_keys(get_assignable_roles($context)); + $rolesinscope = array_values(array_unique(array_merge($policyroles, $assignableroles))); + if (empty($rolesinscope)) { + return []; } - list($rallowed, $params) = $DB->get_in_or_equal(explode(',', $CFG->profileroles), SQL_PARAMS_NAMED, 'a'); + list($rallowed, $params) = $DB->get_in_or_equal($rolesinscope, SQL_PARAMS_NAMED, 'a'); list($contextlist, $cparams) = $DB->get_in_or_equal($context->get_parent_context_ids(true), SQL_PARAMS_NAMED, 'p'); $params = array_merge($params, $cparams); @@ -2547,18 +2552,21 @@ function get_roles_used_in_context(context $context) { */ function get_user_roles_in_course($userid, $courseid) { global $CFG, $DB; - - if (empty($CFG->profileroles)) { - return ''; - } - if ($courseid == SITEID) { $context = context_system::instance(); } else { $context = context_course::instance($courseid); } + // If the current user can assign roles, then they can also see those assignable roles on the profile and participants page, + // provided the roles are assigned to at least 1 user in the context. + $policyroles = empty($CFG->profileroles) ? [] : array_map('trim', explode(',', $CFG->profileroles)); + $assignableroles = array_keys(get_assignable_roles($context)); + $rolesinscope = array_values(array_unique(array_merge($policyroles, $assignableroles))); + if (empty($rolesinscope)) { + return ''; + } - list($rallowed, $params) = $DB->get_in_or_equal(explode(',', $CFG->profileroles), SQL_PARAMS_NAMED, 'a'); + list($rallowed, $params) = $DB->get_in_or_equal($rolesinscope, SQL_PARAMS_NAMED, 'a'); list($contextlist, $cparams) = $DB->get_in_or_equal($context->get_parent_context_ids(true), SQL_PARAMS_NAMED, 'p'); $params = array_merge($params, $cparams); diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php index 04f76feece0..14375ee80a9 100644 --- a/lib/tests/accesslib_test.php +++ b/lib/tests/accesslib_test.php @@ -1533,6 +1533,7 @@ class core_accesslib_testcase extends advanced_testcase { $teacherrole = $DB->get_record('role', array('shortname'=>'editingteacher'), '*', MUST_EXIST); $studentrole = $DB->get_record('role', array('shortname'=>'student'), '*', MUST_EXIST); + $managerrole = $DB->get_record('role', array('shortname' => 'manager'), '*', MUST_EXIST); $course = $this->getDataGenerator()->create_course(); $coursecontext = context_course::instance($course->id); $teacherrename = (object)array('roleid'=>$teacherrole->id, 'name'=>'Učitel', 'contextid'=>$coursecontext->id); @@ -1541,6 +1542,7 @@ class core_accesslib_testcase extends advanced_testcase { $roleids = explode(',', $CFG->profileroles); // Should include teacher and student in new installs. $this->assertTrue(in_array($teacherrole->id, $roleids)); $this->assertTrue(in_array($studentrole->id, $roleids)); + $this->assertFalse(in_array($managerrole->id, $roleids)); $user1 = $this->getDataGenerator()->create_user(); role_assign($teacherrole->id, $user1->id, $coursecontext->id); @@ -1548,6 +1550,8 @@ class core_accesslib_testcase extends advanced_testcase { $user2 = $this->getDataGenerator()->create_user(); role_assign($studentrole->id, $user2->id, $coursecontext->id); $user3 = $this->getDataGenerator()->create_user(); + $user4 = $this->getDataGenerator()->create_user(); + role_assign($managerrole->id, $user4->id, $coursecontext->id); $roles = get_user_roles_in_course($user1->id, $course->id); $this->assertEquals(1, preg_match_all('/,/', $roles, $matches)); @@ -1559,6 +1563,25 @@ class core_accesslib_testcase extends advanced_testcase { $roles = get_user_roles_in_course($user3->id, $course->id); $this->assertSame('', $roles); + + // Managers should be able to see a link to their own role type, given they can assign it in the context. + $this->setUser($user4); + $roles = get_user_roles_in_course($user4->id, $course->id); + $this->assertNotEmpty($roles); + $this->assertEquals(1, count(explode(',', $roles))); + $this->assertTrue(strpos($roles, role_get_name($managerrole, $coursecontext)) !== false); + + // Managers should see 2 roles if viewing a user who has been enrolled as a student and a teacher in the course. + $roles = get_user_roles_in_course($user1->id, $course->id); + $this->assertEquals(2, count(explode(',', $roles))); + $this->assertTrue(strpos($roles, role_get_name($studentrole, $coursecontext)) !== false); + $this->assertTrue(strpos($roles, role_get_name($teacherrole, $coursecontext)) !== false); + + // Students should not see the manager role if viewing a manager's profile. + $this->setUser($user2); + $roles = get_user_roles_in_course($user4->id, $course->id); + $this->assertEmpty($roles); // Should see 0 roles on the manager's profile. + $this->assertFalse(strpos($roles, role_get_name($managerrole, $coursecontext)) !== false); } /** @@ -3166,6 +3189,136 @@ class core_accesslib_testcase extends advanced_testcase { $this->assertTrue(array_key_exists($student->id, $users)); $this->assertFalse(array_key_exists($guest->id, $users)); } + + /** + * Test the get_profile_roles() function. + */ + public function test_get_profile_roles() { + global $DB; + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $coursecontext = context_course::instance($course->id); + + // Assign a student role. + $studentrole = $DB->get_record('role', array('shortname' => 'student'), '*', MUST_EXIST); + $user1 = $this->getDataGenerator()->create_user(); + role_assign($studentrole->id, $user1->id, $coursecontext); + + // Assign an editing teacher role. + $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'), '*', MUST_EXIST); + $user2 = $this->getDataGenerator()->create_user(); + role_assign($teacherrole->id, $user2->id, $coursecontext); + + // Create a custom role that can be assigned at course level, but don't assign it yet. + create_role('Custom role', 'customrole', 'Custom course role'); + $customrole = $DB->get_record('role', array('shortname' => 'customrole'), '*', MUST_EXIST); + set_role_contextlevels($customrole->id, [CONTEXT_COURSE]); + allow_assign($teacherrole->id, $customrole->id); // Allow teacher to assign the role in the course. + + // Set the site policy 'profileroles' to show student, teacher and non-editing teacher roles (i.e. not the custom role). + $neteacherrole = $DB->get_record('role', array('shortname' => 'teacher'), '*', MUST_EXIST); + set_config('profileroles', "{$studentrole->id}, {$teacherrole->id}, {$neteacherrole->id}"); + + // A student in the course (given they can't assign roles) should see those roles which are: + // - listed in the 'profileroles' site policy AND + // - are assigned in the course context (or parent contexts). + // In this case, the non-editing teacher role is not assigned and should not be returned. + $expected = [ + $teacherrole->id => (object) [ + 'id' => $teacherrole->id, + 'name' => '', + 'shortname' => $teacherrole->shortname, + 'sortorder' => $teacherrole->sortorder, + 'coursealias' => null + ], + $studentrole->id => (object) [ + 'id' => $studentrole->id, + 'name' => '', + 'shortname' => $studentrole->shortname, + 'sortorder' => $studentrole->sortorder, + 'coursealias' => null + ] + ]; + $this->setUser($user1); + $this->assertEquals($expected, get_profile_roles($coursecontext)); + + // An editing teacher should also see only 2 roles at this stage as only 2 roles are assigned: 'teacher' and 'student'. + $this->setUser($user2); + $this->assertEquals($expected, get_profile_roles($coursecontext)); + + // Assign a custom role in the course. + $user3 = $this->getDataGenerator()->create_user(); + role_assign($customrole->id, $user3->id, $coursecontext); + + // Confirm that the teacher can see the custom role now that it's assigned. + $expectedteacher = [ + $teacherrole->id => (object) [ + 'id' => $teacherrole->id, + 'name' => '', + 'shortname' => $teacherrole->shortname, + 'sortorder' => $teacherrole->sortorder, + 'coursealias' => null + ], + $studentrole->id => (object) [ + 'id' => $studentrole->id, + 'name' => '', + 'shortname' => $studentrole->shortname, + 'sortorder' => $studentrole->sortorder, + 'coursealias' => null + ], + $customrole->id => (object) [ + 'id' => $customrole->id, + 'name' => 'Custom role', + 'shortname' => $customrole->shortname, + 'sortorder' => $customrole->sortorder, + 'coursealias' => null + ] + ]; + $this->setUser($user2); + $this->assertEquals($expectedteacher, get_profile_roles($coursecontext)); + + // And that the student can't, because the role isn't included in the 'profileroles' site policy. + $expectedstudent = [ + $teacherrole->id => (object) [ + 'id' => $teacherrole->id, + 'name' => '', + 'shortname' => $teacherrole->shortname, + 'sortorder' => $teacherrole->sortorder, + 'coursealias' => null + ], + $studentrole->id => (object) [ + 'id' => $studentrole->id, + 'name' => '', + 'shortname' => $studentrole->shortname, + 'sortorder' => $studentrole->sortorder, + 'coursealias' => null + ] + ]; + $this->setUser($user1); + $this->assertEquals($expectedstudent, get_profile_roles($coursecontext)); + + // If we have no roles listed in the site policy, the teacher should only see the student and custom roles. + $expectedteacher = [ + $studentrole->id => (object) [ + 'id' => $studentrole->id, + 'name' => '', + 'shortname' => $studentrole->shortname, + 'sortorder' => $studentrole->sortorder, + 'coursealias' => null + ], + $customrole->id => (object) [ + 'id' => $customrole->id, + 'name' => 'Custom role', + 'shortname' => $customrole->shortname, + 'sortorder' => $customrole->sortorder, + 'coursealias' => null + ] + ]; + set_config('profileroles', ""); + $this->setUser($user2); + $this->assertEquals($expectedteacher, get_profile_roles($coursecontext)); + } } /** diff --git a/user/renderer.php b/user/renderer.php index d881f0cd57f..89ceb2bca3e 100644 --- a/user/renderer.php +++ b/user/renderer.php @@ -281,11 +281,7 @@ class core_user_renderer extends plugin_renderer_base { } // Filter options for role. - $roleseditable = has_capability('moodle/role:assign', $context); $roles = role_fix_names(get_profile_roles($context), $context, ROLENAME_ALIAS, true); - if ($roleseditable) { - $roles += get_assignable_roles($context, ROLENAME_ALIAS); - } $criteria = get_string('role'); $roleoptions = []; foreach ($roles as $id => $role) {