From 07f50b7284e318a6ec9b7a85374891a11d97ab0a Mon Sep 17 00:00:00 2001
From: Neill Magill <neill.magill@nottingham.ac.uk>
Date: Fri, 20 Mar 2015 13:46:27 +0000
Subject: [PATCH 1/2] MDL-49613 access: Unit test for count_role_users

---
 lib/tests/accesslib_test.php | 38 ++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php
index 289131e99fc..de7ba2115ed 100644
--- a/lib/tests/accesslib_test.php
+++ b/lib/tests/accesslib_test.php
@@ -3117,6 +3117,44 @@ class core_accesslib_testcase extends advanced_testcase {
         $this->assertFalse(has_capability('mod/forum:addinstance', $coursecontext, $user));
         $this->assertFalse(has_capability('mod/forum:viewdiscussion', $coursecontext, $user));
     }
+
+    /**
+     * Tests count_role_users function.
+     */
+    public function test_count_role_users() {
+        global $DB;
+        $this->resetAfterTest(true);
+        $generator = self::getDataGenerator();
+        // Create a course in a category, and some users.
+        $category = $generator->create_category();
+        $course = $generator->create_course(array('category' => $category->id));
+        $user1 = $generator->create_user();
+        $user2 = $generator->create_user();
+        $user3 = $generator->create_user();
+        $user4 = $generator->create_user();
+        $user5 = $generator->create_user();
+        $roleid1 = $DB->get_field('role', 'id', array('shortname' => 'manager'), MUST_EXIST);
+        $roleid2 = $DB->get_field('role', 'id', array('shortname' => 'coursecreator'), MUST_EXIST);
+        // Enrol two users as managers onto the course, and 1 onto the category.
+        $generator->enrol_user($user1->id, $course->id, $roleid1);
+        $generator->enrol_user($user2->id, $course->id, $roleid1);
+        $generator->role_assign($roleid1, $user3->id, context_coursecat::instance($category->id));
+        // Enrol 1 user as a coursecreator onto the course, and another onto the category.
+        // This is to ensure we do not count users with roles that are not specified.
+        $generator->enrol_user($user4->id, $course->id, $roleid2);
+        $generator->role_assign($roleid2, $user5->id, context_coursecat::instance($category->id));
+        // Check that the correct users are found on the course.
+        $this->assertEquals(2, count_role_users($roleid1, context_course::instance($course->id), false));
+        $this->assertEquals(3, count_role_users($roleid1, context_course::instance($course->id), true));
+        // Check for the category.
+        $this->assertEquals(1, count_role_users($roleid1, context_coursecat::instance($category->id), false));
+        $this->assertEquals(1, count_role_users($roleid1, context_coursecat::instance($category->id), true));
+        // Have a user with the same role at both the category and course level.
+        $generator->role_assign($roleid1, $user1->id, context_coursecat::instance($category->id));
+        // The course level checks should remain the same.
+        $this->assertEquals(2, count_role_users($roleid1, context_course::instance($course->id), false));
+        $this->assertEquals(3, count_role_users($roleid1, context_course::instance($course->id), true));
+    }
 }
 
 /**

From 43f35d037e051ec17515d28deff93867ed7d6c96 Mon Sep 17 00:00:00 2001
From: Neill Magill <neill.magill@nottingham.ac.uk>
Date: Fri, 20 Mar 2015 14:31:34 +0000
Subject: [PATCH 2/2] MDL-49613 access: count_role_users gives unexpected
 results

The count_role_users function in lib/accesslib.php gives the wrong results if a user has the
same role at multiple levels in a context path.

For example a user is a coursecreator on a course and the category that the course is in,
in this case they would be counted twice, rather than being recognised as the same user.

This change will make the function only count unique users.
---
 lib/accesslib.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/accesslib.php b/lib/accesslib.php
index 6d343b8fa87..49844279f03 100644
--- a/lib/accesslib.php
+++ b/lib/accesslib.php
@@ -4203,7 +4203,7 @@ function count_role_users($roleid, context $context, $parent = false) {
 
     array_unshift($params, $context->id);
 
-    $sql = "SELECT COUNT(u.id)
+    $sql = "SELECT COUNT(DISTINCT u.id)
               FROM {role_assignments} r
               JOIN {user} u ON u.id = r.userid
              WHERE (r.contextid = ? $parentcontexts)