Skip to content

Commit

Permalink
MDL-43130 access: fix user counting when retrieving assignable roles.
Browse files Browse the repository at this point in the history
Previously users assigned the same role in a context via multiple
components would be counted multiple times.
  • Loading branch information
paulholden authored and stronk7 committed Mar 26, 2019
1 parent f1d125b commit ed88c64
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/accesslib.php
Original file line number Diff line number Diff line change
Expand Up @@ -3043,7 +3043,7 @@ function get_assignable_roles(context $context, $rolenamedisplay = ROLENAME_ALIA
$extrafields = '';

if ($withusercounts) {
$extrafields = ', (SELECT count(u.id)
$extrafields = ', (SELECT COUNT(DISTINCT u.id)
FROM {role_assignments} cra JOIN {user} u ON cra.userid = u.id
WHERE cra.roleid = r.id AND cra.contextid = :conid AND u.deleted = 0
) AS usercount';
Expand Down
33 changes: 33 additions & 0 deletions lib/tests/accesslib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -1174,6 +1174,39 @@ public function test_get_assignable_roles() {
}
}

/**
* Test user count of assignable roles in context where users are assigned the role via different components.
*/
public function test_get_assignable_roles_distinct_usercount() {
global $DB;

$this->resetAfterTest(true);

$this->setAdminUser();

$course = $this->getDataGenerator()->create_course();
$context = \context_course::instance($course->id);

$user1 = $this->getDataGenerator()->create_user();
$user2 = $this->getDataGenerator()->create_user();

$studentrole = $DB->get_record('role', ['shortname' => 'student']);

// Assign each user the student role in course.
role_assign($studentrole->id, $user1->id, $context->id);
role_assign($studentrole->id, $user2->id, $context->id);

list($rolenames, $rolecounts, $nameswithcounts) = get_assignable_roles($context, ROLENAME_SHORT, true);
$this->assertEquals(2, $rolecounts[$studentrole->id]);

// Assign first user the student role in course again (this time via 'enrol_self' component).
role_assign($studentrole->id, $user1->id, $context->id, 'enrol_self', 1);

// There are still only two distinct users.
list($rolenames, $rolecounts, $nameswithcounts) = get_assignable_roles($context, ROLENAME_SHORT, true);
$this->assertEquals(2, $rolecounts[$studentrole->id]);
}

/**
* Test getting of all switchable roles.
*/
Expand Down

0 comments on commit ed88c64

Please sign in to comment.