diff --git a/user/selector/lib.php b/user/selector/lib.php index 34a553a008d..b8bc9bb0f0d 100644 --- a/user/selector/lib.php +++ b/user/selector/lib.php @@ -890,7 +890,15 @@ class group_non_members_selector extends groups_user_selector_base { list($searchcondition, $searchparams) = $this->search_sql($search, 'u'); // Build the SQL. - list($enrolsql, $enrolparams) = get_enrolled_sql($context); + $enrolledjoin = get_enrolled_join($context, 'u.id'); + + $wheres = []; + $wheres[] = $enrolledjoin->wheres; + $wheres[] = 'u.deleted = 0'; + $wheres[] = 'gm.id IS NULL'; + $wheres = implode(' AND ', $wheres); + $wheres .= ' AND ' . $searchcondition; + $fields = "SELECT r.id AS roleid, u.id AS userid, " . $this->required_fields_sql('u') . ", (SELECT count(igm.groupid) @@ -898,18 +906,16 @@ class group_non_members_selector extends groups_user_selector_base { JOIN {groups} ig ON igm.groupid = ig.id WHERE igm.userid = u.id AND ig.courseid = :courseid) AS numgroups"; $sql = " FROM {user} u - JOIN ($enrolsql) e ON e.id = u.id + $enrolledjoin->joins LEFT JOIN {role_assignments} ra ON (ra.userid = u.id AND ra.contextid $relatedctxsql AND ra.roleid $roleids) LEFT JOIN {role} r ON r.id = ra.roleid LEFT JOIN {groups_members} gm ON (gm.userid = u.id AND gm.groupid = :groupid) - WHERE u.deleted = 0 - AND gm.id IS NULL - AND $searchcondition"; + WHERE $wheres"; list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext); $orderby = ' ORDER BY ' . $sort; - $params = array_merge($searchparams, $roleparams, $enrolparams, $relatedctxparams); + $params = array_merge($searchparams, $roleparams, $relatedctxparams, $enrolledjoin->params); $params['courseid'] = $this->courseid; $params['groupid'] = $this->groupid; diff --git a/user/tests/group_non_members_selector_test.php b/user/tests/group_non_members_selector_test.php new file mode 100644 index 00000000000..a4837f88fa4 --- /dev/null +++ b/user/tests/group_non_members_selector_test.php @@ -0,0 +1,117 @@ +. + +/** + * Unit tests for {@link group_non_members_selector} class. + * + * @package core_user + * @category test + * @copyright 2019 Huong Nguyen + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/user/selector/lib.php'); + +/** + * Unit tests for {@link group_non_members_selector} class. + * + * @package core_user + * @category test + * @copyright 2019 Huong Nguyen + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_group_non_members_selector_testcase extends advanced_testcase { + + /** + * Test find_users that only return group non members + * + * @throws coding_exception + */ + public function test_find_users_only_return_group_non_member() { + $this->resetAfterTest(); + + // Create course. + $course = $this->getDataGenerator()->create_course(); + + // Create users. + $user1 = $this->getDataGenerator()->create_user(['firstname' => 'User', 'lastname' => '1']); + $user2 = $this->getDataGenerator()->create_user(['firstname' => 'User', 'lastname' => '2']); + $user3 = $this->getDataGenerator()->create_user(['firstname' => 'User', 'lastname' => '3']); + $user4 = $this->getDataGenerator()->create_user(['firstname' => 'User', 'lastname' => '4']); + $user5 = $this->getDataGenerator()->create_user(['firstname' => 'User', 'lastname' => '5']); + + // Create group. + $group = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + + // Enroll users to course. Except User5. + $this->getDataGenerator()->enrol_user($user1->id, $course->id); + $this->getDataGenerator()->enrol_user($user2->id, $course->id); + $this->getDataGenerator()->enrol_user($user3->id, $course->id); + $this->getDataGenerator()->enrol_user($user4->id, $course->id); + + // Assign User1 to Group. + $this->getDataGenerator()->create_group_member(['groupid' => $group->id, 'userid' => $user1->id]); + + // User1 and User5 will not exist in the result. + // User2, User3 and User4 will exist in the result. + $potentialmembersselector = new group_non_members_selector('addselect', + ['groupid' => $group->id, 'courseid' => $course->id]); + foreach ($potentialmembersselector->find_users('User') as $found) { + $this->assertCount(3, $found); + $this->assertArrayNotHasKey($user5->id, $found); + $this->assertArrayNotHasKey($user1->id, $found); + $this->assertArrayHasKey($user2->id, $found); + $this->assertArrayHasKey($user3->id, $found); + $this->assertArrayHasKey($user4->id, $found); + } + + // Assign User2 to Group. + $this->getDataGenerator()->create_group_member(['groupid' => $group->id, 'userid' => $user2->id]); + + // User1, User2 and User5 will not exist in the result. + // User3 and User4 will exist in the result. + $potentialmembersselector = new group_non_members_selector('addselect', + ['groupid' => $group->id, 'courseid' => $course->id]); + foreach ($potentialmembersselector->find_users('User') as $found) { + $this->assertCount(2, $found); + $this->assertArrayNotHasKey($user5->id, $found); + $this->assertArrayNotHasKey($user1->id, $found); + $this->assertArrayNotHasKey($user2->id, $found); + $this->assertArrayHasKey($user3->id, $found); + $this->assertArrayHasKey($user4->id, $found); + } + + // Assign User3 to Group. + $this->getDataGenerator()->create_group_member(['groupid' => $group->id, 'userid' => $user3->id]); + + // User1, User2, User3 and User5 will not exist in the result. + // Only User4 will exist in the result. + $potentialmembersselector = new group_non_members_selector('addselect', + ['groupid' => $group->id, 'courseid' => $course->id]); + foreach ($potentialmembersselector->find_users('User') as $found) { + $this->assertCount(1, $found); + $this->assertArrayNotHasKey($user5->id, $found); + $this->assertArrayNotHasKey($user1->id, $found); + $this->assertArrayNotHasKey($user2->id, $found); + $this->assertArrayNotHasKey($user3->id, $found); + $this->assertArrayHasKey($user4->id, $found); + } + } + +}