From 2e138101ff2a5467d5b7849e4821a906c9af506a Mon Sep 17 00:00:00 2001 From: Laurent David Date: Thu, 18 Jan 2024 09:57:31 +0100 Subject: [PATCH] MDL-80565 report: Revert MDL-41465 changes * Partial revert of changes made in MDL-41465, while keeping the API changes made in enrol/lib. --- report/log/classes/renderable.php | 57 ++-------- report/log/tests/renderable_test.php | 158 -------------------------- report/loglive/classes/renderable.php | 11 -- report/loglive/classes/table_log.php | 17 +-- 4 files changed, 13 insertions(+), 230 deletions(-) delete mode 100644 report/log/tests/renderable_test.php diff --git a/report/log/classes/renderable.php b/report/log/classes/renderable.php index 8bf42f521ac..76433fa6515 100644 --- a/report/log/classes/renderable.php +++ b/report/log/classes/renderable.php @@ -93,9 +93,6 @@ class report_log_renderable implements renderable { /** @var table_log table log which will be used for rendering logs */ public $tablelog; - /** @var array group ids */ - public $grouplist; - /** * Constructor. * @@ -346,40 +343,30 @@ class report_log_renderable implements renderable { } /** - * Return list of groups that are used in this course. This is done when groups are used in the course - * and the user is allowed to see all groups or groups are visible anyway. If groups are used but the - * mode is separate groups and the user is not allowed to see all groups, the list contains the groups - * only, where the user is member. - * If the course uses no groups, the list is empty. + * Return list of groups. * * @return array list of groups. */ public function get_group_list() { - global $USER; - if ($this->grouplist !== null) { - return $this->grouplist; - } // No groups for system. if (empty($this->course)) { - $this->grouplist = []; - return $this->grouplist; + return array(); } $context = context_course::instance($this->course->id); - $this->grouplist = []; + $groups = array(); $groupmode = groups_get_course_groupmode($this->course); - $cgroups = []; if (($groupmode == VISIBLEGROUPS) || - ($groupmode == SEPARATEGROUPS && has_capability('moodle/site:accessallgroups', $context))) { - $cgroups = groups_get_all_groups($this->course->id); - } else if ($groupmode == SEPARATEGROUPS && !has_capability('moodle/site:accessallgroups', $context)) { - $cgroups = groups_get_all_groups($this->course->id, $USER->id); + ($groupmode == SEPARATEGROUPS and has_capability('moodle/site:accessallgroups', $context))) { + // Get all groups. + if ($cgroups = groups_get_all_groups($this->course->id)) { + foreach ($cgroups as $cgroup) { + $groups[$cgroup->id] = $cgroup->name; + } + } } - foreach ($cgroups as $cgroup) { - $this->grouplist[$cgroup->id] = $cgroup->name; - } - return $this->grouplist; + return $groups; } /** @@ -398,29 +385,9 @@ class report_log_renderable implements renderable { $limitfrom = empty($this->showusers) ? 0 : ''; $limitnum = empty($this->showusers) ? COURSE_MAX_USERS_PER_DROPDOWN + 1 : ''; $userfieldsapi = \core_user\fields::for_name(); - - // Get the groups of that course. - $groups = $this->get_group_list(); - // Check here if we are not in group mode, or in group mode but narrow the group selection - // to the group of the user. - if (empty($groups) || !empty($this->groupid) && isset($groups[(int)$this->groupid])) { - // No groups are used in that course, therefore get all users (maybe limited to one group). - $courseusers = get_enrolled_users($context, '', $this->groupid, 'u.id, ' . + $courseusers = get_enrolled_users($context, '', $this->groupid, 'u.id, ' . $userfieldsapi->get_sql('u', false, '', '', false)->selects, null, $limitfrom, $limitnum); - } else { - // The course uses groups, get the users from these groups. - $groupids = array_keys($groups); - try { - $enrolments = enrol_get_course_users($courseid, false, [], [], $groupids); - $courseusers = []; - foreach ($enrolments as $enrolment) { - $courseusers[$enrolment->id] = $enrolment; - } - } catch (Exception $e) { - $courseusers = []; - } - } if (count($courseusers) < COURSE_MAX_USERS_PER_DROPDOWN && !$this->showusers) { $this->showusers = 1; diff --git a/report/log/tests/renderable_test.php b/report/log/tests/renderable_test.php deleted file mode 100644 index 97e09a58ba3..00000000000 --- a/report/log/tests/renderable_test.php +++ /dev/null @@ -1,158 +0,0 @@ -. - -namespace report_log; - -/** - * Class report_log\renderable_test to cover functions in \report_log_renderable. - * - * @package report_log - * @copyright 2023 Stephan Robotta - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. - */ -class renderable_test extends \advanced_testcase { - - /** - * @var [stdClass] The students. - */ - private $student = []; - - /** - * @var [stdClass] The teachers. - */ - private $teacher = []; - - /** - * @var [stdClass] The groups. - */ - private $group = []; - - /** - * @var stdClass The course. - */ - private $course; - - /** - * Set up a course with two groups, three students being each in one of the groups, - * two teachers each in either group while the second teacher is also member of the other group. - * @return void - * @throws \coding_exception - */ - public function setUp(): void { - global $PAGE; - $this->course = $this->getDataGenerator()->create_course(['groupmode' => 1]); - $this->group[] = $this->getDataGenerator()->create_group(['courseid' => $this->course->id]); - $this->group[] = $this->getDataGenerator()->create_group(['courseid' => $this->course->id]); - - for ($i = 0; $i < 3; $i++) { - $this->student[$i] = $this->getDataGenerator()->create_user(); - $this->getDataGenerator()->enrol_user($this->student[$i]->id, $this->course->id, 'student'); - $this->getDataGenerator()->create_group_member([ - 'groupid' => $this->group[$i % 2]->id, - 'userid' => $this->student[$i]->id, - ]); - } - for ($i = 0; $i < 2; $i++) { - $this->teacher[$i] = $this->getDataGenerator()->create_user(); - $this->getDataGenerator()->enrol_user($this->teacher[$i]->id, $this->course->id, 'editingteacher'); - $this->getDataGenerator()->create_group_member([ - 'groupid' => $this->group[$i]->id, - 'userid' => $this->teacher[$i]->id, - ]); - } - // Make teacher2 also member of group1. - $this->getDataGenerator()->create_group_member([ - 'groupid' => $this->group[0]->id, - 'userid' => $this->teacher[1]->id, - ]); - - $PAGE->set_url('/report/log/index.php?id=' . $this->course->id); - $this->resetAfterTest(); - } - - /** - * Test report_log_renderable::get_user_list(). - * @covers \report_log_renderable::get_user_list - * @return void - */ - public function test_get_user_list() { - // Fetch all users of group 1 and the guest user. - $userids = $this->fetch_users_from_renderable((int)$this->student[0]->id); - $this->assertCount(5, $userids); - $this->assertContains((int)$this->student[0]->id, $userids); // His own group (group 1). - $this->assertNotContains((int)$this->student[1]->id, $userids); // He is in group 2. - $this->assertContains((int)$this->teacher[0]->id, $userids); // He is in group 1. - $this->assertContains((int)$this->teacher[1]->id, $userids); // He is in both groups. - - // Fetch users of all groups and the guest user. The teacher has the capability moodle/site:accessallgroups. - $this->setUser($this->teacher[1]->id); - $renderable = new \report_log_renderable("", (int)$this->course->id, $this->teacher[1]->id); - $users = $renderable->get_user_list(); - $this->assertCount(6, $users); - - // Fetch users of group 2 and the guest user. - $userids = $this->fetch_users_from_renderable((int)$this->student[1]->id); - $this->assertCount( 3, $userids); - $this->assertNotContains((int)$this->student[0]->id, $userids); - $this->assertContains((int)$this->student[1]->id, $userids); - $this->assertNotContains((int)$this->teacher[0]->id, $userids); - $this->assertContains((int)$this->teacher[1]->id, $userids); - - // Fetch users of group 2 and test user as teacher2 but limited to his group. - $userids = $this->fetch_users_from_renderable((int)$this->teacher[1]->id, (int)$this->group[1]->id); - $this->assertCount( 3, $userids); - $this->assertNotContains((int)$this->student[0]->id, $userids); - $this->assertContains((int)$this->student[1]->id, $userids); - $this->assertNotContains((int)$this->teacher[0]->id, $userids); - $this->assertContains((int)$this->teacher[1]->id, $userids); - - } - - /** - * Helper function to return a list of user ids from the renderable object. - * @param int $userid - * @param ?int $groupid - * @return array - */ - protected function fetch_users_from_renderable(int $userid, ?int $groupid = 0): array { - $this->setUser($userid); - $renderable = new \report_log_renderable( - "", (int)$this->course->id, $userid, 0, '', $groupid); - $users = $renderable->get_user_list(); - return \array_keys($users); - } - - /** - * Test report_log_renderable::get_group_list(). - * @covers \report_log_renderable::get_group_list - * @return void - */ - public function test_get_group_list() { - - // The student sees his own group only. - $this->setUser($this->student[0]->id); - $renderable = new \report_log_renderable("", (int)$this->course->id, $this->student[0]->id); - $groups = $renderable->get_group_list(); - $this->assertCount(1, $groups); - - // While the teacher is allowed to see all groups. - $this->setUser($this->teacher[0]->id); - $renderable = new \report_log_renderable("", (int)$this->course->id, $this->teacher[0]->id); - $groups = $renderable->get_group_list(); - $this->assertCount(2, $groups); - - } -} diff --git a/report/loglive/classes/renderable.php b/report/loglive/classes/renderable.php index 06e26904e15..4c094c19ab0 100644 --- a/report/loglive/classes/renderable.php +++ b/report/loglive/classes/renderable.php @@ -168,8 +168,6 @@ class report_loglive_renderable implements renderable { * @return stdClass filters */ protected function setup_filters() { - global $USER; - $readers = $this->get_readers(); // Set up filters. @@ -180,15 +178,6 @@ class report_loglive_renderable implements renderable { if (!has_capability('moodle/site:viewanonymousevents', $context)) { $filter->anonymous = 0; } - if (groups_get_course_groupmode($this->course) == SEPARATEGROUPS && - !has_capability('moodle/site:accessallgroups', $context)) { - if ($cgroups = groups_get_all_groups($this->course->id, $USER->id)) { - $filter->groups = []; - foreach ($cgroups as $cgroup) { - $filter->groups[] = (int)$cgroup->id; - } - } - } } else { $filter->courseid = 0; } diff --git a/report/loglive/classes/table_log.php b/report/loglive/classes/table_log.php index 7ab1b9c5835..19ca5e1743c 100644 --- a/report/loglive/classes/table_log.php +++ b/report/loglive/classes/table_log.php @@ -56,7 +56,6 @@ class report_loglive_table_log extends table_sql { * - int userid: user id * - int|string modid: Module id or "site_errors" to view site errors * - int groupid: Group id - * - array groups: List of group ids * - \core\log\sql_reader logreader: reader from which data will be fetched. * - int edulevel: educational level. * - string action: view action @@ -299,29 +298,14 @@ class report_loglive_table_log extends table_sql { * @param bool $useinitialsbar do you want to use the initials bar. */ public function query_db($pagesize, $useinitialsbar = true) { - global $DB; $joins = array(); $params = array(); // Set up filtering. if (!empty($this->filterparams->courseid)) { - // For a normal course, set the course filter. $joins[] = "courseid = :courseid"; $params['courseid'] = $this->filterparams->courseid; - // If we have a course, then check if the groups filter is set. - if ($this->filterparams->courseid != SITEID && !empty($this->filterparams->groups)) { - // If that's the case, limit the users to be in the groups only, defined by the filter. - $useringroups = []; - foreach ($this->filterparams->groups as $groupid) { - $gusers = groups_get_members($groupid, 'u.id'); - $useringroups = array_merge($useringroups, array_keys($gusers)); - } - $useringroups = array_unique($useringroups); - list($ugsql, $ugparams) = $DB->get_in_or_equal($useringroups, SQL_PARAMS_NAMED); - $joins[] = 'userid ' . $ugsql; - $params = array_merge($params, $ugparams); - } } if (!empty($this->filterparams->date)) { @@ -333,6 +317,7 @@ class report_loglive_table_log extends table_sql { $joins[] = "anonymous = :anon"; $params['anon'] = $this->filterparams->anonymous; } + $selector = implode(' AND ', $joins); $total = $this->filterparams->logreader->get_events_select_count($selector, $params);