diff --git a/report/log/classes/renderable.php b/report/log/classes/renderable.php index 8bf42f521ac..34ce7dfb60f 100644 --- a/report/log/classes/renderable.php +++ b/report/log/classes/renderable.php @@ -93,7 +93,11 @@ class report_log_renderable implements renderable { /** @var table_log table log which will be used for rendering logs */ public $tablelog; - /** @var array group ids */ + /** + * @var array group ids + * @deprecated since Moodle 4.4 - please do not use this public property + * @todo MDL-81155 remove this property as it is not used anymore. + */ public $grouplist; /** @@ -254,26 +258,14 @@ class report_log_renderable implements renderable { // Setup for group handling. $groupmode = groups_get_course_groupmode($this->course); if ($groupmode == SEPARATEGROUPS and !has_capability('moodle/site:accessallgroups', $context)) { - $selectedgroup = -1; - } else if ($groupmode) { - $selectedgroup = $this->groupid; - } else { - $selectedgroup = 0; - } - - if ($selectedgroup === -1) { if (isset($SESSION->currentgroup[$this->course->id])) { $selectedgroup = $SESSION->currentgroup[$this->course->id]; - } else { - $selectedgroup = groups_get_all_groups($this->course->id, $USER->id); - if (is_array($selectedgroup)) { - $groupids = array_keys($selectedgroup); - $selectedgroup = array_shift($groupids); - $SESSION->currentgroup[$this->course->id] = $selectedgroup; - } else { - $selectedgroup = 0; - } + } else if ($this->groupid > 0) { + $SESSION->currentgroup[$this->course->id] = $this->groupid; + $selectedgroup = $this->groupid; } + } else if ($groupmode) { + $selectedgroup = $this->groupid; } return $selectedgroup; } @@ -357,29 +349,24 @@ class report_log_renderable implements renderable { 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 []; } $context = context_course::instance($this->course->id); - $this->grouplist = []; $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); + $grouplist = []; + $userid = $groupmode == SEPARATEGROUPS ? $USER->id : 0; + if (has_capability('moodle/site:accessallgroups', $context)) { + $userid = 0; } - foreach ($cgroups as $cgroup) { - $this->grouplist[$cgroup->id] = $cgroup->name; + $cgroups = groups_get_all_groups($this->course->id, $userid); + if (!empty($cgroups)) { + $grouplist = array_column($cgroups, 'name', 'id'); } - return $this->grouplist; + $this->grouplist = $grouplist; // Keep compatibility with MDL-41465. + return $grouplist; } /** @@ -396,31 +383,33 @@ class report_log_renderable implements renderable { } $context = context_course::instance($courseid); $limitfrom = empty($this->showusers) ? 0 : ''; - $limitnum = empty($this->showusers) ? COURSE_MAX_USERS_PER_DROPDOWN + 1 : ''; + $limitnum = empty($this->showusers) ? COURSE_MAX_USERS_PER_DROPDOWN + 1 : ''; $userfieldsapi = \core_user\fields::for_name(); - // Get the groups of that course. + // Get the groups of that course that the user can see. $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, ' . - $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 = []; + $groupids = array_keys($groups); + // Now doublecheck the value of groupids and deal with special case like USERWITHOUTGROUP. + $groupmode = groups_get_course_groupmode($this->course); + if ( + has_capability('moodle/site:accessallgroups', $context) + || $groupmode != SEPARATEGROUPS + || empty($groupids) + ) { + $groupids[] = USERSWITHOUTGROUP; + } + // First case, the user has selected a group and user is in this group. + if ($this->groupid > 0) { + if (!isset($groups[$this->groupid])) { + // The user is not in this group, so we will ignore the group selection. + $groupids = 0; + } else { + $groupids = [$this->groupid]; } } + $courseusers = get_enrolled_users($context, '', $groupids, 'u.id, ' . + $userfieldsapi->get_sql('u', false, '', '', false)->selects, + null, $limitfrom, $limitnum); if (count($courseusers) < COURSE_MAX_USERS_PER_DROPDOWN && !$this->showusers) { $this->showusers = 1; diff --git a/report/log/classes/table_log.php b/report/log/classes/table_log.php index e64dfad0267..4cb1da64d1b 100644 --- a/report/log/classes/table_log.php +++ b/report/log/classes/table_log.php @@ -23,7 +23,8 @@ */ defined('MOODLE_INTERNAL') || die; - +global $CFG; +require_once($CFG->libdir . '/tablelib.php'); /** * Table log class for displaying logs. * @@ -403,7 +404,7 @@ class report_log_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; + global $DB, $USER; $joins = array(); $params = array(); @@ -438,14 +439,35 @@ class report_log_table_log extends table_sql { } // Getting all members of a group. - if ($groupid and empty($this->filterparams->userid)) { - if ($gusers = groups_get_members($groupid)) { - $gusers = array_keys($gusers); - $joins[] = 'userid IN (' . implode(',', $gusers) . ')'; + if (empty($this->filterparams->userid)) { + if ($groupid) { + if ($gusers = groups_get_members($groupid)) { + $gusers = array_keys($gusers); + $joins[] = 'userid IN (' . implode(',', $gusers) . ')'; + } else { + $joins[] = 'userid = 0'; // No users in groups, so we want something that will always be false. + } } else { - $joins[] = 'userid = 0'; // No users in groups, so we want something that will always be false. + // No group selected and we are not filtering by user, so we want all users that are visible to the current user. + // If we are in a course, then let's check what logs we can see. + $course = get_course($this->filterparams->courseid); + $groupmode = groups_get_course_groupmode($course); + $context = context_course::instance($this->filterparams->courseid); + $userid = 0; + if ($groupmode == SEPARATEGROUPS && !has_capability('moodle/site:accessallgroups', $context)) { + $userid = $USER->id; + } + $cgroups = groups_get_all_groups($this->filterparams->courseid, $userid); + $cgroups = array_keys($cgroups); + if ($groupmode != SEPARATEGROUPS || has_capability('moodle/site:accessallgroups', $context)) { + $cgroups[] = USERSWITHOUTGROUP; + } + // If that's the case, limit the users to be in the groups only, defined by the filter. + [$groupmembersql, $groupmemberparams] = groups_get_members_ids_sql($cgroups, $context); + $joins[] = "userid IN ($groupmembersql)"; + $params = array_merge($params, $groupmemberparams); } - } else if (!empty($this->filterparams->userid)) { + } else { $joins[] = "userid = :userid"; $params['userid'] = $this->filterparams->userid; } diff --git a/report/log/tests/renderable_test.php b/report/log/tests/renderable_test.php index 97e09a58ba3..5dd39cee406 100644 --- a/report/log/tests/renderable_test.php +++ b/report/log/tests/renderable_test.php @@ -16,6 +16,9 @@ namespace report_log; +use context_course; +use core_user; + /** * Class report_log\renderable_test to cover functions in \report_log_renderable. * @@ -24,135 +27,414 @@ namespace report_log; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. */ class renderable_test extends \advanced_testcase { + /** + * @var int The course with separate groups. + */ + const COURSE_SEPARATE_GROUP = 0; + /** + * @var int The course with separate groups. + */ + const COURSE_VISIBLE_GROUP = 1; + /** + * @var int The course with separate groups. + */ + const COURSE_NO_GROUP = 2; + /** + * @var array The setup of users. + */ + const SETUP_USER_DEFS = [ + // Make student2 also member of group1. + 'student' => [ + 'student0' => ['group0'], + 'student1' => ['group1'], + 'student2' => ['group0', 'group1'], + 'student3' => [], + ], + // Make teacher2 also member of group1. + 'teacher' => [ + 'teacher0' => ['group0'], + 'teacher1' => ['group1'], + 'teacher2' => ['group0', 'group1'], + ], + // Make editingteacher also member of group1. + 'editingteacher' => [ + 'editingteacher0' => ['group0'], + 'editingteacher1' => ['group1'], + 'editingteacher2' => ['group0', 'group1'], + ], + ]; + /** + * @var array|\stdClass all users indexed by username. + */ + private $users = []; + /** + * @var array The groups by courses (array of array). + */ + private $groupsbycourse = []; + /** + * @var array The courses. + */ + private $courses; /** - * @var [stdClass] The students. + * Get the data provider for test_get_user_list(). + * + * @return array */ - private $student = []; + public static function get_user_visibility_list_provider(): array { + return [ + 'separategroups: student 0' => [ + self::COURSE_SEPARATE_GROUP, + 'student0', + // All users in group 0. + [ + 'student0', 'student2', + 'teacher0', 'teacher2', + 'editingteacher0', 'editingteacher2', + ], + ], + 'separategroups: student 1' => [ + self::COURSE_SEPARATE_GROUP, + 'student1', + // All users in group 1. + [ + 'student1', 'student2', 'teacher1', 'teacher2', 'editingteacher1', + 'editingteacher2', + ], + ], + 'separategroups: editing teacher 0' => [ + self::COURSE_SEPARATE_GROUP, + 'editingteacher0', + // All users (including student3 who is not in a group). + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'separategroups: teacher 0' => [ + self::COURSE_SEPARATE_GROUP, + 'teacher0', + // All users in group 0. + [ + 'student0', 'student2', + 'teacher0', 'teacher2', + 'editingteacher0', 'editingteacher2', + ], + ], + 'separategroups: teacher 2' => [ + self::COURSE_SEPARATE_GROUP, + 'teacher2', + // All users in group 0 and 1. + [ + 'student0', 'student1', 'student2', + 'teacher0', 'teacher1', 'teacher2', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'separategroups: teacher 2 with group0 selected' => [ + self::COURSE_SEPARATE_GROUP, + 'teacher2', + // All users in group 0. + [ + 'student0', 'student2', + 'teacher0', 'teacher2', + 'editingteacher0', 'editingteacher2', + ], + 'group0', + ], + 'separategroups: teacher 2 with group1 selected' => [ + self::COURSE_SEPARATE_GROUP, + 'teacher2', + // All users in group 1. + [ + 'student1', 'student2', + 'teacher1', 'teacher2', + 'editingteacher1', 'editingteacher2', + ], + 'group1', + ], + 'visiblegroup: teacher 0 with group1 selected' => [ + self::COURSE_VISIBLE_GROUP, + 'teacher2', + // All users in group 1. + [ + 'student1', 'student2', + 'teacher1', 'teacher2', + 'editingteacher1', 'editingteacher2', + ], + 'group1', + ], + 'visiblegroup: teacher 0 without group selected' => [ + self::COURSE_VISIBLE_GROUP, + 'teacher2', + // All users. + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'visiblegroup: editing teacher' => [ + self::COURSE_VISIBLE_GROUP, + 'editingteacher0', + // All users. + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'visiblegroup: student' => [ + self::COURSE_VISIBLE_GROUP, + 'student0', + // All users. + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'nogroup: teacher 0' => [ + self::COURSE_VISIBLE_GROUP, + 'teacher2', + // All users. + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'nogroup: editing teacher 0' => [ + self::COURSE_VISIBLE_GROUP, + 'editingteacher0', + // All users. + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'nogroup: student' => [ + self::COURSE_VISIBLE_GROUP, + 'student0', + // All users. + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + ]; + } /** - * @var [stdClass] The teachers. + * Data provider for test_get_group_list(). + * + * @return array */ - private $teacher = []; - - /** - * @var [stdClass] The groups. - */ - private $group = []; - - /** - * @var stdClass The course. - */ - private $course; + public static function get_group_list_provider(): array { + return [ + // The student sees his own group only. + 'separategroup: student in one group' => [self::COURSE_SEPARATE_GROUP, 'student0', 1], + 'separategroup: student in two groups' => [self::COURSE_SEPARATE_GROUP, 'student2', 2], + // While the teacher is not allowed to see all groups. + 'separategroup: teacher in one group' => [self::COURSE_SEPARATE_GROUP, 'teacher0', 1], + 'separategroup: teacher in two groups' => [self::COURSE_SEPARATE_GROUP, 'teacher2', 2], + // But editing teacher should see all. + 'separategroup: editingteacher' => [self::COURSE_SEPARATE_GROUP, 'editingteacher0', 2], + // The student sees all groups. + 'visiblegroup: student in one group' => [self::COURSE_VISIBLE_GROUP, 'student0', 2], + // Same for teacher. + 'visiblegroup: teacher in one group' => [self::COURSE_VISIBLE_GROUP, 'teacher0', 2], + // And editing teacher. + 'visiblegroup: editingteacher' => [self::COURSE_VISIBLE_GROUP, 'editingteacher0', 2], + // No group. + 'nogroups: student in one group' => [self::COURSE_NO_GROUP, 'student0', 0], + // Same for teacher. + 'nogroups: teacher in one group' => [self::COURSE_NO_GROUP, 'teacher0', 0], + // And editing teacher. + 'nogroups: editingteacher' => [self::COURSE_NO_GROUP, 'editingteacher0', 0], + ]; + } /** * 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(); + $this->courses[self::COURSE_SEPARATE_GROUP] = $this->getDataGenerator()->create_course(['groupmode' => SEPARATEGROUPS]); + $this->courses[self::COURSE_VISIBLE_GROUP] = $this->getDataGenerator()->create_course(['groupmode' => VISIBLEGROUPS]); + $this->courses[self::COURSE_NO_GROUP] = $this->getDataGenerator()->create_course(); + + foreach ($this->courses as $coursetype => $course) { + if ($coursetype == self::COURSE_NO_GROUP) { + continue; + } + $this->groupsbycourse[$coursetype] = []; + $this->groupsbycourse[$coursetype]['group0'] = + $this->getDataGenerator()->create_group(['courseid' => $course->id, 'name' => 'group0']); + $this->groupsbycourse[$coursetype]['group1'] = + $this->getDataGenerator()->create_group(['courseid' => $course->id, 'name' => 'group1']); + } + + foreach (self::SETUP_USER_DEFS as $role => $userdefs) { + foreach ($userdefs as $username => $groups) { + $user = $this->getDataGenerator()->create_user( + [ + 'username' => $username, + 'firstname' => "FN{$role}{$username}", + 'lastname' => "LN{$role}{$username}", + ]); + foreach ($this->courses as $coursetype => $course) { + $this->getDataGenerator()->enrol_user($user->id, $course->id, $role); + foreach ($groups as $groupname) { + if ($coursetype == self::COURSE_NO_GROUP) { + continue; + } + $this->getDataGenerator()->create_group_member([ + 'groupid' => $this->groupsbycourse[$coursetype][$groupname]->id, + 'userid' => $user->id, + ]); + } + } + $this->users[$username] = $user; + } + } } /** * Test report_log_renderable::get_user_list(). - * @covers \report_log_renderable::get_user_list + * + * @param int $courseindex + * @param string $username + * @param array $expectedusers + * @param string|null $groupname + * @covers \report_log_renderable::get_user_list + * @dataProvider get_user_visibility_list_provider * @return void */ - public function test_get_user_list() { + public function test_get_user_list(int $courseindex, string $username, array $expectedusers, + string $groupname = null): void { + global $PAGE, $CFG; + $currentcourse = $this->courses[$courseindex]; + $PAGE->set_url('/report/log/index.php?id=' . $currentcourse->id); // 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); + $currentuser = $this->users[$username]; + $this->setUser($currentuser->id); + $groupid = 0; + if ($groupname) { + $groupid = $this->groupsbycourse[$courseindex][$groupname]->id; + } $renderable = new \report_log_renderable( - "", (int)$this->course->id, $userid, 0, '', $groupid); - $users = $renderable->get_user_list(); - return \array_keys($users); + "", (int) $currentcourse->id, $currentuser->id, 0, '', $groupid); + $userlist = $renderable->get_user_list(); + unset($userlist[$CFG->siteguest]); // We ignore guest. + $usersid = array_keys($userlist); + + $users = array_map(function($userid) { + return core_user::get_user($userid); + }, $usersid); + + // Now check that the users are the expected ones. + asort($expectedusers); + $userlistbyname = array_column($users, 'username'); + asort($userlistbyname); + $this->assertEquals(array_values($expectedusers), array_values($userlistbyname)); + + // Check that users are in order lastname > firstname > id. + $sortedusers = $users; + // Sort user by lastname > firstname > id. + usort($sortedusers, function($a, $b) { + if ($a->lastname != $b->lastname) { + return $a->lastname <=> $b->lastname; + } + if ($a->firstname != $b->firstname) { + return $a->firstname <=> $b->firstname; + } + return $a->id <=> $b->id; + }); + + $sortedusernames = array_column($sortedusers, 'username'); + $userlistbyname = array_column($users, 'username'); + $this->assertEquals($sortedusernames, $userlistbyname); + } /** * Test report_log_renderable::get_group_list(). - * @covers \report_log_renderable::get_group_list + * + * @covers \report_log_renderable::get_group_list + * @dataProvider get_group_list_provider * @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); + public function test_get_group_list($courseindex, $username, $expectedcount): void { + global $PAGE; + $PAGE->set_url('/report/log/index.php?id=' . $this->courses[$courseindex]->id); + $this->setUser($this->users[$username]->id); + $renderable = new \report_log_renderable("", (int) $this->courses[$courseindex]->id, $this->users[$username]->id); $groups = $renderable->get_group_list(); - $this->assertCount(1, $groups); + $this->assertCount($expectedcount, $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); + /** + * Test table_log + * + * @param int $courseindex + * @param string $username + * @param array $expectedusers + * @param string|null $groupname + * @covers \report_log_renderable::get_user_list + * @dataProvider get_user_visibility_list_provider + * @return void + */ + public function test_get_table_logs(int $courseindex, string $username, array $expectedusers, ?string $groupname = null): void { + global $DB, $PAGE; + $this->preventResetByRollback(); // This is to ensure that we can actually trigger event and record them in the log store. + // Configure log store. + set_config('enabled_stores', 'logstore_standard', 'tool_log'); + $manager = get_log_manager(true); + $DB->delete_records('logstore_standard_log'); + foreach ($this->courses as $course) { + foreach ($this->users as $user) { + $eventdata = [ + 'context' => context_course::instance($course->id), + 'userid' => $user->id, + ]; + $event = \core\event\course_viewed::create($eventdata); + $event->trigger(); + } + } + $stores = $manager->get_readers(); + $store = $stores['logstore_standard']; + // Build the report. + $currentuser = $this->users[$username]; + $this->setUser($currentuser->id); + $groupid = 0; + if ($groupname) { + $groupid = $this->groupsbycourse[$courseindex][$groupname]->id; + } + $PAGE->set_url('/report/log/index.php?id=' . $this->courses[$courseindex]->id); + $renderable = new \report_log_renderable("", (int) $this->courses[$courseindex]->id, 0, 0, '', $groupid); + $renderable->setup_table(); + $table = $renderable->tablelog; + $store->flush(); + $table->query_db(100); + $usernames = []; + foreach ($table->rawdata as $event) { + if (get_class($event) !== \core\event\course_viewed::class) { + continue; + } + $user = core_user::get_user($event->userid, '*', MUST_EXIST); + $usernames[] = $user->username; + } + sort($expectedusers); + sort($usernames); + $this->assertEquals($expectedusers, $usernames); } } 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..296849f5d07 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,31 @@ 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; - + global $USER; $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 we are in a course, then let's check what logs we can see. + $course = get_course($this->filterparams->courseid); + $groupmode = groups_get_course_groupmode($course); + $context = context_course::instance($this->filterparams->courseid); + $userid = 0; + if ($groupmode == SEPARATEGROUPS && !has_capability('moodle/site:accessallgroups', $context)) { + $userid = $USER->id; } + $cgroups = groups_get_all_groups($this->filterparams->courseid, $userid); + $cgroups = array_keys($cgroups); + if ($groupmode != SEPARATEGROUPS || has_capability('moodle/site:accessallgroups', $context) || empty($cgroups)) { + $cgroups[] = USERSWITHOUTGROUP; + } + // If that's the case, limit the users to be in the groups only, defined by the filter. + [$groupmembersql, $groupmemberparams] = groups_get_members_ids_sql($cgroups, $context); + $joins[] = "userid IN ($groupmembersql)"; + $params = array_merge($params, $groupmemberparams); } if (!empty($this->filterparams->date)) { @@ -333,6 +334,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); diff --git a/report/loglive/tests/table_log_test.php b/report/loglive/tests/table_log_test.php new file mode 100644 index 00000000000..f411eb36a48 --- /dev/null +++ b/report/loglive/tests/table_log_test.php @@ -0,0 +1,317 @@ +. +namespace report_loglive; + +use advanced_testcase; +use context_course; +use core_user; + +/** + * Tests for table log and groups. + * + * @package report_loglive + * @copyright 2024 onwards Laurent David + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ +class table_log_test extends advanced_testcase { + /** + * @var int The course with separate groups. + */ + const COURSE_SEPARATE_GROUP = 0; + /** + * @var int The course with separate groups. + */ + const COURSE_VISIBLE_GROUP = 1; + /** + * @var int The course with separate groups. + */ + const COURSE_NO_GROUP = 2; + /** + * @var array The setup of users. + */ + const SETUP_USER_DEFS = [ + // Make student2 also member of group1. + 'student' => [ + 'student0' => ['group0'], + 'student1' => ['group1'], + 'student2' => ['group0', 'group1'], + 'student3' => [], + ], + // Make teacher2 also member of group1. + 'teacher' => [ + 'teacher0' => ['group0'], + 'teacher1' => ['group1'], + 'teacher2' => ['group0', 'group1'], + 'teacher3' => [], + ], + // Make editingteacher also member of group1. + 'editingteacher' => [ + 'editingteacher0' => ['group0'], + 'editingteacher1' => ['group1'], + 'editingteacher2' => ['group0', 'group1'], + ], + ]; + /** + * @var array|\stdClass all users indexed by username. + */ + private $users = []; + /** + * @var array The groups by courses (array of array). + */ + private $groupsbycourse = []; + /** + * @var array The courses. + */ + private $courses; + + /** + * Data provider for test_get_table_logs. + * + * @return array + */ + public static function get_report_logs_provider(): array { + return [ + 'separategroups: student 0' => [ + self::COURSE_SEPARATE_GROUP, + 'student0', + // All users in group 0. + [ + 'student0', 'student2', + 'teacher0', 'teacher2', + 'editingteacher0', 'editingteacher2', + ], + ], + 'separategroups: student 1' => [ + self::COURSE_SEPARATE_GROUP, + 'student1', + // All users in group1. + [ + 'student1', 'student2', + 'teacher1', 'teacher2', + 'editingteacher1', 'editingteacher2', + ], + ], + 'separategroups: student 2' => [ + self::COURSE_SEPARATE_GROUP, + 'student2', + // All users in group0 and group1. + [ + 'student0', 'student1', 'student2', + 'teacher0', 'teacher1', 'teacher2', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'separategroups: student3' => [ + self::COURSE_SEPARATE_GROUP, + 'student3', + // Student 3 is not in any group so should only see user without a group. + [ + 'student3', + 'teacher3', + ], + ], + 'separategroups: editing teacher 0' => [ + self::COURSE_SEPARATE_GROUP, + 'editingteacher0', + // All users including student 3 as we can see all users (event the one not in a group). + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', 'teacher3', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'separategroups: teacher 0' => [ + self::COURSE_SEPARATE_GROUP, + 'teacher0', + // All users in group 0. + [ + 'student0', 'student2', + 'teacher0', 'teacher2', + 'editingteacher0', 'editingteacher2', + ], + ], + 'separategroups: teacher 2' => [ + self::COURSE_SEPARATE_GROUP, + 'teacher2', + // All users in group0 and group1. + [ + 'student0', 'student1', 'student2', + 'teacher0', 'teacher1', 'teacher2', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'separategroups: teacher 3' => [ + self::COURSE_SEPARATE_GROUP, + 'teacher3', + // Teacher 3 is not in any group so should only see user without a group. + [ + 'student3', + 'teacher3', + ], + ], + 'visiblegroup: editing teacher' => [ + self::COURSE_VISIBLE_GROUP, + 'editingteacher0', + // All users. + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', 'teacher3', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'visiblegroup: student' => [ + self::COURSE_VISIBLE_GROUP, + 'student0', + // All users. + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', 'teacher3', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'nogroup: teacher 0' => [ + self::COURSE_VISIBLE_GROUP, + 'teacher2', + // All users. + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', 'teacher3', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'nogroup: editing teacher 0' => [ + self::COURSE_VISIBLE_GROUP, + 'editingteacher0', + // All users. + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', 'teacher3', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + 'nogroup: student' => [ + self::COURSE_VISIBLE_GROUP, + 'student0', + // All users. + [ + 'student0', 'student1', 'student2', 'student3', + 'teacher0', 'teacher1', 'teacher2', 'teacher3', + 'editingteacher0', 'editingteacher1', 'editingteacher2', + ], + ], + ]; + } + + /** + * 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 $DB; + $this->resetAfterTest(); + $this->preventResetByRollback(); // This is to ensure that we can actually trigger event and record them in the log store. + $this->courses[self::COURSE_SEPARATE_GROUP] = $this->getDataGenerator()->create_course(['groupmode' => SEPARATEGROUPS]); + $this->courses[self::COURSE_VISIBLE_GROUP] = $this->getDataGenerator()->create_course(['groupmode' => VISIBLEGROUPS]); + $this->courses[self::COURSE_NO_GROUP] = $this->getDataGenerator()->create_course(); + + foreach ($this->courses as $coursetype => $course) { + if ($coursetype == self::COURSE_NO_GROUP) { + continue; + } + $this->groupsbycourse[$coursetype] = []; + $this->groupsbycourse[$coursetype]['group0'] = + $this->getDataGenerator()->create_group(['courseid' => $course->id, 'name' => 'group0']); + $this->groupsbycourse[$coursetype]['group1'] = + $this->getDataGenerator()->create_group(['courseid' => $course->id, 'name' => 'group1']); + } + + foreach (self::SETUP_USER_DEFS as $role => $userdefs) { + foreach ($userdefs as $username => $groups) { + $user = $this->getDataGenerator()->create_user( + [ + 'username' => $username, + 'firstname' => "FN{$role}{$username}", + 'lastname' => "LN{$role}{$username}", + ]); + foreach ($this->courses as $coursetype => $course) { + $this->getDataGenerator()->enrol_user($user->id, $course->id, $role); + foreach ($groups as $groupname) { + if ($coursetype == self::COURSE_NO_GROUP) { + continue; + } + $this->getDataGenerator()->create_group_member([ + 'groupid' => $this->groupsbycourse[$coursetype][$groupname]->id, + 'userid' => $user->id, + ]); + } + } + $this->users[$username] = $user; + } + } + // Configure log store. + set_config('enabled_stores', 'logstore_standard', 'tool_log'); + get_log_manager(true); + $DB->delete_records('logstore_standard_log'); + + foreach ($this->courses as $course) { + foreach ($this->users as $user) { + $eventdata = [ + 'context' => context_course::instance($course->id), + 'userid' => $user->id, + ]; + $event = \core\event\course_viewed::create($eventdata); + $event->trigger(); + } + } + } + + /** + * Test table_log + * + * @param int $courseindex + * @param string $username + * @param array $expectedusers + * @covers \report_log_renderable::get_user_list + * @dataProvider get_report_logs_provider + * @return void + */ + public function test_get_table_logs(int $courseindex, string $username, array $expectedusers): void { + $manager = get_log_manager(); + $stores = $manager->get_readers(); + $store = $stores['logstore_standard']; + // Build the report. + $url = new \moodle_url("/report/loglive/index.php"); + $renderable = new \report_loglive_renderable('logstore_standard', $this->courses[$courseindex], $url); + $table = $renderable->get_table(); + $currentuser = $this->users[$username]; + $this->setUser($currentuser->id); + $store->flush(); + $table->query_db(100); + $filteredevents = array_filter($table->rawdata, fn($event) => get_class($event) === \core\event\course_viewed::class); + $usernames = array_map( + function($event) { + $user = core_user::get_user($event->userid, '*', MUST_EXIST); + return $user->username; + }, + $filteredevents); + sort($expectedusers); + sort($usernames); + $this->assertEquals($expectedusers, $usernames); + } +}