Merge branch 'MDL-79174_master' of https://github.com/marxjohnson/moodle

availability/classes/info.php

@@ -49,6 +49,9 @@ abstract class info {
     /** @var tree Availability configuration, decoded from JSON; null if unset */
     protected $availabilitytree;
 
+    /** @var array The groups the current user belongs to. */
+    protected $groups;
+
     /** @var array|null Array of information about current restore if any */
     protected static $restoreinfo = null;
 
@@ -65,6 +68,7 @@ public function __construct($course, $visible, $availability) {
         $this->course = $course;
         $this->visible = (bool)$visible;
         $this->availability = $availability;
+        $this->groups = null;
     }
 
     /**
@@ -800,4 +804,28 @@ public static function completion_value_used($course, $cmid) {
         }
         return false;
     }
+
+    /**
+     * Returns groups that the current user belongs to on the course. Note: If not already
+     * available, this may make a database query.
+     *
+     * This will include groups the user is not allowed to see themselves, so check visibility
+     * before displaying groups to the user.
+     *
+     * @param int $groupingid Grouping ID or 0 (default) for all groups
+     * @return int[] Array of int (group id) => int (same group id again); empty array if none
+     */
+    public function get_groups(int $groupingid = 0): array {
+        global $USER;
+        if (is_null($this->groups)) {
+            $allgroups = groups_get_user_groups($this->course->id, $USER->id, true);
+            $this->groups = $allgroups;
+        } else {
+            $allgroups = $this->groups;
+        }
+        if (!isset($allgroups[$groupingid])) {
+            return [];
+        }
+        return $allgroups[$groupingid];
+    }
 }

availability/condition/group/classes/condition.php

