From 6a8f6dd9259730e8c6a068d41f748bda0b47c10b Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Wed, 22 May 2024 09:18:41 +0100 Subject: [PATCH] MDL-81850 availability_group: correctly retrieve groups for given user. --- .upgradenotes/MDL-81850-2024053009132487.yml | 8 +++++++ availability/classes/info.php | 22 ++++++++++--------- .../condition/group/classes/condition.php | 2 +- .../condition/group/tests/condition_test.php | 11 +++++++--- 4 files changed, 29 insertions(+), 14 deletions(-) create mode 100644 .upgradenotes/MDL-81850-2024053009132487.yml diff --git a/.upgradenotes/MDL-81850-2024053009132487.yml b/.upgradenotes/MDL-81850-2024053009132487.yml new file mode 100644 index 00000000000..721c8f4c6f3 --- /dev/null +++ b/.upgradenotes/MDL-81850-2024053009132487.yml @@ -0,0 +1,8 @@ +issueNumber: MDL-81850 +notes: + availability: + - message: >- + The base class `info::get_groups` method has a `$userid` parameter to + specify for which user you want to retrieve course groups (defaults to + current user) + type: changed diff --git a/availability/classes/info.php b/availability/classes/info.php index 35443425b95..52b64a2c292 100644 --- a/availability/classes/info.php +++ b/availability/classes/info.php @@ -49,8 +49,8 @@ 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 The groups each user belongs to. */ + protected $groups = []; /** @var array|null Array of information about current restore if any */ protected static $restoreinfo = null; @@ -61,14 +61,12 @@ abstract class info { * @param \stdClass $course Course object * @param int $visible Value of visible flag (eye icon) * @param string $availability Availability definition (JSON format) or null - * @throws \coding_exception If data is not valid JSON format */ public function __construct($course, $visible, $availability) { // Set basic values. $this->course = $course; $this->visible = (bool)$visible; $this->availability = $availability; - $this->groups = null; } /** @@ -806,22 +804,26 @@ abstract class info { } /** - * Returns groups that the current user belongs to on the course. Note: If not already + * Returns groups that the given 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 + * @param int $userid User ID or 0 (default) for current user * @return int[] Array of int (group id) => int (same group id again); empty array if none */ - public function get_groups(int $groupingid = 0): array { + public function get_groups(int $groupingid = 0, int $userid = 0): array { global $USER; - if (is_null($this->groups)) { - $allgroups = groups_get_user_groups($this->course->id, $USER->id, true); - $this->groups = $allgroups; + if (empty($userid)) { + $userid = $USER->id; + } + if (!array_key_exists($userid, $this->groups)) { + $allgroups = groups_get_user_groups($this->course->id, $userid, true); + $this->groups[$userid] = $allgroups; } else { - $allgroups = $this->groups; + $allgroups = $this->groups[$userid]; } if (!isset($allgroups[$groupingid])) { return []; diff --git a/availability/condition/group/classes/condition.php b/availability/condition/group/classes/condition.php index feeae1c6ce3..362885de195 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_groups(); + $groups = $info->get_groups(0, $userid); if ($this->groupid) { $allow = in_array($this->groupid, $groups); } else { diff --git a/availability/condition/group/tests/condition_test.php b/availability/condition/group/tests/condition_test.php index f20a3d9a554..ca5cb2b609b 100644 --- a/availability/condition/group/tests/condition_test.php +++ b/availability/condition/group/tests/condition_test.php @@ -47,9 +47,9 @@ class condition_test extends \advanced_testcase { // Make a test course and user. $generator = $this->getDataGenerator(); $course = $generator->create_course(); - $user = $generator->create_user(); - $generator->enrol_user($user->id, $course->id); - $this->setUser($user); + $user = $generator->create_and_enrol($course); + $usertwo = $generator->create_and_enrol($course); + $info = new \core_availability\mock_info($course, $user->id); // Make 2 test groups, one in a grouping and one not. @@ -76,6 +76,8 @@ class condition_test extends \advanced_testcase { // Recheck. $this->assertTrue($cond->is_available(false, $info, true, $user->id)); $this->assertFalse($cond->is_available(true, $info, true, $user->id)); + $this->assertFalse($cond->is_available(false, $info, true, $usertwo->id)); + $this->assertTrue($cond->is_available(true, $info, true, $usertwo->id)); $information = $cond->get_description(false, true, $info); $information = \core_availability\info::format_info($information, $course); $this->assertMatchesRegularExpression('~do not belong to.*G1!~', $information); @@ -83,11 +85,14 @@ class condition_test extends \advanced_testcase { // Check group 2 works also. $cond = new condition((object)array('id' => (int)$group2->id)); $this->assertTrue($cond->is_available(false, $info, true, $user->id)); + $this->assertFalse($cond->is_available(false, $info, true, $usertwo->id)); // What about an 'any group' condition? $cond = new condition((object)array()); $this->assertTrue($cond->is_available(false, $info, true, $user->id)); $this->assertFalse($cond->is_available(true, $info, true, $user->id)); + $this->assertFalse($cond->is_available(false, $info, true, $usertwo->id)); + $this->assertTrue($cond->is_available(true, $info, true, $usertwo->id)); $information = $cond->get_description(false, true, $info); $information = \core_availability\info::format_info($information, $course); $this->assertMatchesRegularExpression('~do not belong to any~', $information);