diff --git a/availability/classes/info.php b/availability/classes/info.php index b54d919a615..ffab7b25c4f 100644 --- a/availability/classes/info.php +++ b/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 @@ abstract class info { $this->course = $course; $this->visible = (bool)$visible; $this->availability = $availability; + $this->groups = null; } /** @@ -800,4 +804,28 @@ abstract class info { } 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]; + } } diff --git a/availability/condition/group/classes/condition.php b/availability/condition/group/classes/condition.php index fd247e26053..feeae1c6ce3 100644 --- a/availability/condition/group/classes/condition.php +++ b/availability/condition/group/classes/condition.php @@ -71,7 +71,7 @@ class condition extends \core_availability\condition { $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 { diff --git a/availability/condition/group/tests/behat/availability_group.feature b/availability/condition/group/tests/behat/availability_group.feature index 57e4d4cc1a6..4d9e1a97c31 100644 --- a/availability/condition/group/tests/behat/availability_group.feature +++ b/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" diff --git a/availability/condition/group/tests/condition_test.php b/availability/condition/group/tests/condition_test.php index eb221ea9234..f20a3d9a554 100644 --- a/availability/condition/group/tests/condition_test.php +++ b/availability/condition/group/tests/condition_test.php @@ -49,6 +49,7 @@ class condition_test extends \advanced_testcase { $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 @@ class condition_test extends \advanced_testcase { // 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)); diff --git a/lib/grouplib.php b/lib/grouplib.php index 1c251f3f5df..d4c4ade52a6 100644 --- a/lib/grouplib.php +++ b/lib/grouplib.php @@ -447,9 +447,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)) { @@ -461,7 +464,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'); @@ -473,7 +476,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 diff --git a/lib/tests/grouplib_test.php b/lib/tests/grouplib_test.php index ea02fe88a85..7045b2fb4ec 100644 --- a/lib/tests/grouplib_test.php +++ b/lib/tests/grouplib_test.php @@ -2109,6 +2109,12 @@ class grouplib_test extends \advanced_testcase { // 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 @@ class grouplib_test extends \advanced_testcase { // 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]); diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 7a07804319d..09826ce68c3 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -1,6 +1,11 @@ This files describes API changes in core libraries and APIs, information provided here is intended especially for developers. +=== 4.2.7 === +* 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.2.6 === * The `html_writer::select_time` method has a `$timezone` parameter to use when formatting the time parameter