From dd97173944ccd418363f2e383732ffc9daba0e98 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 0e20f1f526d..025cf294c11 100644
--- a/lib/tests/accesslib_test.php
+++ b/lib/tests/accesslib_test.php
@@ -3130,6 +3130,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 e115db6dce6c6386883f32de68383ba4393b0b25 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 51675e9a363..b60be06abc5 100644
--- a/lib/accesslib.php
+++ b/lib/accesslib.php
@@ -4232,7 +4232,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)