From 9cd7403fe682569543379daf90b16dc2596f448d Mon Sep 17 00:00:00 2001
From: Jake Dallimore <jake@moodle.com>
Date: Thu, 21 Sep 2017 19:49:19 +0800
Subject: [PATCH] MDL-37810 roles: fix profile roles logic to include all roles

If the user has the role:assign capability then the list of profile roles
will include any role assigned in the context or above.
---
 lib/accesslib.php            | 27 ++++++++++++++++-----------
 lib/tests/accesslib_test.php | 11 +++++++++--
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/lib/accesslib.php b/lib/accesslib.php
index 0ca34fbe07a..7f90c5cd3ef 100644
--- a/lib/accesslib.php
+++ b/lib/accesslib.php
@@ -2745,18 +2745,21 @@ function get_component_string($component, $contextlevel) {
  * Gets the list of roles assigned to this context and up (parents)
  * 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.
+ * b) if applicable, those roles that are assigned in the context.
  *
  * @param context $context
  * @return array
  */
 function get_profile_roles(context $context) {
     global $CFG, $DB;
-    // 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 the current user can assign roles, then they can see all roles on the profile and participants page,
+    // provided the roles are assigned to at least 1 user in the context. If not, only the policy-defined roles.
+    if (has_capability('moodle/role:assign', $context)) {
+        $rolesinscope = array_keys(get_all_roles($context));
+    } else {
+        $rolesinscope = empty($CFG->profileroles) ? [] : array_map('trim', explode(',', $CFG->profileroles));
+    }
+
     if (empty($rolesinscope)) {
         return [];
     }
@@ -2825,11 +2828,13 @@ function get_user_roles_in_course($userid, $courseid) {
     } 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 the current user can assign roles, then they can see all roles on the profile and participants page,
+    // provided the roles are assigned to at least 1 user in the context. If not, only the policy-defined roles.
+    if (has_capability('moodle/role:assign', $context)) {
+        $rolesinscope = array_keys(get_all_roles($context));
+    } else {
+        $rolesinscope = empty($CFG->profileroles) ? [] : array_map('trim', explode(',', $CFG->profileroles));
+    }
     if (empty($rolesinscope)) {
         return '';
     }
diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php
index 83b41fc5182..07535f25562 100644
--- a/lib/tests/accesslib_test.php
+++ b/lib/tests/accesslib_test.php
@@ -3266,7 +3266,7 @@ class core_accesslib_testcase extends advanced_testcase {
         $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.
+        // If we have no roles listed in the site policy, the teacher should be able to see the assigned roles.
         $expectedteacher = [
             $studentrole->id => (object) [
                 'id' => $studentrole->id,
@@ -3281,7 +3281,14 @@ class core_accesslib_testcase extends advanced_testcase {
                 'shortname' => $customrole->shortname,
                 'sortorder' => $customrole->sortorder,
                 'coursealias' => null
-            ]
+            ],
+            $teacherrole->id => (object) [
+                'id' => $teacherrole->id,
+                'name' => '',
+                'shortname' => $teacherrole->shortname,
+                'sortorder' => $teacherrole->sortorder,
+                'coursealias' => null
+            ],
         ];
         set_config('profileroles', "");
         $this->setUser($user2);