diff --git a/lib/enrollib.php b/lib/enrollib.php index 2df4d50d238..4ed18bbe72f 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -1290,7 +1290,7 @@ function is_enrolled(context $context, $user = null, $withcapability = '', $only * @param string|array $capability optional, may include a capability name, or array of names. * If an array is provided then this is the equivalent of a logical 'OR', * i.e. the user needs to have one of these capabilities. - * @param int $group optional, 0 indicates no current group, otherwise the group id + * @param int $group optional, 0 indicates no current group and USERSWITHOUTGROUP users without any group; otherwise the group id * @param bool $onlyactive consider only active enrolments in enabled plugins and time restrictions * @param bool $onlysuspended inverse of onlyactive, consider only suspended enrolments * @param int $enrolid The enrolment ID. If not 0, only users enrolled using this enrolment method will be returned. @@ -1315,9 +1315,12 @@ function get_enrolled_with_capabilities_join(context $context, $prefix = '', $ca } if ($group) { - $groupjoin = groups_get_members_join($group, $uid); + $groupjoin = groups_get_members_join($group, $uid, $context); $joins[] = $groupjoin->joins; $params = array_merge($params, $groupjoin->params); + if (!empty($groupjoin->wheres)) { + $wheres[] = $groupjoin->wheres; + } } $joins = implode("\n", $joins); @@ -1335,7 +1338,7 @@ function get_enrolled_with_capabilities_join(context $context, $prefix = '', $ca * * @param context $context * @param string $withcapability - * @param int $groupid 0 means ignore groups, any other value limits the result by group id + * @param int $groupid 0 means ignore groups, USERSWITHOUTGROUP without any group and any other value limits the result by group id * @param bool $onlyactive consider only active enrolments in enabled plugins and time restrictions * @param bool $onlysuspended inverse of onlyactive, consider only suspended enrolments * @param int $enrolid The enrolment ID. If not 0, only users enrolled using this enrolment method will be returned. @@ -1461,7 +1464,7 @@ function get_enrolled_join(context $context, $useridcolumn, $onlyactive = false, * * @param context $context * @param string $withcapability - * @param int $groupid 0 means ignore groups, any other value limits the result by group id + * @param int $groupid 0 means ignore groups, USERSWITHOUTGROUP without any group and any other value limits the result by group id * @param string $userfields requested user record fields * @param string $orderby * @param int $limitfrom return a subset of records, starting at this point (optional, required if $limitnum is set). diff --git a/lib/grouplib.php b/lib/grouplib.php index 336e372dcf5..687eef1b54f 100644 --- a/lib/grouplib.php +++ b/lib/grouplib.php @@ -38,6 +38,11 @@ define('SEPARATEGROUPS', 1); */ define('VISIBLEGROUPS', 2); +/** + * This is for filtering users without any group. + */ +define('USERSWITHOUTGROUP', -1); + /** * Determines if a group with a given groupid exists. @@ -976,15 +981,20 @@ function groups_group_visible($groupid, $course, $cm = null, $userid = null) { * Get sql and parameters that will return user ids for a group * * @param int $groupid + * @param context $context Course context or a context within a course. Mandatory when $groupid = USERSWITHOUTGROUP * @return array($sql, $params) + * @throws coding_exception if empty or invalid context submitted when $groupid = USERSWITHOUTGROUP */ -function groups_get_members_ids_sql($groupid) { - $groupjoin = groups_get_members_join($groupid, 'u.id'); +function groups_get_members_ids_sql($groupid, context $context = null) { + $groupjoin = groups_get_members_join($groupid, 'u.id', $context); $sql = "SELECT DISTINCT u.id FROM {user} u $groupjoin->joins WHERE u.deleted = 0"; + if (!empty($groupjoin->wheres)) { + $sql .= ' AND '. $groupjoin->wheres; + } return array($sql, $groupjoin->params); } @@ -992,20 +1002,42 @@ function groups_get_members_ids_sql($groupid) { /** * Get sql join to return users in a group * - * @param int $groupid + * @param int $groupid The groupid, 0 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 $groupid = USERSWITHOUTGROUP * @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($groupid, $useridcolumn) { +function groups_get_members_join($groupid, $useridcolumn, context $context = null) { // Use unique prefix just in case somebody makes some SQL magic with the result. static $i = 0; $i++; $prefix = 'gm' . $i . '_'; - $join = "JOIN {groups_members} {$prefix}gm ON ({$prefix}gm.userid = $useridcolumn AND {$prefix}gm.groupid = :{$prefix}gmid)"; - $param = array("{$prefix}gmid" => $groupid); + $coursecontext = (!empty($context)) ? $context->get_course_context() : null; + if ($groupid == USERSWITHOUTGROUP && 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'); + } - return new \core\dml\sql_join($join, '', $param); + if ($groupid == USERSWITHOUTGROUP) { + // 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 = array("{$prefix}gcourseid" => $coursecontext->instanceid); + } else { + // Get members of defined groupid. + $join = "JOIN {groups_members} {$prefix}gm + ON ({$prefix}gm.userid = $useridcolumn AND {$prefix}gm.groupid = :{$prefix}gmid)"; + $where = ''; + $param = array("{$prefix}gmid" => $groupid); + } + + return new \core\dml\sql_join($join, $where, $param); } /** diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php index 3ff61927e08..f8b9a868abd 100644 --- a/lib/tests/accesslib_test.php +++ b/lib/tests/accesslib_test.php @@ -2335,6 +2335,40 @@ class core_accesslib_testcase extends advanced_testcase { } + /** + * Test that enrolled users SQL does not return any values for users + * without a group when $context is not a valid course context. + */ + public function test_get_enrolled_sql_userswithoutgroup() { + global $DB; + + $this->resetAfterTest(); + + $systemcontext = context_system::instance(); + $course = $this->getDataGenerator()->create_course(); + $coursecontext = context_course::instance($course->id); + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + + $this->getDataGenerator()->enrol_user($user1->id, $course->id); + $this->getDataGenerator()->enrol_user($user2->id, $course->id); + + $group = $this->getDataGenerator()->create_group(array('courseid' => $course->id)); + groups_add_member($group, $user1); + + $enrolled = get_enrolled_users($coursecontext); + $this->assertCount(2, $enrolled); + + // Get users without any group on the course context. + $enrolledwithoutgroup = get_enrolled_users($coursecontext, '', USERSWITHOUTGROUP); + $this->assertCount(1, $enrolledwithoutgroup); + $this->assertFalse(isset($enrolledwithoutgroup[$user1->id])); + + // Get users without any group on the system context (it should throw an exception). + $this->expectException('coding_exception'); + get_enrolled_users($systemcontext, '', USERSWITHOUTGROUP); + } + public function get_enrolled_sql_provider() { return array( array( diff --git a/lib/tests/grouplib_test.php b/lib/tests/grouplib_test.php index f9bfbeeea88..ccf66c7d008 100644 --- a/lib/tests/grouplib_test.php +++ b/lib/tests/grouplib_test.php @@ -176,7 +176,6 @@ class core_grouplib_testcase extends advanced_testcase { $this->assertEquals($grouping, groups_get_grouping_by_idnumber($course->id, $idnumber2)); } - public function test_groups_get_members_ids_sql() { global $DB; @@ -185,7 +184,9 @@ class core_grouplib_testcase extends advanced_testcase { $generator = $this->getDataGenerator(); $course = $generator->create_course(); - $student = $generator->create_user(); + $coursecontext = context_course::instance($course->id); + $student1 = $generator->create_user(); + $student2 = $generator->create_user(); $plugin = enrol_get_plugin('manual'); $role = $DB->get_record('role', array('shortname' => 'student')); $group = $generator->create_group(array('courseid' => $course->id)); @@ -196,20 +197,122 @@ class core_grouplib_testcase extends advanced_testcase { $this->assertNotEquals($instance, false); - // Enrol the user in the course. - $plugin->enrol_user($instance, $student->id, $role->id); + // Enrol users in the course. + $plugin->enrol_user($instance, $student1->id, $role->id); + $plugin->enrol_user($instance, $student2->id, $role->id); - list($sql, $params) = groups_get_members_ids_sql($group->id, true); + list($sql, $params) = groups_get_members_ids_sql($group->id); // Test an empty group. $users = $DB->get_records_sql($sql, $params); - - $this->assertFalse(array_key_exists($student->id, $users)); - groups_add_member($group->id, $student->id); + $this->assertFalse(array_key_exists($student1->id, $users)); // Test with a group member. + groups_add_member($group->id, $student1->id); $users = $DB->get_records_sql($sql, $params); - $this->assertTrue(array_key_exists($student->id, $users)); + $this->assertTrue(array_key_exists($student1->id, $users)); + } + + public function test_groups_get_members_ids_sql_valid_context() { + global $DB; + + $this->resetAfterTest(true); + + $generator = $this->getDataGenerator(); + + $course = $generator->create_course(); + $coursecontext = context_course::instance($course->id); + $student1 = $generator->create_user(); + $student2 = $generator->create_user(); + $plugin = enrol_get_plugin('manual'); + $role = $DB->get_record('role', array('shortname' => 'student')); + $group = $generator->create_group(array('courseid' => $course->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); + + // Add student1 to the group. + groups_add_member($group->id, $student1->id); + + // Test with members at any group and with a valid $context. + list($sql, $params) = groups_get_members_ids_sql(USERSWITHOUTGROUP, $coursecontext); + $users = $DB->get_records_sql($sql, $params); + $this->assertFalse(array_key_exists($student1->id, $users)); + $this->assertTrue(array_key_exists($student2->id, $users)); + } + + public function test_groups_get_members_ids_sql_empty_context() { + global $DB; + + $this->resetAfterTest(true); + + $generator = $this->getDataGenerator(); + + $course = $generator->create_course(); + $coursecontext = context_course::instance($course->id); + $student1 = $generator->create_user(); + $student2 = $generator->create_user(); + $plugin = enrol_get_plugin('manual'); + $role = $DB->get_record('role', array('shortname' => 'student')); + $group = $generator->create_group(array('courseid' => $course->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); + + // Add student1 to the group. + groups_add_member($group->id, $student1->id); + + // Test with members at any group and without the $context. + $this->expectException('coding_exception'); + list($sql, $params) = groups_get_members_ids_sql(USERSWITHOUTGROUP); + } + + public function test_groups_get_members_ids_sql_invalid_context() { + global $DB; + + $this->resetAfterTest(true); + + $generator = $this->getDataGenerator(); + + $course = $generator->create_course(); + $coursecontext = context_course::instance($course->id); + $student1 = $generator->create_user(); + $student2 = $generator->create_user(); + $plugin = enrol_get_plugin('manual'); + $role = $DB->get_record('role', array('shortname' => 'student')); + $group = $generator->create_group(array('courseid' => $course->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); + + // Add student1 to the group. + groups_add_member($group->id, $student1->id); + + // Test with members at any group and with an invalid $context. + $syscontext = context_system::instance(); + $this->expectException('coding_exception'); + list($sql, $params) = groups_get_members_ids_sql(USERSWITHOUTGROUP, $syscontext); } public function test_groups_get_group_by_name() { diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 70b6a59494b..e4e9d5b425b 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -121,6 +121,9 @@ information provided here is intended especially for developers. - phone1 - phone2 - address +* New optional parameter $context for the groups_get_members_join() function and ability to filter users that are not members of +any group. Besides, groups_get_members_ids_sql, get_enrolled_sql and get_enrolled_users now accepts -1 (USERSWITHOUTGROUP) for +the groupid field. === 3.5 === diff --git a/user/classes/participants_table.php b/user/classes/participants_table.php index 8f999f51b47..d6861180e55 100644 --- a/user/classes/participants_table.php +++ b/user/classes/participants_table.php @@ -136,7 +136,7 @@ class participants_table extends \table_sql { * Sets up the table. * * @param int $courseid - * @param int|false $currentgroup False if groups not used, int if groups used, 0 for all groups. + * @param int|false $currentgroup False if groups not used, int if groups used, 0 all groups, USERSWITHOUTGROUP for no group * @param int $accesssince The time the user last accessed the site * @param int $roleid The role we are including, 0 means all enrolled users * @param int $enrolid The applied filter for the user enrolment ID. diff --git a/user/index.php b/user/index.php index f36491cef55..fccd1df64ac 100644 --- a/user/index.php +++ b/user/index.php @@ -196,7 +196,7 @@ if (!empty($groupid)) { } } -if ($groupid && ($course->groupmode != SEPARATEGROUPS || $canaccessallgroups)) { +if ($groupid > 0 && ($course->groupmode != SEPARATEGROUPS || $canaccessallgroups)) { $grouprenderer = $PAGE->get_renderer('core_group'); $groupdetailpage = new \core_group\output\group_details($groupid); echo $grouprenderer->group_details($groupdetailpage); diff --git a/user/lib.php b/user/lib.php index bb1d75a7882..e0605a9668e 100644 --- a/user/lib.php +++ b/user/lib.php @@ -1272,7 +1272,7 @@ function user_get_tagged_users($tag, $exclusivemode = false, $fromctx = 0, $ctx * Returns the SQL used by the participants table. * * @param int $courseid The course id - * @param int $groupid The groupid, 0 means all groups + * @param int $groupid The groupid, 0 means all groups and USERSWITHOUTGROUP no group * @param int $accesssince The time since last access, 0 means any time * @param int $roleid The role id, 0 means all roles * @param int $enrolid The enrolment id, 0 means all enrolment methods will be returned. @@ -1464,7 +1464,7 @@ function user_get_participants_sql($courseid, $groupid = 0, $accesssince = 0, $r * Returns the total number of participants for a given course. * * @param int $courseid The course id - * @param int $groupid The groupid, 0 means all groups + * @param int $groupid The groupid, 0 means all groups and USERSWITHOUTGROUP no group * @param int $accesssince The time since last access, 0 means any time * @param int $roleid The role id, 0 means all roles * @param int $enrolid The applied filter for the user enrolment ID. @@ -1488,7 +1488,7 @@ function user_get_total_participants($courseid, $groupid = 0, $accesssince = 0, * Returns the participants for a given course. * * @param int $courseid The course id - * @param int $groupid The group id + * @param int $groupid The groupid, 0 means all groups and USERSWITHOUTGROUP no group * @param int $accesssince The time since last access * @param int $roleid The role id * @param int $enrolid The applied filter for the user enrolment ID. diff --git a/user/renderer.php b/user/renderer.php index 35711ce11e0..1e8461f695d 100644 --- a/user/renderer.php +++ b/user/renderer.php @@ -145,6 +145,11 @@ class core_user_renderer extends plugin_renderer_base { if (has_capability('moodle/site:accessallgroups', $context) || $course->groupmode != SEPARATEGROUPS) { // List all groups if the user can access all groups, or we are in visible group mode or no groups mode. $groups = $manager->get_all_groups(); + if (!empty($groups)) { + // Add 'No group' option, to enable filtering users without any group. + $nogroup[USERSWITHOUTGROUP] = (object)['name' => get_string('nogroup', 'group')]; + $groups = $nogroup + $groups; + } } else { // Otherwise, just list the groups the user belongs to. $groups = groups_get_all_groups($course->id, $USER->id); diff --git a/user/tests/behat/filter_participants.feature b/user/tests/behat/filter_participants.feature index 968fbbde56a..f0097f25a6c 100644 --- a/user/tests/behat/filter_participants.feature +++ b/user/tests/behat/filter_participants.feature @@ -9,6 +9,7 @@ Feature: Course participants can be filtered | fullname | shortname | groupmode | | Course 1 | C1 | 1 | | Course 2 | C2 | 0 | + | Course 3 | C3 | 0 | And the following "users" exist: | username | firstname | lastname | email | idnumber | country | city | maildisplay | | student1 | Student | 1 | student1@example.com | SID1 | | SCITY1 | 0 | @@ -25,17 +26,26 @@ Feature: Course participants can be filtered | student1 | C2 | student | 0 | | | student2 | C2 | student | 0 | | | student3 | C2 | student | 0 | | + | student1 | C3 | student | 0 | | + | student2 | C3 | student | 0 | | + | student3 | C3 | student | 0 | | | teacher1 | C1 | editingteacher | 0 | | | teacher1 | C2 | editingteacher | 0 | | + | teacher1 | C3 | editingteacher | 0 | | And the following "groups" exist: | name | course | idnumber | | Group 1 | C1 | G1 | | Group 2 | C1 | G2 | + | Group A | C3 | GA | + | Group B | C3 | GB | And the following "group members" exist: | user | group | | student2 | G1 | | student2 | G2 | | student3 | G2 | + | student1 | GA | + | student2 | GA | + | student2 | GB | @javascript Scenario: No filters applied @@ -62,12 +72,32 @@ Feature: Course participants can be filtered # Note the 'XX-IGNORE-XX' elements are for when there is less than 2 'not expected' items. Examples: | filter1 | expected1 | expected2 | expected3 | notexpected1 | notexpected2 | + | Group: No group | Student 1 | Student 4 | Teacher 1 | Student 2 | Student 3 | | Group: Group 1 | Student 2 | | | Student 1 | Student 3 | | Group: Group 2 | Student 2 | Student 3 | | Student 1 | XX-IGNORE-XX | | Role: Teacher | Teacher 1 | | | Student 1 | Student 2 | | Status: Active | Teacher 1 | Student 1 | Student 3 | Student 2 | Student 4 | | Status: Inactive | Student 2 | Student 4 | | Teacher 1 | Student 1 | + @javascript + Scenario Outline: Filter users which are group members in several courses + Given I log in as "teacher1" + And I am on "Course 3" course homepage + And I navigate to course participants + When I open the autocomplete suggestions list + And I click on "" item in the autocomplete list + Then I should see "" in the "participants" "table" + And I should see "" in the "participants" "table" + And I should see "" in the "participants" "table" + And I should not see "" in the "participants" "table" + And I should not see "" in the "participants" "table" + # Note the 'XX-IGNORE-XX' elements are for when there is less than 2 'not expected' items. + Examples: + | filter1 | expected1 | expected2 | expected3 | notexpected1 | notexpected2 | + | Group: No group | Student 3 | | | Student 1 | Student 2 | + | Group: Group A | Student 1 | Student 2 | | Student 3 | XX-IGNORE-XX | + | Group: Group B | Student 2 | | | Student 1 | Student 3 | + @javascript Scenario: Multiple filters applied Given I log in as "teacher1" diff --git a/user/tests/userlib_test.php b/user/tests/userlib_test.php index b7f6d6c4567..8516c0b658f 100644 --- a/user/tests/userlib_test.php +++ b/user/tests/userlib_test.php @@ -940,6 +940,12 @@ class core_userliblib_testcase extends advanced_testcase { $this->assertEquals($student1->id, $userset->current()->id); $this->assertEquals(1, iterator_count($userset)); + + // Search for users without any group. + $userset = user_get_participants($course->id, USERSWITHOUTGROUP, 0, $roleids['student'], 0, -1, ''); + + $this->assertEquals($student3->id, $userset->current()->id); + $this->assertEquals(1, iterator_count($userset)); } /**