From e481f05c0da39e61100d8d952dbc54f3b2debd8d Mon Sep 17 00:00:00 2001
From: Ankit Agarwal <ankit@moodle.com>
Date: Wed, 10 Jul 2013 10:49:11 +0800
Subject: [PATCH] MDL-40354 groups: Fix issues in groups_group_visible()

---
 lib/grouplib.php            | 20 ++++++++++----------
 lib/tests/grouplib_test.php |  4 ++++
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/lib/grouplib.php b/lib/grouplib.php
index be99f74d335..528bf5819af 100644
--- a/lib/grouplib.php
+++ b/lib/grouplib.php
@@ -835,17 +835,17 @@ function groups_group_visible($groupid, $course, $cm = null, $userid = null) {
         return true;
     }
 
-    // Group mode is separate, check if user can see requested group.
-    $groups = empty($cm) ? groups_get_all_groups($course->id, $userid) : groups_get_activity_allowed_groups($cm, $userid);
-    if (array_key_exists($groupid, $groups)) {
-        // User can see the group.
+    $context = empty($cm) ? context_course::instance($course->id) : context_module::instance($cm->id);
+    if (has_capability('moodle/site:accessallgroups', $context, $userid)) {
+        // User can see everything. Groupid = 0 is handled here as well.
         return true;
-    }
-
-    // User wants to see all groups.
-    if ($groupid == 0) {
-        $context = empty($cm) ? context_course::instance($course->id) : context_module::instance($cm->id);
-        return has_capability('moodle/site:accessallgroups', $context, $userid);
+    } else if ($groupid != 0) {
+        // Group mode is separate, and user doesn't have access all groups capability. Check if user can see requested group.
+        $groups = empty($cm) ? groups_get_all_groups($course->id, $userid) : groups_get_activity_allowed_groups($cm, $userid);
+        if (array_key_exists($groupid, $groups)) {
+            // User can see the group.
+            return true;
+        }
     }
     return false;
 }
diff --git a/lib/tests/grouplib_test.php b/lib/tests/grouplib_test.php
index 2b6ab8fdfdb..82b2bcb9260 100644
--- a/lib/tests/grouplib_test.php
+++ b/lib/tests/grouplib_test.php
@@ -546,6 +546,8 @@ class grouplib_testcase extends advanced_testcase {
         $this->assertFalse($result); // Requesting all groups.
         $result = groups_group_visible(0, $course, null, $user3->id);
         $this->assertTrue($result); // Requesting all groups.
+        $result = groups_group_visible($group1->id, $course, null, $user3->id);
+        $this->assertTrue($result); // Make sure user with access to all groups can see any group.
 
         $cm->groupmode = NOGROUPS;
         $result = groups_group_visible($group1->id, $course, $cm, $user1->id);
@@ -556,6 +558,8 @@ class grouplib_testcase extends advanced_testcase {
         $this->assertFalse($result); // Cm with separate groups.
         $result = groups_group_visible($group1->id, $course, $cm, $user2->id);
         $this->assertTrue($result); // Cm with separate groups.
+        $result = groups_group_visible($group1->id, $course, $cm, $user3->id);
+        $this->assertTrue($result); // Make sure user with access to all groups can see any group.
 
         $cm->groupmode = VISIBLEGROUPS;
         $result = groups_group_visible($group1->id, $course, $cm, $user1->id);