MDL-68348 lib: User filter match types support - groups

Updated groups_get_members_join to support different filter match types
This commit is contained in:
Michael Hawkins
2020-05-25 18:35:08 +08:00
parent 25d9dabdcf
commit 03397c817b
4 changed files with 554 additions and 30 deletions
+101 -28
View File
@@ -43,6 +43,20 @@ define('VISIBLEGROUPS', 2);
*/
define('USERSWITHOUTGROUP', -1);
/**
* 'None' join type, used when filtering by groups (logical NOT)
*/
define('GROUPS_JOIN_NONE', 0);
/**
* 'Any' join type, used when filtering by groups (logical OR)
*/
define('GROUPS_JOIN_ANY', 1);
/**
* 'All' join type, used when filtering by groups (logical AND)
*/
define('GROUPS_JOIN_ALL', 2);
/**
* Determines if a group with a given groupid exists.
@@ -983,15 +997,16 @@ function groups_group_visible($groupid, $course, $cm = null, $userid = null) {
*
* @param int|array $groupids Where this is an array of multiple groups, it will match on members of any of the groups
* @param context $context Course context or a context within a course. Mandatory when $groupid = USERSWITHOUTGROUP
* @param int $groupsjointype Join type logic used. Defaults to 'Any' (logical OR).
* @return array($sql, $params)
* @throws coding_exception if empty or invalid context submitted when $groupid = USERSWITHOUTGROUP
*/
function groups_get_members_ids_sql($groupids, context $context = null) {
function groups_get_members_ids_sql($groupids, context $context = null, $groupsjointype = GROUPS_JOIN_ANY) {
if (!is_array($groupids)) {
$groupids = [$groupids];
}
$groupjoin = groups_get_members_join($groupids, 'u.id', $context);
$groupjoin = groups_get_members_join($groupids, 'u.id', $context, $groupsjointype);
$sql = "SELECT DISTINCT u.id
FROM {user} u
@@ -1010,10 +1025,11 @@ function groups_get_members_ids_sql($groupids, context $context = null) {
* @param int|array $groupids The groupids, 0 or [] means all groups and USERSWITHOUTGROUP no group
* @param string $useridcolumn The column of the user id from the calling SQL, e.g. u.id
* @param context $context Course context or a context within a course. Mandatory when $groupids includes USERSWITHOUTGROUP
* @param int $jointype Join type logic used. Defaults to 'Any' (logical OR).
* @return \core\dml\sql_join Contains joins, wheres, params
* @throws coding_exception if empty or invalid context submitted when $groupid = USERSWITHOUTGROUP
*/
function groups_get_members_join($groupids, $useridcolumn, context $context = null) {
function groups_get_members_join($groupids, $useridcolumn, context $context = null, int $jointype = GROUPS_JOIN_ANY) {
global $DB;
// Use unique prefix just in case somebody makes some SQL magic with the result.
@@ -1025,43 +1041,100 @@ function groups_get_members_join($groupids, $useridcolumn, context $context = nu
$groupids = $groupids ? [$groupids] : [];
}
$join = '';
$where = '';
$param = [];
$coursecontext = (!empty($context)) ? $context->get_course_context() : null;
if (in_array(USERSWITHOUTGROUP, $groupids) && empty($coursecontext)) {
// Throw an exception if $context is empty or invalid because it's needed to get the users without any group.
throw new coding_exception('Missing or wrong $context parameter in an attempt to get members without any group');
}
// Handle cases where we need to include users not in any groups.
// Handle cases where we need to include/exclude users not in any groups.
if (($nogroupskey = array_search(USERSWITHOUTGROUP, $groupids)) !== false) {
// Get members without any group.
$join = "LEFT JOIN (
SELECT g.courseid, m.groupid, m.userid
FROM {groups_members} m
JOIN {groups} g ON g.id = m.groupid
) {$prefix}gm ON ({$prefix}gm.userid = {$useridcolumn} AND {$prefix}gm.courseid = :{$prefix}gcourseid)";
$where = "{$prefix}gm.userid IS NULL";
$param = ["{$prefix}gcourseid" => $coursecontext->instanceid];
unset($groupids[$nogroupskey]);
$join .= "LEFT JOIN (
SELECT g.courseid, m.groupid, m.userid
FROM {groups_members} m
JOIN {groups} g ON g.id = m.groupid
) {$prefix}gm ON ({$prefix}gm.userid = {$useridcolumn} AND {$prefix}gm.courseid = :{$prefix}gcourseid)";
// Handle any groups that also need to be included (eg searching for users in no groups OR within specified groups).
if (!empty($groupids)) {
list($groupssql, $groupsparams) = $DB->get_in_or_equal($groupids, SQL_PARAMS_NAMED, $prefix);
$join .= "LEFT JOIN {groups_members} {$prefix}gm2
ON ({$prefix}gm2.userid = {$useridcolumn} AND {$prefix}gm2.groupid {$groupssql})";
// TODO: This only handles 'Any' (logical OR) of the provided groups. MDL-68348 will add 'All' and 'None' support.
$where = "({$where} OR {$prefix}gm2.userid IS NOT NULL)";
$param = array_merge($param, $groupsparams);
// Join type 'None' when filtering by 'no groups' means match users in at least one group.
if ($jointype == GROUPS_JOIN_NONE) {
$where = "{$prefix}gm.userid IS NOT NULL";
} else {
// All other cases need to match users not in any group.
$where = "{$prefix}gm.userid IS NULL";
}
} else {
// Get members of defined group IDs only.
list($groupssql, $param) = $DB->get_in_or_equal($groupids, SQL_PARAMS_NAMED, $prefix);
$param = ["{$prefix}gcourseid" => $coursecontext->instanceid];
unset($groupids[$nogroupskey]);
}
// TODO: This only handles 'Any' (logical OR) of the provided groups. MDL-68348 will add 'All' and 'None' support.
$join = "JOIN {groups_members} {$prefix}gm
ON ({$prefix}gm.userid = {$useridcolumn} AND {$prefix}gm.groupid {$groupssql})";
$where = '';
// Handle any specified groups that need to be included.
if (!empty($groupids)) {
switch ($jointype) {
case GROUPS_JOIN_ALL:
// Handle matching all of the provided groups (logical AND).
$joinallwheres = [];
$aliaskey = 0;
foreach ($groupids as $groupid) {
$gmalias = "{$prefix}gm{$aliaskey}";
$aliaskey++;
$join .= "LEFT JOIN {groups_members} {$gmalias}
ON ({$gmalias}.userid = {$useridcolumn} AND {$gmalias}.groupid = :{$gmalias}param)";
$joinallwheres[] = "{$gmalias}.userid IS NOT NULL";
$param["{$gmalias}param"] = $groupid;
}
// Members of all of the specified groups only.
if (empty($where)) {
$where = '(' . implode(' AND ', $joinallwheres) . ')';
} else {
// Members of the specified groups and also no groups.
// NOTE: This will always return no results, because you cannot be in specified groups and also be in no groups.
$where = '(' . $where . ' AND ' . implode(' AND ', $joinallwheres) . ')';
}
break;
case GROUPS_JOIN_ANY:
// Handle matching any of the provided groups (logical OR).
list($groupssql, $groupsparams) = $DB->get_in_or_equal($groupids, SQL_PARAMS_NAMED, $prefix);
$join .= "LEFT JOIN {groups_members} {$prefix}gm2
ON ({$prefix}gm2.userid = {$useridcolumn} AND {$prefix}gm2.groupid {$groupssql})";
$param = array_merge($param, $groupsparams);
// Members of any of the specified groups only.
if (empty($where)) {
$where = "{$prefix}gm2.userid IS NOT NULL";
} else {
// Members of any of the specified groups or no groups.
$where = "({$where} OR {$prefix}gm2.userid IS NOT NULL)";
}
break;
case GROUPS_JOIN_NONE:
// Handle matching none of the provided groups (logical NOT).
list($groupssql, $groupsparams) = $DB->get_in_or_equal($groupids, SQL_PARAMS_NAMED, $prefix);
$join .= "LEFT JOIN {groups_members} {$prefix}gm2
ON ({$prefix}gm2.userid = {$useridcolumn} AND {$prefix}gm2.groupid {$groupssql})";
$param = array_merge($param, $groupsparams);
// Members of none of the specified groups only.
if (empty($where)) {
$where = "{$prefix}gm2.userid IS NULL";
} else {
// Members of any unspecified groups (not a member of the specified groups, and not a member of no groups).
$where = "({$where} AND {$prefix}gm2.userid IS NULL)";
}
break;
}
}
return new \core\dml\sql_join($join, $where, $param);
+108
View File
@@ -260,6 +260,114 @@ class core_grouplib_testcase extends advanced_testcase {
$this->assertTrue(array_key_exists($student2->id, $users));
}
public function test_groups_get_members_ids_sql_multiple_groups_join_types() {
global $DB;
$this->resetAfterTest(true);
$generator = $this->getDataGenerator();
$course = $generator->create_course();
$student1 = $generator->create_user();
$student2 = $generator->create_user();
$student3 = $generator->create_user();
$student4 = $generator->create_user();
$student5 = $generator->create_user();
$student6 = $generator->create_user();
$plugin = enrol_get_plugin('manual');
$role = $DB->get_record('role', array('shortname' => 'student'));
$group1 = $generator->create_group(array('courseid' => $course->id));
$group2 = $generator->create_group(array('courseid' => $course->id));
$group3 = $generator->create_group(array('courseid' => $course->id));
// Only groups 1 and 2 specified in SQL (group 3 helps cover the None case).
$groupids = [
$group1->id,
$group2->id,
];
$instance = $DB->get_record('enrol', array(
'courseid' => $course->id,
'enrol' => 'manual',
));
$this->assertNotEquals($instance, false);
// Enrol users in the course.
$plugin->enrol_user($instance, $student1->id, $role->id);
$plugin->enrol_user($instance, $student2->id, $role->id);
$plugin->enrol_user($instance, $student3->id, $role->id);
$plugin->enrol_user($instance, $student4->id, $role->id);
$plugin->enrol_user($instance, $student5->id, $role->id);
$plugin->enrol_user($instance, $student6->id, $role->id);
// Generate SQL with the different groups join types for members of group1 and group2.
list($sqlany, $paramsany) = groups_get_members_ids_sql($groupids, null, GROUPS_JOIN_ANY);
list($sqlall, $paramsall) = groups_get_members_ids_sql($groupids, null, GROUPS_JOIN_ALL);
list($sqlnone, $paramsnone) = groups_get_members_ids_sql($groupids, null, GROUPS_JOIN_NONE);
// Any - Test empty groups, no matches.
$users = $DB->get_records_sql($sqlany, $paramsany);
$this->assertFalse(array_key_exists($student1->id, $users));
$this->assertFalse(array_key_exists($student2->id, $users));
$this->assertFalse(array_key_exists($student3->id, $users));
$this->assertFalse(array_key_exists($student4->id, $users));
$this->assertFalse(array_key_exists($student5->id, $users));
$this->assertFalse(array_key_exists($student6->id, $users));
// All - Test empty groups, no matches.
$users = $DB->get_records_sql($sqlall, $paramsall);
$this->assertFalse(array_key_exists($student1->id, $users));
$this->assertFalse(array_key_exists($student2->id, $users));
$this->assertFalse(array_key_exists($student3->id, $users));
$this->assertFalse(array_key_exists($student4->id, $users));
$this->assertFalse(array_key_exists($student5->id, $users));
$this->assertFalse(array_key_exists($student6->id, $users));
// None - Test empty groups, all match.
$users = $DB->get_records_sql($sqlnone, $paramsnone);
$this->assertTrue(array_key_exists($student1->id, $users));
$this->assertTrue(array_key_exists($student2->id, $users));
$this->assertTrue(array_key_exists($student3->id, $users));
$this->assertTrue(array_key_exists($student4->id, $users));
$this->assertTrue(array_key_exists($student5->id, $users));
$this->assertTrue(array_key_exists($student6->id, $users));
// Assign various group member combinations.
groups_add_member($group1->id, $student1->id);
groups_add_member($group1->id, $student2->id);
groups_add_member($group1->id, $student3->id);
groups_add_member($group2->id, $student2->id);
groups_add_member($group2->id, $student3->id);
groups_add_member($group2->id, $student4->id);
groups_add_member($group3->id, $student5->id);
// Any - Test students in one or both of groups 1 and 2 matched.
$users = $DB->get_records_sql($sqlany, $paramsany);
$this->assertTrue(array_key_exists($student1->id, $users));
$this->assertTrue(array_key_exists($student2->id, $users));
$this->assertTrue(array_key_exists($student3->id, $users));
$this->assertTrue(array_key_exists($student4->id, $users));
$this->assertFalse(array_key_exists($student5->id, $users));
$this->assertFalse(array_key_exists($student6->id, $users));
// All - Test only students in both groups 1 and 2 matched.
$users = $DB->get_records_sql($sqlall, $paramsall);
$this->assertTrue(array_key_exists($student2->id, $users));
$this->assertTrue(array_key_exists($student3->id, $users));
$this->assertFalse(array_key_exists($student1->id, $users));
$this->assertFalse(array_key_exists($student4->id, $users));
$this->assertFalse(array_key_exists($student5->id, $users));
$this->assertFalse(array_key_exists($student6->id, $users));
// None - Test only students not in group 1 or 2 matched.
$users = $DB->get_records_sql($sqlnone, $paramsnone);
$this->assertTrue(array_key_exists($student5->id, $users));
$this->assertTrue(array_key_exists($student6->id, $users));
$this->assertFalse(array_key_exists($student1->id, $users));
$this->assertFalse(array_key_exists($student2->id, $users));
$this->assertFalse(array_key_exists($student3->id, $users));
$this->assertFalse(array_key_exists($student4->id, $users));
}
public function test_groups_get_members_ids_sql_valid_context() {
global $DB;
+27 -2
View File
@@ -131,7 +131,7 @@ class participants_search {
* @return array
*/
protected function get_participants_sql(string $additionalwhere, array $additionalparams): array {
$isfrontpage = ($this->courseid == SITEID);
$isfrontpage = ($this->course->id == SITEID);
$accesssince = 0;
// Whether to match on users who HAVE accessed since the given time (ie false is 'inactive for more than x').
$matchaccesssince = false;
@@ -317,7 +317,7 @@ class participants_search {
// Prepare any groups filtering.
if ($groupids) {
$groupjoin = groups_get_members_join($groupids, $uid, $this->context);
$groupjoin = groups_get_members_join($groupids, $uid, $this->context, $this->get_groups_jointype());
$joins[] = $groupjoin->joins;
$params = array_merge($params, $groupjoin->params);
if (!empty($groupjoin->wheres)) {
@@ -458,6 +458,31 @@ class participants_search {
return new \core\dml\sql_join($joins, $wheres, $params);
}
/**
* Fetch the groups filter's grouplib jointype, based on its filterset jointype.
* This mapping is to ensure compatibility between the two, should their values ever differ.
*
* @return int
*/
protected function get_groups_jointype(): int {
$groupsfilter = $this->filterset->get_filter('groups');
switch ($groupsfilter->get_join_type()) {
case $groupsfilter::JOINTYPE_NONE:
$groupsjoin = GROUPS_JOIN_NONE;
break;
case $groupsfilter::JOINTYPE_ALL:
$groupsjoin = GROUPS_JOIN_ALL;
break;
default:
// Default to ANY jointype.
$groupsjoin = GROUPS_JOIN_ANY;
break;
}
return $groupsjoin;
}
/**
* Prepare SQL where clause and associated parameters for any roles filtering being performed.
*
@@ -1132,6 +1132,324 @@ class participants_search_test extends advanced_testcase {
return $finaltests;
}
/**
* Ensure that the groups filter works as expected with the provided test cases.
*
* @param array $usersdata The list of users to create
* @param array $groupsavailable The names of groups that should be created in the course
* @param array $filtergroups The names of groups to filter by
* @param int $jointype The join type to use when combining filter values
* @param int $count The expected count
* @param array $expectedusers
* @dataProvider groups_provider
*/
public function test_groups_filter(array $usersdata, array $groupsavailable, array $filtergroups, int $jointype, int $count,
array $expectedusers): void {
$course = $this->getDataGenerator()->create_course();
$coursecontext = context_course::instance($course->id);
$users = [];
// Prepare data for filtering by users in no groups.
$nogroupsdata = (object) [
'id' => USERSWITHOUTGROUP,
];
// Map group names to group data.
$groupsdata = ['nogroups' => $nogroupsdata];
foreach ($groupsavailable as $groupname) {
$groupinfo = [
'courseid' => $course->id,
'name' => $groupname,
];
$groupsdata[$groupname] = $this->getDataGenerator()->create_group($groupinfo);
}
foreach ($usersdata as $username => $userdata) {
$user = $this->getDataGenerator()->create_user(['username' => $username]);
$this->getDataGenerator()->enrol_user($user->id, $course->id, 'student');
if (array_key_exists('groups', $userdata)) {
foreach ($userdata['groups'] as $groupname) {
$userinfo = [
'userid' => $user->id,
'groupid' => (int) $groupsdata[$groupname]->id,
];
$this->getDataGenerator()->create_group_member($userinfo);
}
}
$users[$username] = $user;
}
// Create a secondary course with users. We should not see these users.
$this->create_course_with_users(1, 1, 1, 1);
// Create the basic filter.
$filterset = new participants_filterset();
$filterset->add_filter(new integer_filter('courseid', null, [(int) $course->id]));
// Create the groups filter.
$groupsfilter = new integer_filter('groups');
$filterset->add_filter($groupsfilter);
// Configure the filter.
foreach ($filtergroups as $filtergroupname) {
$groupsfilter->add_filter_value((int) $groupsdata[$filtergroupname]->id);
}
$groupsfilter->set_join_type($jointype);
// Run the search.
$search = new participants_search($course, $coursecontext, $filterset);
$rs = $search->get_participants();
$this->assertInstanceOf(moodle_recordset::class, $rs);
$records = $this->convert_recordset_to_array($rs);
$this->assertCount($count, $records);
$this->assertEquals($count, $search->get_total_participants_count());
foreach ($expectedusers as $expecteduser) {
$this->assertArrayHasKey($users[$expecteduser]->id, $records);
}
}
/**
* Data provider for groups filter tests.
*
* @return array
*/
public function groups_provider(): array {
$tests = [
'Users in different groups' => (object) [
'groupsavailable' => [
'groupa',
'groupb',
'groupc',
],
'users' => [
'a' => [
'groups' => ['groupa'],
],
'b' => [
'groups' => ['groupb'],
],
'c' => [
'groups' => ['groupa', 'groupb'],
],
'd' => [
'groups' => [],
],
],
'expect' => [
// Tests for jointype: ANY.
'ANY: No filter' => (object) [
'groups' => [],
'jointype' => filter::JOINTYPE_ANY,
'count' => 4,
'expectedusers' => [
'a',
'b',
'c',
'd',
],
],
'ANY: Filter on a single group' => (object) [
'groups' => ['groupa'],
'jointype' => filter::JOINTYPE_ANY,
'count' => 2,
'expectedusers' => [
'a',
'c',
],
],
'ANY: Filter on a group with no members' => (object) [
'groups' => ['groupc'],
'jointype' => filter::JOINTYPE_ANY,
'count' => 0,
'expectedusers' => [],
],
'ANY: Filter on multiple groups' => (object) [
'groups' => ['groupa', 'groupb'],
'jointype' => filter::JOINTYPE_ANY,
'count' => 3,
'expectedusers' => [
'a',
'b',
'c',
],
],
'ANY: Filter on members of no groups only' => (object) [
'groups' => ['nogroups'],
'jointype' => filter::JOINTYPE_ANY,
'count' => 1,
'expectedusers' => [
'd',
],
],
'ANY: Filter on a single group or no groups' => (object) [
'groups' => ['groupa', 'nogroups'],
'jointype' => filter::JOINTYPE_ANY,
'count' => 3,
'expectedusers' => [
'a',
'c',
'd',
],
],
'ANY: Filter on multiple groups or no groups' => (object) [
'groups' => ['groupa', 'groupb', 'nogroups'],
'jointype' => filter::JOINTYPE_ANY,
'count' => 4,
'expectedusers' => [
'a',
'b',
'c',
'd',
],
],
// Tests for jointype: ALL.
'ALL: No filter' => (object) [
'groups' => [],
'jointype' => filter::JOINTYPE_ALL,
'count' => 4,
'expectedusers' => [
'a',
'b',
'c',
'd',
],
],
'ALL: Filter on a single group' => (object) [
'groups' => ['groupa'],
'jointype' => filter::JOINTYPE_ALL,
'count' => 2,
'expectedusers' => [
'a',
'c',
],
],
'ALL: Filter on a group with no members' => (object) [
'groups' => ['groupc'],
'jointype' => filter::JOINTYPE_ALL,
'count' => 0,
'expectedusers' => [],
],
'ALL: Filter on members of no groups only' => (object) [
'groups' => ['nogroups'],
'jointype' => filter::JOINTYPE_ALL,
'count' => 1,
'expectedusers' => [
'd',
],
],
'ALL: Filter on multiple groups' => (object) [
'groups' => ['groupa', 'groupb'],
'jointype' => filter::JOINTYPE_ALL,
'count' => 1,
'expectedusers' => [
'c',
],
],
'ALL: Filter on a single group and no groups' => (object) [
'groups' => ['groupa', 'nogroups'],
'jointype' => filter::JOINTYPE_ALL,
'count' => 0,
'expectedusers' => [],
],
'ALL: Filter on multiple groups and no groups' => (object) [
'groups' => ['groupa', 'groupb', 'nogroups'],
'jointype' => filter::JOINTYPE_ALL,
'count' => 0,
'expectedusers' => [],
],
// Tests for jointype: NONE.
'NONE: No filter' => (object) [
'groups' => [],
'jointype' => filter::JOINTYPE_NONE,
'count' => 4,
'expectedusers' => [
'a',
'b',
'c',
'd',
],
],
'NONE: Filter on a single group' => (object) [
'groups' => ['groupa'],
'jointype' => filter::JOINTYPE_NONE,
'count' => 2,
'expectedusers' => [
'b',
'd',
],
],
'NONE: Filter on a group with no members' => (object) [
'groups' => ['groupc'],
'jointype' => filter::JOINTYPE_NONE,
'count' => 4,
'expectedusers' => [
'a',
'b',
'c',
'd',
],
],
'NONE: Filter on members of no groups only' => (object) [
'groups' => ['nogroups'],
'jointype' => filter::JOINTYPE_NONE,
'count' => 3,
'expectedusers' => [
'a',
'b',
'c',
],
],
'NONE: Filter on multiple groups' => (object) [
'groups' => ['groupa', 'groupb'],
'jointype' => filter::JOINTYPE_NONE,
'count' => 1,
'expectedusers' => [
'd',
],
],
'NONE: Filter on a single group and no groups' => (object) [
'groups' => ['groupa', 'nogroups'],
'jointype' => filter::JOINTYPE_NONE,
'count' => 1,
'expectedusers' => [
'b',
],
],
'NONE: Filter on multiple groups and no groups' => (object) [
'groups' => ['groupa', 'groupb', 'nogroups'],
'jointype' => filter::JOINTYPE_NONE,
'count' => 0,
'expectedusers' => [],
],
],
],
];
$finaltests = [];
foreach ($tests as $testname => $testdata) {
foreach ($testdata->expect as $expectname => $expectdata) {
$finaltests["{$testname} => {$expectname}"] = [
'users' => $testdata->users,
'groupsavailable' => $testdata->groupsavailable,
'filtergroups' => $expectdata->groups,
'jointype' => $expectdata->jointype,
'count' => $expectdata->count,
'expectedusers' => $expectdata->expectedusers,
];
}
}
return $finaltests;
}
/**
* Ensure that the last access filter works as expected with the provided test cases.
*