@@ -71,7 +71,7 @@ public function is_available($not, \core_availability\info $info, $grabthelot, $
         $allow = true;
         if (!has_capability('moodle/site:accessallgroups', $context, $userid)) {
             // Get all groups the user belongs to.
-            $groups = $info->get_modinfo()->get_groups();
+            $groups = $info->get_groups();
             if ($this->groupid) {
                 $allow = in_array($this->groupid, $groups);
             } else {

availability/condition/group/tests/behat/availability_group.feature

@@ -108,3 +108,49 @@ Feature: availability_group
     When I am on the "C1" "Course" page logged in as "student1"
     Then I should see "Not available unless: You belong to G-One"
     And I should not see "G-Un"
+
+  @javascript
+  Scenario: Condition using a hidden group
+    And the following "groups" exist:
+      | name         | course | idnumber | visibility |
+      | Hidden Group | C1     | GA       | 3          |
+    And I log in as "teacher1"
+    And I add a page activity to course "Course 1" section "1"
+    And I expand all fieldsets
+
+    # Page P1 any group.
+    And I am on the "P1" "page activity editing" page
+    And I expand all fieldsets
+    And I click on "Add restriction..." "button"
+    Then "Group" "button" should exist in the "Add restriction..." "dialogue"
+    Given I click on "Group" "button" in the "Add restriction..." "dialogue"
+    And I set the field "Group" to "(Any group)"
+    And I click on ".availability-item .availability-eye img" "css_element"
+    And I click on "Save and return to course" "button"
+
+    # Page P2 with hidden group.
+    And I am on the "P2" "page activity editing" page
+    And I expand all fieldsets
+    And I click on "Add restriction..." "button"
+    And I click on "Group" "button" in the "Add restriction..." "dialogue"
+    And I set the field "Group" to "Hidden Group"
+    And I click on "Save and return to course" "button"
+
+    # Log back in as student.
+    When I am on the "Course 1" "course" page logged in as "student1"
+
+    # No pages should appear yet.
+    Then I should not see "P1" in the "region-main" "region"
+    And I should not see "P2" in the "region-main" "region"
+    And I should not see "Hidden Group"
+
+    # Add to groups and log out/in again.
+    Given the following "group members" exist:
+      | user     | group |
+      | student1 | GA    |
+    And I am on "Course 1" course homepage
+
+    # P1 (any groups) and P2 should show. The user should not see the hidden group mentioned anywhere.
+    Then I should see "P1" in the "region-main" "region"
+    And I should see "P2" in the "region-main" "region"
+    And I should not see "Hidden Group"

availability/condition/group/tests/condition_test.php

@@ -49,6 +49,7 @@ public function test_usage() {
         $course = $generator->create_course();
         $user = $generator->create_user();
         $generator->enrol_user($user->id, $course->id);
+        $this->setUser($user);
         $info = new \core_availability\mock_info($course, $user->id);
 
         // Make 2 test groups, one in a grouping and one not.
@@ -70,7 +71,7 @@ public function test_usage() {
         // Add user to groups and refresh cache.
         groups_add_member($group1, $user);
         groups_add_member($group2, $user);
-        get_fast_modinfo($course->id, 0, true);
+        $info = new \core_availability\mock_info($course, $user->id);
 
         // Recheck.
         $this->assertTrue($cond->is_available(false, $info, true, $user->id));

lib/grouplib.php

@@ -457,9 +457,12 @@ function groups_get_my_groups() {
  * @category group
  * @param int $courseid
  * @param int $userid $USER if not specified
+ * @param bool $includehidden Include groups with GROUP_VISIBILITY_NONE that the user is a member of, but is not allowed to see
+ *    themselves. Use this parameter with care - it is the responsibility of the calling code to ensure these groups are not exposed
+ *    to the user, as this could have privacy implications.
  * @return array Array[groupingid][groupid] including grouping id 0 which means all groups
  */
-function groups_get_user_groups($courseid, $userid=0) {
+function groups_get_user_groups(int $courseid, int $userid = 0, bool $includehidden = false): array {
     global $USER, $DB;
 
     if (empty($courseid)) {
@@ -471,7 +474,7 @@ function groups_get_user_groups($courseid, $userid=0) {
     }
 
     $usergroups = false;
-    $viewhidden = has_capability('moodle/course:viewhiddengroups', context_course::instance($courseid));
+    $viewhidden = $includehidden || has_capability('moodle/course:viewhiddengroups', context_course::instance($courseid));
     $viewall = \core_group\visibility::can_view_all_groups($courseid);
 
     $cache = cache::make('core', 'user_group_groupings');
@@ -483,7 +486,7 @@ function groups_get_user_groups($courseid, $userid=0) {
 
     if ($usergroups === false) {
 
-        $sql = "SELECT g.id, g.courseid, gg.groupingid
+        $sql = "SELECT g.id, g.courseid, gg.groupingid, g.visibility
                   FROM {groups} g
                   JOIN {groups_members} gm ON gm.groupid = g.id
              LEFT JOIN {groupings_groups} gg ON gg.groupid = g.id

lib/tests/grouplib_test.php

@@ -2109,6 +2109,12 @@ public function test_get_user_groups_with_visibility() {
         // Own groups - should see all groups except group with visibility::NONE.
         $usergroups1 = groups_get_user_groups($course->id, $users[1]->id);
         $this->assertEquals([$groups['all']->id, $groups['members']->id, $groups['own']->id], $usergroups1[0]);
+        // Own groups including hidden - should see all groups.
+        $usergroups1hidden = groups_get_user_groups($course->id, $users[1]->id, true);
+        $this->assertEquals(
+            [$groups['all']->id, $groups['members']->id, $groups['own']->id, $groups['none']->id],
+            $usergroups1hidden[0]
+        );
         // Fellow member of a group with visiblity::MEMBERS. Should see that group.
         $usergroups2 = groups_get_user_groups($course->id, $users[2]->id);
         $this->assertEquals([$groups['members']->id], $usergroups2[0]);
@@ -2118,6 +2124,9 @@ public function test_get_user_groups_with_visibility() {
         // Fellow member of a group with visiblity::NONE. Should not see that group.
         $usergroups4 = groups_get_user_groups($course->id, $users[4]->id);
         $this->assertEmpty($usergroups4[0]);
+        // Fellow member of a group with visiblity::NONE including hidden. Should see that group.
+        $usergroups4hidden = groups_get_user_groups($course->id, $users[4]->id, true);
+        $this->assertEquals([$groups['none']->id], $usergroups4hidden[0]);
 
         // Run as a user with viewhiddengroups. Should see all group memberships for each member.
         $this->setUser($users[5]);

lib/upgrade.txt

@@ -145,6 +145,9 @@ information provided here is intended especially for developers.
 * There is a new method called enrol_plugin::get_welcome_message_contact() that returns the contact details for the course welcome message.
 * There is a new method called enrol_plugin::send_course_welcome_message_to_user() that sends the course welcome message to a user.
 * The list of standard, and deleted, plugins is now located as JSON at lib/plugins.json.
+* groups_get_user_groups() in grouplib has a new $includehidden paramater, which will return groups for user even if they have
+  GROUP_VISIBILITY_NONE. This is false by default, and should be used with care. The calling code must ensure that these groups
+  are not exposed to the user as this may be a privacy issue.
 
 === 4.3 ===