Merge branch 'MDL-79611-402' of https://github.com/paulholden/moodle …

…into MOODLE_402_STABLE
moodle · Oct 11, 2023 · 4a2b27a · 4a2b27a
2 parents 7cb3f15 + aed9018
commit 4a2b27a
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 8 deletions.
diff --git a/lib/grouplib.php b/lib/grouplib.php
@@ -636,17 +636,19 @@ function groups_has_membership($cm, $userid=null) {
  * @param int $groupid The groupid to get the users for
  * @param int $fields The fields to return
  * @param int $sort optional sorting of returned users
- * @return array|bool Returns an array of the users for the specified
- * group or false if no users or an error returned.
+ * @return array Returns an array of the users for the specified group
  */
 function groups_get_members($groupid, $fields='u.*', $sort='lastname ASC') {
-    global $DB, $USER;
+    global $DB;
 
     if (empty($groupid)) {
         return [];
     }
 
     $courseid = $DB->get_field('groups', 'courseid', ['id' => $groupid]);
+    if ($courseid === false) {
+        return [];
+    }
 
     $select = "SELECT $fields";
     $from = "FROM {user} u

diff --git a/lib/tests/grouplib_test.php b/lib/tests/grouplib_test.php
@@ -1755,6 +1755,8 @@ public function test_groups_user_groups_visible() {
 
     /**
      * Tests for groups_get_groups_members() method.
+     *
+     * @covers ::groups_get_groups_members
      */
     public function test_groups_get_groups_members() {
         $this->resetAfterTest(true);
@@ -1817,10 +1819,9 @@ public function test_groups_get_groups_members() {
         $members = groups_get_groups_members([$group3->id]);
         $this->assertCount(0, $members);
 
-        // Test groups_get_members.
-        $members = groups_get_members($group2->id, 'u.*', 'u.id ASC');
-        $this->assertCount(2, $members);
-        $this->assertEquals([$user1->id, $user2->id], array_keys($members));
+        // Our second group.
+        $members = groups_get_groups_members([$group2->id]);
+        $this->assertEqualsCanonicalizing([$user1->id, $user2->id], array_column($members, 'id'));
 
         // Test the info matches group membership for the entire course.
         $groups  = groups_get_all_groups($course2->id, 0, 0, 'g.*', true);
@@ -2182,10 +2183,34 @@ public function test_groups_is_member_with_visibility(): void {
         $this->assertTrue(groups_is_member($groups['none']->id, $users[4]->id));
     }
 
+    /**
+     * Test groups_get_members
+     *
+     * @covers ::groups_get_members
+     */
+    public function test_groups_get_members(): void {
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+        $userone = $this->getDataGenerator()->create_and_enrol($course);
+        $usertwo = $this->getDataGenerator()->create_and_enrol($course);
+
+        $group = $this->getDataGenerator()->create_group(['courseid' => $course->id]);
+        $this->getDataGenerator()->create_group_member(['groupid' => $group->id, 'userid' => $userone->id]);
+        $this->getDataGenerator()->create_group_member(['groupid' => $group->id, 'userid' => $usertwo->id]);
+
+        $users = groups_get_members($group->id);
+        $this->assertEqualsCanonicalizing([$userone->id, $usertwo->id], array_column($users, 'id'));
+
+        // Test invalid group.
+        $users = groups_get_members(-1);
+        $this->assertEmpty($users);
+    }
+
     /**
      * Test groups_get_members() using groups with different visibility settings.
      *
-     * @covers \groups_get_members()
+     * @covers ::groups_get_members
      */
     public function test_groups_get_members_with_visibility(): void {
         list($users, $groups) = $this->create_groups_with_visibilty();