From c71fe8690cb5b6223b5655b4a33f20b8b98bef24 Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Tue, 31 Jul 2018 15:31:58 +1000 Subject: [PATCH 1/8] MDL-58768 Calendar: Added userid param to calendar_get_default_courses --- calendar/lib.php | 23 ++++++++++++++--------- calendar/tests/lib_test.php | 20 ++++++++++++++++++++ calendar/upgrade.txt | 3 +++ 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/calendar/lib.php b/calendar/lib.php index fa5d22671e8..a08f735a1db 100644 --- a/calendar/lib.php +++ b/calendar/lib.php @@ -2303,20 +2303,25 @@ function calendar_delete_event_allowed($event) { * * @param int $courseid (optional) If passed, an additional course can be returned for admins (the current course). * @param string $fields Comma separated list of course fields to return. - * @param bool $canmanage If true, this will return the list of courses the current user can create events in, rather + * @param bool $canmanage If true, this will return the list of courses the user can create events in, rather * than the list of courses they see events from (an admin can always add events in a course * calendar, even if they are not enrolled in the course). + * @param int $userid (optional) The user which this function returns the default courses for. + * By default the current user. * @return array $courses Array of courses to display */ -function calendar_get_default_courses($courseid = null, $fields = '*', $canmanage=false) { - global $CFG, $DB; +function calendar_get_default_courses($courseid = null, $fields = '*', $canmanage = false, int $userid = null) { + global $CFG, $USER; - if (!isloggedin()) { - return array(); + if (!$userid) { + if (!isloggedin()) { + return array(); + } + $userid = $USER->id; } - if (has_capability('moodle/calendar:manageentries', context_system::instance()) && - (!empty($CFG->calendar_adminseesall) || $canmanage)) { + if ((!empty($CFG->calendar_adminseesall) || $canmanage) && + has_capability('moodle/calendar:manageentries', context_system::instance(), $userid)) { // Add a c. prefix to every field as expected by get_courses function. $fieldlist = explode(',', $fields); @@ -2326,11 +2331,11 @@ function calendar_get_default_courses($courseid = null, $fields = '*', $canmanag }, $fieldlist); $courses = get_courses('all', 'c.shortname', implode(',', $prefixedfields)); } else { - $courses = enrol_get_my_courses($fields); + $courses = enrol_get_users_courses($userid, true, $fields); } if ($courseid && $courseid != SITEID) { - if (empty($courses[$courseid]) && has_capability('moodle/calendar:manageentries', context_system::instance())) { + if (empty($courses[$courseid]) && has_capability('moodle/calendar:manageentries', context_system::instance(), $userid)) { // Allow a site admin to see calendars from courses he is not enrolled in. // This will come from $COURSE. $courses[$courseid] = get_course($courseid); diff --git a/calendar/tests/lib_test.php b/calendar/tests/lib_test.php index efb67818dcd..90230a903c9 100644 --- a/calendar/tests/lib_test.php +++ b/calendar/tests/lib_test.php @@ -727,6 +727,26 @@ class core_calendar_lib_testcase extends advanced_testcase { // Enrolled course only (ignore current). $this->assertCount(1, $courses); + // Now, log out and test again. + $this->setUser(); + + $CFG->calendar_adminseesall = false; + + $courses = calendar_get_default_courses(null, '*', false, $teacher->id); + // Only enrolled in one course. + $this->assertCount(1, $courses); + $courses = calendar_get_default_courses($course2->id, '*', false, $teacher->id); + // Enrolled course only (ignore current). + $this->assertCount(1, $courses); + // This setting should not affect teachers. + $CFG->calendar_adminseesall = true; + $courses = calendar_get_default_courses(null, '*', false, $teacher->id); + // Only enrolled in one course. + $this->assertCount(1, $courses); + $courses = calendar_get_default_courses($course2->id, '*', false, $teacher->id); + // Enrolled course only (ignore current). + $this->assertCount(1, $courses); + } public function test_calendar_get_allowed_event_types_no_types() { diff --git a/calendar/upgrade.txt b/calendar/upgrade.txt index b2973e53f0a..3d594f9837b 100644 --- a/calendar/upgrade.txt +++ b/calendar/upgrade.txt @@ -1,6 +1,9 @@ This files describes API changes in /calendar/* , information provided here is intended especially for developers. +=== 3.4.5 === +* calendar_get_default_courses() function now has optional $userid parameter. + === 3.4 === * calendar_get_mini, and calendar_get_upcoming have been deprecated. Please update to use the new exporters and renderers. * added core_calendar_get_valid_event_timestart_range and core_calendar_event_timestart_updated callbacks for module events From a20071b2fe066ab75b554e44049c66e5e019d1ec Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Tue, 31 Jul 2018 15:44:58 +1000 Subject: [PATCH 2/8] MDL-58768 Calendar: Added user param to calendar_set_filters Decoupled calendar_set_filters from $USER. Also removed the backward compatibility bit from the function because it was a left over. --- calendar/lib.php | 39 +++++------ calendar/tests/lib_test.php | 125 ++++++++++++++++++++++++++++++++++++ calendar/upgrade.txt | 1 + 3 files changed, 143 insertions(+), 22 deletions(-) diff --git a/calendar/lib.php b/calendar/lib.php index a08f735a1db..0289548737a 100644 --- a/calendar/lib.php +++ b/calendar/lib.php @@ -2025,34 +2025,29 @@ function calendar_events_by_day($events, $month, $year, &$eventsbyday, &$duratio * * @param array $courseeventsfrom An array of courses to load calendar events for * @param bool $ignorefilters specify the use of filters, false is set as default + * @param stdClass $user The user object. This defaults to the global $USER object. * @return array An array of courses, groups, and user to load calendar events for based upon filters */ -function calendar_set_filters(array $courseeventsfrom, $ignorefilters = false) { - global $USER, $CFG; +function calendar_set_filters(array $courseeventsfrom, $ignorefilters = false, stdClass $user = null) { + global $CFG, $USER; - // For backwards compatability we have to check whether the courses array contains - // just id's in which case we need to load course objects. - $coursestoload = array(); - foreach ($courseeventsfrom as $id => $something) { - if (!is_object($something)) { - $coursestoload[] = $id; - unset($courseeventsfrom[$id]); - } + if (is_null($user)) { + $user = $USER; } $courses = array(); - $user = false; + $userid = false; $group = false; // Get the capabilities that allow seeing group events from all groups. $allgroupscaps = array('moodle/site:accessallgroups', 'moodle/calendar:manageentries'); - $isloggedin = isloggedin(); + $isvaliduser = !empty($user->id); - if ($ignorefilters || calendar_show_event_type(CALENDAR_EVENT_COURSE)) { + if ($ignorefilters || calendar_show_event_type(CALENDAR_EVENT_COURSE, $user)) { $courses = array_keys($courseeventsfrom); } - if ($ignorefilters || calendar_show_event_type(CALENDAR_EVENT_GLOBAL)) { + if ($ignorefilters || calendar_show_event_type(CALENDAR_EVENT_GLOBAL, $user)) { $courses[] = SITEID; } $courses = array_unique($courses); @@ -2066,11 +2061,11 @@ function calendar_set_filters(array $courseeventsfrom, $ignorefilters = false) { $courses[] = SITEID; } - if ($ignorefilters || ($isloggedin && calendar_show_event_type(CALENDAR_EVENT_USER))) { - $user = $USER->id; + if ($ignorefilters || ($isvaliduser && calendar_show_event_type(CALENDAR_EVENT_USER, $user))) { + $userid = $user->id; } - if (!empty($courseeventsfrom) && (calendar_show_event_type(CALENDAR_EVENT_GROUP) || $ignorefilters)) { + if (!empty($courseeventsfrom) && (calendar_show_event_type(CALENDAR_EVENT_GROUP, $user) || $ignorefilters)) { if (count($courseeventsfrom) == 1) { $course = reset($courseeventsfrom); @@ -2082,16 +2077,16 @@ function calendar_set_filters(array $courseeventsfrom, $ignorefilters = false) { if ($group === false) { if (!empty($CFG->calendar_adminseesall) && has_any_capability($allgroupscaps, \context_system::instance())) { $group = true; - } else if ($isloggedin) { + } else if ($isvaliduser) { $groupids = array(); foreach ($courseeventsfrom as $courseid => $course) { // If the user is an editing teacher in there. - if (!empty($USER->groupmember[$course->id])) { + if (!empty($user->groupmember[$course->id])) { // We've already cached the users groups for this course so we can just use that. - $groupids = array_merge($groupids, $USER->groupmember[$course->id]); + $groupids = array_merge($groupids, $user->groupmember[$course->id]); } else if ($course->groupmode != NOGROUPS || !$course->groupmodeforce) { // If this course has groups, show events from all of those related to the current user. - $coursegroups = groups_get_user_groups($course->id, $USER->id); + $coursegroups = groups_get_user_groups($course->id, $user->id); $groupids = array_merge($groupids, $coursegroups['0']); } } @@ -2105,7 +2100,7 @@ function calendar_set_filters(array $courseeventsfrom, $ignorefilters = false) { $courses = false; } - return array($courses, $group, $user); + return array($courses, $group, $userid); } /** diff --git a/calendar/tests/lib_test.php b/calendar/tests/lib_test.php index 90230a903c9..1d20e9c7e19 100644 --- a/calendar/tests/lib_test.php +++ b/calendar/tests/lib_test.php @@ -906,4 +906,129 @@ class core_calendar_lib_testcase extends advanced_testcase { $types = calendar_get_allowed_event_types($course->id); $this->assertTrue($types['group']); } + + /** + * This is a setup helper function that create some users, courses, groups and group memberships. + * This is useful to prepare the environment for testing the calendar_set_filters function. + * + * @return array An array of ($users, $courses, $coursegroups) + */ + protected function setup_test_calendar_set_filters() { + $generator = $this->getDataGenerator(); + + // Create some users. + $users = []; + $users[] = $generator->create_user(); + $users[] = $generator->create_user(); + $users[] = $generator->create_user(); + + // Create some courses. + $courses = []; + $courses[] = $generator->create_course(); + $courses[] = $generator->create_course(); + $courses[] = $generator->create_course(); + $courses[] = $generator->create_course(); + + // Create some groups. + $coursegroups = []; + $coursegroups[$courses[0]->id] = []; + $coursegroups[$courses[0]->id][] = $generator->create_group(['courseid' => $courses[0]->id]); + $coursegroups[$courses[0]->id][] = $generator->create_group(['courseid' => $courses[0]->id]); + $coursegroups[$courses[2]->id] = []; + $coursegroups[$courses[2]->id][] = $generator->create_group(['courseid' => $courses[2]->id]); + $coursegroups[$courses[2]->id][] = $generator->create_group(['courseid' => $courses[2]->id]); + $coursegroups[$courses[3]->id] = []; + $coursegroups[$courses[3]->id][] = $generator->create_group(['courseid' => $courses[3]->id]); + $coursegroups[$courses[3]->id][] = $generator->create_group(['courseid' => $courses[3]->id]); + + // Create some enrolments and group memberships. + $generator->enrol_user($users[0]->id, $courses[0]->id, 'student'); + $generator->create_group_member(['groupid' => $coursegroups[$courses[0]->id][0]->id, 'userid' => $users[0]->id]); + $generator->enrol_user($users[1]->id, $courses[0]->id, 'student'); + $generator->create_group_member(['groupid' => $coursegroups[$courses[0]->id][1]->id, 'userid' => $users[1]->id]); + $generator->enrol_user($users[0]->id, $courses[1]->id, 'student'); + $generator->enrol_user($users[0]->id, $courses[2]->id, 'student'); + + return array($users, $courses, $coursegroups); + } + + /** + * This function tests calendar_set_filters for the case when user is not logged in. + */ + public function test_calendar_set_filters_not_logged_in() { + $this->resetAfterTest(); + + list($users, $courses, $coursegroups) = $this->setup_test_calendar_set_filters(); + + $defaultcourses = calendar_get_default_courses(null, '*', false, $users[0]->id); + list($courseids, $groupids, $userid) = calendar_set_filters($defaultcourses); + + $this->assertEquals( + [$courses[0]->id, $courses[1]->id, $courses[2]->id, SITEID], + array_values($courseids), + '', 0.0, 10, true); + $this->assertFalse($groupids); + $this->assertFalse($userid); + } + + /** + * This function tests calendar_set_filters for the case when no one is logged in, but a user id is provided. + */ + public function test_calendar_set_filters_not_logged_in_with_user() { + $this->resetAfterTest(); + + list($users, $courses, $coursegroups) = $this->setup_test_calendar_set_filters(); + + $defaultcourses = calendar_get_default_courses(null, '*', false, $users[1]->id); + list($courseids, $groupids, $userid) = calendar_set_filters($defaultcourses, false, $users[1]); + + $this->assertEquals(array($courses[0]->id, SITEID), array_values($courseids)); + $this->assertEquals(array($coursegroups[$courses[0]->id][1]->id), $groupids); + $this->assertEquals($users[1]->id, $userid); + + $defaultcourses = calendar_get_default_courses(null, '*', false, $users[0]->id); + list($courseids, $groupids, $userid) = calendar_set_filters($defaultcourses, false, $users[0]); + + $this->assertEquals( + [$courses[0]->id, $courses[1]->id, $courses[2]->id, SITEID], + array_values($courseids), + '', 0.0, 10, true); + $this->assertEquals(array($coursegroups[$courses[0]->id][0]->id), $groupids); + $this->assertEquals($users[0]->id, $userid); + + } + + /** + * This function tests calendar_set_filters for the case when user is logged in, but no user id is provided. + */ + public function test_calendar_set_filters_logged_in_no_user() { + $this->resetAfterTest(); + + list($users, $courses, $coursegroups) = $this->setup_test_calendar_set_filters(); + + $this->setUser($users[0]); + $defaultcourses = calendar_get_default_courses(null, '*', false, $users[0]->id); + list($courseids, $groupids, $userid) = calendar_set_filters($defaultcourses, false); + $this->assertEquals([$courses[0]->id, $courses[1]->id, $courses[2]->id, SITEID], array_values($courseids), '', 0.0, 10, + true); + $this->assertEquals(array($coursegroups[$courses[0]->id][0]->id), $groupids); + $this->assertEquals($users[0]->id, $userid); + } + + /** + * This function tests calendar_set_filters for the case when a user is logged in, but another user id is provided. + */ + public function test_calendar_set_filters_logged_in_another_user() { + $this->resetAfterTest(); + + list($users, $courses, $coursegroups) = $this->setup_test_calendar_set_filters(); + + $this->setUser($users[0]); + $defaultcourses = calendar_get_default_courses(null, '*', false, $users[1]->id); + list($courseids, $groupids, $userid) = calendar_set_filters($defaultcourses, false, $users[1]); + + $this->assertEquals(array($courses[0]->id, SITEID), array_values($courseids)); + $this->assertEquals(array($coursegroups[$courses[0]->id][1]->id), $groupids); + $this->assertEquals($users[1]->id, $userid); + } } diff --git a/calendar/upgrade.txt b/calendar/upgrade.txt index 3d594f9837b..c5fde56b077 100644 --- a/calendar/upgrade.txt +++ b/calendar/upgrade.txt @@ -3,6 +3,7 @@ information provided here is intended especially for developers. === 3.4.5 === * calendar_get_default_courses() function now has optional $userid parameter. +* calendar_set_filters() function now has optional $user parameter. === 3.4 === * calendar_get_mini, and calendar_get_upcoming have been deprecated. Please update to use the new exporters and renderers. From 991850894e42de97c6fb646579ad70022baaaf49 Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Thu, 29 Mar 2018 01:58:06 +1100 Subject: [PATCH 3/8] MDL-58768 Calendar: Update get_raw_events_legacy_implementation Update get_raw_events_legacy_implementation to work with the passed $users parameter instead of the logged in user. Also, fixed a bug where get_raw_events_legacy_implementation was failing when $users was set to true, and a list of $groups was given. --- .../raw_event_retrieval_strategy.php | 114 +++++++++---- .../raw_event_retrieval_strategy_test.php | 158 ++++++++++++++---- 2 files changed, 204 insertions(+), 68 deletions(-) diff --git a/calendar/classes/local/event/strategies/raw_event_retrieval_strategy.php b/calendar/classes/local/event/strategies/raw_event_retrieval_strategy.php index 5b863b2ccae..37d1a8038e2 100644 --- a/calendar/classes/local/event/strategies/raw_event_retrieval_strategy.php +++ b/calendar/classes/local/event/strategies/raw_event_retrieval_strategy.php @@ -96,11 +96,24 @@ class raw_event_retrieval_strategy implements raw_event_retrieval_strategy_inter return array(); } + if (is_numeric($users)) { + $users = array($users); + } + if (is_numeric($groups)) { + $groups = array($groups); + } + if (is_numeric($courses)) { + $courses = array($courses); + } + if (is_numeric($categories)) { + $categories = array($categories); + } + // Array of filter conditions. To be concatenated by the OR operator. $filters = []; // User filter. - if ((is_array($users) && !empty($users)) or is_numeric($users)) { + if (is_array($users) && !empty($users)) { // Events from a number of users. list($insqlusers, $inparamsusers) = $DB->get_in_or_equal($users, SQL_PARAMS_NAMED); $filters[] = "(e.userid $insqlusers AND e.courseid = 0 AND e.groupid = 0 AND e.categoryid = 0)"; @@ -112,7 +125,7 @@ class raw_event_retrieval_strategy implements raw_event_retrieval_strategy_inter // Boolean false (no users at all): We don't need to do anything. // Group filter. - if ((is_array($groups) && !empty($groups)) or is_numeric($groups)) { + if (is_array($groups) && !empty($groups)) { // Events from a number of groups. list($insqlgroups, $inparamsgroups) = $DB->get_in_or_equal($groups, SQL_PARAMS_NAMED); $filters[] = "e.groupid $insqlgroups"; @@ -124,7 +137,7 @@ class raw_event_retrieval_strategy implements raw_event_retrieval_strategy_inter // Boolean false (no groups at all): We don't need to do anything. // Course filter. - if ((is_array($courses) && !empty($courses)) or is_numeric($courses)) { + if (is_array($courses) && !empty($courses)) { list($insqlcourses, $inparamscourses) = $DB->get_in_or_equal($courses, SQL_PARAMS_NAMED); $filters[] = "(e.groupid = 0 AND e.courseid $insqlcourses)"; $params = array_merge($params, $inparamscourses); @@ -134,7 +147,7 @@ class raw_event_retrieval_strategy implements raw_event_retrieval_strategy_inter } // Category filter. - if ((is_array($categories) && !empty($categories)) or is_numeric($categories)) { + if (is_array($categories) && !empty($categories)) { list($insqlcategories, $inparamscategories) = $DB->get_in_or_equal($categories, SQL_PARAMS_NAMED); $filters[] = "(e.groupid = 0 AND e.courseid = 0 AND e.categoryid $insqlcategories)"; $params = array_merge($params, $inparamscategories); @@ -168,54 +181,81 @@ class raw_event_retrieval_strategy implements raw_event_retrieval_strategy_inter // Build SQL subquery and conditions for filtered events based on priorities. $subquerywhere = ''; $subqueryconditions = []; - - // Get the user's courses. Otherwise, get the default courses being shown by the calendar. - $usercourses = calendar_get_default_courses(null, 'id, category, groupmode, groupmodeforce'); - - // Set calendar filters. - list($usercourses, $usergroups, $user) = calendar_set_filters($usercourses, true); $subqueryparams = []; + $allusercourses = []; - // Flag to indicate whether the query needs to exclude group overrides. - $viewgroupsonly = false; + if (is_array($users) && !empty($users)) { + $userrecords = $DB->get_records_sql("SELECT * FROM {user} WHERE id $insqlusers", $inparamsusers); + foreach ($userrecords as $userrecord) { + // Get the user's courses. Otherwise, get the default courses being shown by the calendar. + $usercourses = calendar_get_default_courses(null, 'id, category, groupmode, groupmodeforce', + false, $userrecord->id); - if ($user) { - // Set filter condition for the user's events. - $subqueryconditions[] = "(ev.userid = :user AND ev.courseid = 0 AND ev.groupid = 0 AND ev.categoryid = 0)"; - $subqueryparams['user'] = $user; + // Set calendar filters. + list($usercourses, $usergroups, $user) = calendar_set_filters($usercourses, true, $userrecord); - foreach ($usercourses as $courseid) { - if (has_capability('moodle/site:accessallgroups', \context_course::instance($courseid))) { - $usergroupmembership = groups_get_all_groups($courseid, $user, 0, 'g.id'); - if (count($usergroupmembership) == 0) { - $viewgroupsonly = true; - break; + $allusercourses = array_merge($allusercourses, $usercourses); + + // Flag to indicate whether the query needs to exclude group overrides. + $viewgroupsonly = false; + + if ($user) { + // Set filter condition for the user's events. + // Even though $user is a single scalar, we still use get_in_or_equal() because we are inside a loop. + list($inusers, $inuserparams) = $DB->get_in_or_equal($user, SQL_PARAMS_NAMED); + $subqueryconditions[] = "(ev.userid $inusers AND ev.courseid = 0 AND ev.groupid = 0 AND ev.categoryid = 0)"; + $subqueryparams = array_merge($subqueryparams, $inuserparams); + + foreach ($usercourses as $courseid) { + if (has_capability('moodle/site:accessallgroups', \context_course::instance($courseid), $userrecord)) { + $usergroupmembership = groups_get_all_groups($courseid, $user, 0, 'g.id'); + if (count($usergroupmembership) == 0) { + $viewgroupsonly = true; + break; + } + } } } - } - } - // Set filter condition for the user's group events. - if ($usergroups === true || $viewgroupsonly) { - // Fetch group events, but not group overrides. - $subqueryconditions[] = "(ev.groupid != 0 AND ev.eventtype = 'group')"; - } else if (!empty($usergroups)) { - // Fetch group events and group overrides. - list($inusergroups, $inusergroupparams) = $DB->get_in_or_equal($usergroups, SQL_PARAMS_NAMED); - $subqueryconditions[] = "(ev.groupid $inusergroups)"; - $subqueryparams = array_merge($subqueryparams, $inusergroupparams); + // Set filter condition for the user's group events. + if ($usergroups === true || $viewgroupsonly) { + // Fetch group events, but not group overrides. + $subqueryconditions[] = "(ev.groupid != 0 AND ev.eventtype = 'group')"; + } else if (!empty($usergroups)) { + // Fetch group events and group overrides. + list($inusergroups, $inusergroupparams) = $DB->get_in_or_equal($usergroups, SQL_PARAMS_NAMED); + $subqueryconditions[] = "(ev.groupid $inusergroups)"; + $subqueryparams = array_merge($subqueryparams, $inusergroupparams); + } + } + } else if ($users === true) { + // Events from ALL users. + $subqueryconditions[] = "(ev.userid != 0 AND ev.courseid = 0 AND ev.groupid = 0 AND ev.categoryid = 0)"; + + if (is_array($groups)) { + // Events from a number of groups. + list($insqlgroups, $inparamsgroups) = $DB->get_in_or_equal($groups, SQL_PARAMS_NAMED); + $subqueryconditions[] = "ev.groupid $insqlgroups"; + $subqueryparams = array_merge($subqueryparams, $inparamsgroups); + } else if ($groups === true) { + // Events from ALL groups. + $subqueryconditions[] = "ev.groupid != 0"; + } + + if ($courses === true) { + // ALL course events. It's not needed to worry about users' access as $users = true. + $subqueryconditions[] = "(ev.groupid = 0 AND ev.courseid != 0 AND ev.categoryid = 0)"; + } } // Get courses to be used for the subquery. $subquerycourses = []; if (is_array($courses)) { $subquerycourses = $courses; - } else if (is_numeric($courses)) { - $subquerycourses[] = $courses; } // Merge with user courses, if necessary. - if (!empty($usercourses)) { - $subquerycourses = array_merge($subquerycourses, $usercourses); + if (!empty($allusercourses)) { + $subquerycourses = array_merge($subquerycourses, $allusercourses); // Make sure we remove duplicate values. $subquerycourses = array_unique($subquerycourses); } diff --git a/calendar/tests/raw_event_retrieval_strategy_test.php b/calendar/tests/raw_event_retrieval_strategy_test.php index 9d8b1375b65..c8a58a1e45e 100644 --- a/calendar/tests/raw_event_retrieval_strategy_test.php +++ b/calendar/tests/raw_event_retrieval_strategy_test.php @@ -88,15 +88,29 @@ class core_calendar_raw_event_retrieval_strategy_testcase extends advanced_testc $this->assertCount(2, $events); // Disable the lesson module. - $modulerecord = $DB->get_record('modules', ['name' => 'lesson']); - $modulerecord->visible = 0; - $DB->update_record('modules', $modulerecord); + $DB->set_field('modules', 'visible', 0, ['name' => 'lesson']); // Check that we only return the assign event. $events = $retrievalstrategy->get_raw_events(null, [0], null); $this->assertCount(1, $events); $event = reset($events); $this->assertEquals('assign', $event->modulename); + + // Now, log out and repeat the above test in the reverse order. + $this->setUser(); + + // Check that we only return the assign event (given that the lesson module is still disabled). + $events = $retrievalstrategy->get_raw_events([$student->id], [0], null); + $this->assertCount(1, $events); + $event = reset($events); + $this->assertEquals('assign', $event->modulename); + + // Enable the lesson module. + $DB->set_field('modules', 'visible', 1, ['name' => 'lesson']); + + // Get all events. + $events = $retrievalstrategy->get_raw_events(null, [0], null); + $this->assertCount(2, $events); } /** @@ -203,37 +217,37 @@ class core_calendar_raw_event_retrieval_strategy_testcase extends advanced_testc calendar_event::create($event, false); } - $timestart = $now - 100; - $timeend = $now + (3 * 86400); $groups = [$group1->id, $group2->id]; - // Get user override events. - $this->setUser($useroverridestudent); - $events = $retrievalstrategy->get_raw_events([$useroverridestudent->id], $groups, [$course->id]); - $this->assertCount(1, $events); - $event = reset($events); - $this->assertEquals('Assignment 1 due date - User override', $event->name); + // Do the following tests multiple times when logged in with different users. Also run the whole set when logged out. + // In any cases, the tests should not depend on the logged-in user. + foreach ([$useroverridestudent, $nogroupstudent, $group12student, $group1student, null] as $login) { + $this->setUser($login); - // Get events for user that does not belong to any group and has no user override events. - $this->setUser($nogroupstudent); - $events = $retrievalstrategy->get_raw_events([$nogroupstudent->id], $groups, [$course->id]); - $this->assertCount(1, $events); - $event = reset($events); - $this->assertEquals('Assignment 1 due date', $event->name); + // Get user override events. + $events = $retrievalstrategy->get_raw_events([$useroverridestudent->id], $groups, [$course->id]); + $this->assertCount(1, $events); + $event = reset($events); + $this->assertEquals('Assignment 1 due date - User override', $event->name); - // Get events for user that belongs to groups A and B and has no user override events. - $this->setUser($group12student); - $events = $retrievalstrategy->get_raw_events([$group12student->id], $groups, [$course->id]); - $this->assertCount(1, $events); - $event = reset($events); - $this->assertEquals('Assignment 1 due date - Group A override', $event->name); + // Get events for user that does not belong to any group and has no user override events. + $events = $retrievalstrategy->get_raw_events([$nogroupstudent->id], $groups, [$course->id]); + $this->assertCount(1, $events); + $event = reset($events); + $this->assertEquals('Assignment 1 due date', $event->name); - // Get events for user that belongs to group A and has no user override events. - $this->setUser($group1student); - $events = $retrievalstrategy->get_raw_events([$group1student->id], $groups, [$course->id]); - $this->assertCount(1, $events); - $event = reset($events); - $this->assertEquals('Assignment 1 due date - Group A override', $event->name); + // Get events for user that belongs to groups A and B and has no user override events. + $events = $retrievalstrategy->get_raw_events([$group12student->id], $groups, [$course->id]); + $this->assertCount(1, $events); + $event = reset($events); + $this->assertEquals('Assignment 1 due date - Group A override', $event->name); + + // Get events for user that belongs to group A and has no user override events. + $events = $retrievalstrategy->get_raw_events([$group1student->id], $groups, [$course->id]); + $this->assertCount(1, $events); + $event = reset($events); + $this->assertEquals('Assignment 1 due date - Group A override', $event->name); + } // Add repeating events. $repeatingevents = [ @@ -282,8 +296,6 @@ class core_calendar_raw_event_retrieval_strategy_testcase extends advanced_testc * Test retrieval strategy with category specifications. */ public function test_get_raw_events_category() { - global $DB; - $this->resetAfterTest(); $retrievalstrategy = new raw_event_retrieval_strategy(); $generator = $this->getDataGenerator(); @@ -341,5 +353,89 @@ class core_calendar_raw_event_retrieval_strategy_testcase extends advanced_testc $events = $retrievalstrategy->get_raw_events(null, null, null, [$category1->id, $category2->id]); $this->assertCount(2, $events); } + + public function test_get_raw_events_for_multiple_users() { + $this->resetAfterTest(); + + $generator = $this->getDataGenerator(); + + // Create users. + $user1 = $generator->create_user(); + $user2 = $generator->create_user(); + $user3 = $generator->create_user(); + + // Create user events. + $events = [ + [ + 'name' => 'User1 Event', + 'eventtype' => 'user', + 'userid' => $user1->id, + 'timestart' => time(), + ], [ + 'name' => 'User2 Event', + 'eventtype' => 'user', + 'userid' => $user2->id, + 'timestart' => time(), + ], [ + 'name' => 'User3 Event', + 'eventtype' => 'user', + 'userid' => $user3->id, + 'timestart' => time(), + ] + ]; + foreach ($events as $event) { + calendar_event::create($event, false); + } + + $retrievalstrategy = new raw_event_retrieval_strategy(); + + // Get all events. + $events = $retrievalstrategy->get_raw_events([$user1->id, $user2->id]); + $this->assertCount(2, $events); + $this->assertEquals( + ['User1 Event', 'User2 Event'], + array_column($events, 'name'), + '', 0.0, 10, true); + } + + public function test_get_raw_events_for_groups_with_no_members() { + $this->resetAfterTest(); + + $generator = $this->getDataGenerator(); + + $course = $generator->create_course(); + + // Create groups. + $group1 = $generator->create_group(['courseid' => $course->id, 'name' => 'Group 1']); + $group2 = $generator->create_group(['courseid' => $course->id, 'name' => 'Group 2']); + + // Create group events. + $events = [ + [ + 'name' => 'Group 1 Event', + 'eventtype' => 'group', + 'groupid' => $group1->id, + 'timestart' => time(), + ], [ + 'name' => 'Group 2 Event', + 'eventtype' => 'group', + 'groupid' => $group2->id, + 'timestart' => time(), + ] + ]; + foreach ($events as $event) { + calendar_event::create($event, false); + } + + $retrievalstrategy = new raw_event_retrieval_strategy; + + // Get group eventsl. + $events = $retrievalstrategy->get_raw_events(null, [$group1->id, $group2->id]); + $this->assertCount(2, $events); + $this->assertEquals( + ['Group 1 Event', 'Group 2 Event'], + array_column($events, 'name'), + '', 0.0, 10, true); + } } From b2704139ad581d6c1fb3edfc863c4354d6b8dcf0 Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Thu, 5 Apr 2018 00:17:40 +1000 Subject: [PATCH 4/8] MDL-58768 coursecat: coursecat::get to accept a $user parameter --- lib/coursecatlib.php | 14 +++--- lib/tests/coursecatlib_test.php | 77 +++++++++++++++++++++++++++++++++ lib/upgrade.txt | 2 + 3 files changed, 87 insertions(+), 6 deletions(-) diff --git a/lib/coursecatlib.php b/lib/coursecatlib.php index 5ad20b73ac5..8833387190a 100644 --- a/lib/coursecatlib.php +++ b/lib/coursecatlib.php @@ -212,7 +212,7 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { /** * Returns coursecat object for requested category * - * If category is not visible to user it is treated as non existing + * If category is not visible to the given user, it is treated as non existing * unless $alwaysreturnhidden is set to true * * If id is 0, the pseudo object for root category is returned (convenient @@ -226,10 +226,11 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { * returned even if this category is not visible to the current user * (category is hidden and user does not have * 'moodle/category:viewhiddencategories' capability). Use with care! + * @param int|stdClass $user The user id or object. By default (null) checks the visibility to the current user. * @return null|coursecat * @throws moodle_exception */ - public static function get($id, $strictness = MUST_EXIST, $alwaysreturnhidden = false) { + public static function get($id, $strictness = MUST_EXIST, $alwaysreturnhidden = false, $user = null) { if (!$id) { if (!isset(self::$coursecat0)) { $record = new stdClass(); @@ -251,7 +252,7 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { $coursecatrecordcache->set($id, $coursecat); } } - if ($coursecat && ($alwaysreturnhidden || $coursecat->is_uservisible())) { + if ($coursecat && ($alwaysreturnhidden || $coursecat->is_uservisible($user))) { return $coursecat; } else { if ($strictness == MUST_EXIST) { @@ -580,17 +581,18 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { } /** - * Checks if this course category is visible to current user + * Checks if this course category is visible to a user. * * Please note that methods coursecat::get (without 3rd argumet), * coursecat::get_children(), etc. return only visible categories so it is * usually not needed to call this function outside of this class * + * @param int|stdClass $user The user id or object. By default (null) checks the visibility to the current user. * @return bool */ - public function is_uservisible() { + public function is_uservisible($user = null) { return !$this->id || $this->visible || - has_capability('moodle/category:viewhiddencategories', $this->get_context()); + has_capability('moodle/category:viewhiddencategories', $this->get_context(), $user); } /** diff --git a/lib/tests/coursecatlib_test.php b/lib/tests/coursecatlib_test.php index 54b10995477..1b7e280cbff 100644 --- a/lib/tests/coursecatlib_test.php +++ b/lib/tests/coursecatlib_test.php @@ -797,6 +797,83 @@ class core_coursecatlib_testcase extends advanced_testcase { $this->assertEquals("{$cat1name} / {$cat2name} / {$cat4name}", $category4->get_nested_name(false)); } + public function test_coursecat_is_uservisible() { + global $USER; + + // Create category 1 as visible. + $category1 = coursecat::create(array('name' => 'Cat1', 'visible' => 1)); + // Create category 2 as hidden. + $category2 = coursecat::create(array('name' => 'Cat2', 'visible' => 0)); + + $this->assertTrue($category1->is_uservisible()); + $this->assertFalse($category2->is_uservisible()); + + $this->assign_capability('moodle/category:viewhiddencategories'); + + $this->assertTrue($category1->is_uservisible()); + $this->assertTrue($category2->is_uservisible()); + + // First, store current user's id, then login as another user. + $userid = $USER->id; + $this->setUser($this->getDataGenerator()->create_user()); + + // User $user should still have the moodle/category:viewhiddencategories capability. + $this->assertTrue($category1->is_uservisible($userid)); + $this->assertTrue($category2->is_uservisible($userid)); + + $this->assign_capability('moodle/category:viewhiddencategories', CAP_INHERIT); + + $this->assertTrue($category1->is_uservisible()); + $this->assertFalse($category2->is_uservisible()); + } + + public function test_current_user_coursecat_get() { + $this->assign_capability('moodle/category:viewhiddencategories'); + + // Create category 1 as visible. + $category1 = coursecat::create(array('name' => 'Cat1', 'visible' => 1)); + // Create category 2 as hidden. + $category2 = coursecat::create(array('name' => 'Cat2', 'visible' => 0)); + + $this->assertEquals($category1->id, coursecat::get($category1->id)->id); + $this->assertEquals($category2->id, coursecat::get($category2->id)->id); + + // Login as another user to test coursecat::get. + $this->setUser($this->getDataGenerator()->create_user()); + $this->assertEquals($category1->id, coursecat::get($category1->id)->id); + + // Expecting to get an exception as this new user does not have the moodle/category:viewhiddencategories capability. + $this->expectException('moodle_exception'); + $this->expectExceptionMessage('unknowncategory'); + coursecat::get($category2->id); + } + + public function test_another_user_coursecat_get() { + global $USER; + + $this->assign_capability('moodle/category:viewhiddencategories'); + + // Create category 1 as visible. + $category1 = coursecat::create(array('name' => 'Cat1', 'visible' => 1)); + // Create category 2 as hidden. + $category2 = coursecat::create(array('name' => 'Cat2', 'visible' => 0)); + + // First, store current user's object, then login as another user. + $user1 = $USER; + $user2 = $this->getDataGenerator()->create_user(); + $this->setUser($user2); + + $this->assertEquals($category1->id, coursecat::get($category1->id, MUST_EXIST, false, $user1)->id); + $this->assertEquals($category2->id, coursecat::get($category2->id, MUST_EXIST, false, $user1)->id); + + $this->setUser($user1); + + $this->assertEquals($category1->id, coursecat::get($category1->id, MUST_EXIST, false, $user2)->id); + $this->expectException('moodle_exception'); + $this->expectExceptionMessage('unknowncategory'); + coursecat::get($category2->id, MUST_EXIST, false, $user2); + } + /** * Creates a draft area for current user and fills it with fake files * diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 04ae5771411..7bd76e0363b 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -6,6 +6,8 @@ information provided here is intended especially for developers. * Custom AJAX handlers for the form autocomplete fields can now optionally return string in their processResults() callback. If a string is returned, it is displayed instead of the list if suggested items. This can be used, for example, to inform the user that there are too many items matching the current search criteria. +* coursecat::get() now has optional $user parameter. +* coursecat::is_uservisible() now has optional $user parameter. === 3.4.3 === From 856d6bcaf76c9e91db711d5cf526ad466a2cc4b5 Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Thu, 5 Apr 2018 00:37:56 +1000 Subject: [PATCH 5/8] MDL-58768 calendar: Add $requestinguser to the container class The requesting user is not always the same as the current user. The container class now allows a different user to be set as the requesting user. All capability checks are done against the requesting user. Also, the $requestinguser is passed to core_calendar_provide_event_action and core_calendar_is_event_visible callback functions. These callback functions need to be updated in all activity modules to accept a $user parameter. --- calendar/classes/local/event/container.php | 69 +++++++++++++++++++--- calendar/lib.php | 5 ++ calendar/upgrade.txt | 2 + mod/upgrade.txt | 5 ++ 4 files changed, 73 insertions(+), 8 deletions(-) diff --git a/calendar/classes/local/event/container.php b/calendar/classes/local/event/container.php index 398304ebc20..347ddd9b80a 100644 --- a/calendar/classes/local/event/container.php +++ b/calendar/classes/local/event/container.php @@ -81,6 +81,11 @@ class container { */ protected static $modulecache = array(); + /** + * @var int The requesting user. All capability checks are done against this user. + */ + protected static $requestinguserid; + /** * Initialises the dependency graph if it hasn't yet been. */ @@ -117,11 +122,13 @@ class container { [self::class, 'apply_component_provide_event_action'], [self::class, 'apply_component_is_event_visible'], function ($dbrow) { + $requestinguserid = self::get_requesting_user(); + if (!empty($dbrow->categoryid)) { // This is a category event. Check that the category is visible to this user. - $category = \coursecat::get($dbrow->categoryid, IGNORE_MISSING, true); + $category = \coursecat::get($dbrow->categoryid, IGNORE_MISSING, true, $requestinguserid); - if (empty($category) || !$category->is_uservisible()) { + if (empty($category) || !$category->is_uservisible($requestinguserid)) { return true; } } @@ -131,7 +138,7 @@ class container { return false; } - $instances = get_fast_modinfo($dbrow->courseid)->instances; + $instances = get_fast_modinfo($dbrow->courseid, $requestinguserid)->instances; // If modinfo doesn't know about the module, we should ignore it. if (!isset($instances[$dbrow->modulename]) || !isset($instances[$dbrow->modulename][$dbrow->instance])) { @@ -152,9 +159,11 @@ class container { // 3) Only process modules for courses that are visible OR if the course is not visible, the user // has the capability to view hidden courses. $coursecontext = \context_course::instance($dbrow->courseid); - $canseecourse = has_capability('moodle/course:view', $coursecontext) || is_enrolled($coursecontext); + $canseecourse = has_capability('moodle/course:view', $coursecontext, $requestinguserid) || + is_enrolled($coursecontext, $requestinguserid); $canseecourse = $canseecourse && - ($cm->get_course()->visible || has_capability('moodle/course:viewhiddencourses', $coursecontext)); + ($cm->get_course()->visible || + has_capability('moodle/course:viewhiddencourses', $coursecontext, $requestinguserid)); if (!$cm->uservisible || !$canseecourse) { return true; } @@ -186,6 +195,7 @@ class container { * Reset all static caches, called between tests. */ public static function reset_caches() { + self::$requestinguserid = null; self::$eventfactory = null; self::$eventmapper = null; self::$eventvault = null; @@ -225,6 +235,31 @@ class container { return self::$eventvault; } + /** + * Sets the requesting user so that all capability checks are done against this user. + * Setting the requesting user (hence calling this function) is optional and if you do not so, + * $USER will be used as the requesting user. However, if you wish to set the requesting user yourself, + * you should call this function before any other function of the container class is called. + * + * @param int $userid The user id. + * @throws \coding_exception + */ + public static function set_requesting_user($userid) { + self::$requestinguserid = $userid; + } + + /** + * Returns the requesting user id. + * It usually is the current user unless it has been set explicitly using set_requesting_user. + * + * @return int + */ + public static function get_requesting_user() { + global $USER; + + return empty(self::$requestinguserid) ? $USER->id : self::$requestinguserid; + } + /** * Calls callback 'core_calendar_provide_event_action' from the component responsible for the event * @@ -240,14 +275,23 @@ class container { $mapper = self::$eventmapper; $action = null; if ($event->get_course_module()) { + $requestinguserid = self::get_requesting_user(); + $legacyevent = $mapper->from_event_to_legacy_event($event); + // We know for a fact that the the requesting user might be different from the logged in user, + // but the event mapper is not aware of that. + if (empty($event->user) && !empty($legacyevent->userid)) { + $legacyevent->userid = $requestinguserid; + } + // TODO MDL-58866 Only activity modules currently support this callback. // Any other event will not be displayed on the dashboard. $action = component_callback( 'mod_' . $event->get_course_module()->get('modname'), 'core_calendar_provide_event_action', [ - $mapper->from_event_to_legacy_event($event), - self::$actionfactory + $legacyevent, + self::$actionfactory, + $requestinguserid ] ); } @@ -274,12 +318,21 @@ class container { $mapper = self::$eventmapper; $eventvisible = null; if ($event->get_course_module()) { + $requestinguserid = self::get_requesting_user(); + $legacyevent = $mapper->from_event_to_legacy_event($event); + // We know for a fact that the the requesting user might be different from the logged in user, + // but the event mapper is not aware of that. + if (empty($event->user) && !empty($legacyevent->userid)) { + $legacyevent->userid = $requestinguserid; + } + // TODO MDL-58866 Only activity modules currently support this callback. $eventvisible = component_callback( 'mod_' . $event->get_course_module()->get('modname'), 'core_calendar_is_event_visible', [ - $mapper->from_event_to_legacy_event($event) + $legacyevent, + $requestinguserid ] ); } diff --git a/calendar/lib.php b/calendar/lib.php index 0289548737a..7070818be41 100644 --- a/calendar/lib.php +++ b/calendar/lib.php @@ -3312,6 +3312,11 @@ function calendar_get_legacy_events($tstart, $tend, $users, $groups, $courses, return $param; }, [$users, $groups, $courses, $categories]); + // If a single user is provided, we can use that for capability checks. + // Otherwise current logged in user is used - See MDL-58768. + if (is_array($userparam) && count($userparam) == 1) { + \core_calendar\local\event\container::set_requesting_user($userparam[0]); + } $mapper = \core_calendar\local\event\container::get_event_mapper(); $events = \core_calendar\local\api::get_events( $tstart, diff --git a/calendar/upgrade.txt b/calendar/upgrade.txt index c5fde56b077..d117042633a 100644 --- a/calendar/upgrade.txt +++ b/calendar/upgrade.txt @@ -4,6 +4,8 @@ information provided here is intended especially for developers. === 3.4.5 === * calendar_get_default_courses() function now has optional $userid parameter. * calendar_set_filters() function now has optional $user parameter. +* The core_calendar\local\event\container class now provides two new helper methods for getting and setting the requesting user: + set_requesting_user() and get_requesting_user(). === 3.4 === * calendar_get_mini, and calendar_get_upcoming have been deprecated. Please update to use the new exporters and renderers. diff --git a/mod/upgrade.txt b/mod/upgrade.txt index c4d60c80582..43849485386 100644 --- a/mod/upgrade.txt +++ b/mod/upgrade.txt @@ -1,6 +1,11 @@ This files describes API changes in /mod/* - activity modules, information provided here is intended especially for developers. +=== 3.4.5 === + +* Now, when mod__core_calendar_is_event_visible or mod__core_calendar_provide_event_action callback functions + are called, the userid of the requesting user is also passed to them. + === 3.4.3 === * There is a new privacy API that every subsystem and plugin has to implement so that the site can become GDPR From ec92334c18aa1f9beee6752fe15469d14425dcd9 Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Thu, 5 Apr 2018 01:15:47 +1000 Subject: [PATCH 6/8] MDL-58768 mod_assign: Add $user param to assign::can_grade --- mod/assign/locallib.php | 5 +++-- mod/assign/tests/locallib_test.php | 5 +++++ mod/assign/upgrade.txt | 3 +++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index dd24fc97795..9ae6cd0d7d9 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -3306,11 +3306,12 @@ class assign { /** * Does this user have grade permission for this assignment? * + * @param int|stdClass $user The object or id of the user who will do the editing (default to current user). * @return bool */ - public function can_grade() { + public function can_grade($user = null) { // Permissions check. - if (!has_capability('mod/assign:grade', $this->context)) { + if (!has_capability('mod/assign:grade', $this->context, $user)) { return false; } diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php index 882f9cc546a..659ae737664 100644 --- a/mod/assign/tests/locallib_test.php +++ b/mod/assign/tests/locallib_test.php @@ -1720,6 +1720,11 @@ class mod_assign_locallib_testcase extends advanced_testcase { $this->setUser($teacher); $this->assertEquals(true, $assign->can_grade()); + // Test the viewgrades capability for other users. + $this->setUser(); + $this->assertTrue($assign->can_grade($teacher->id)); + $this->assertFalse($assign->can_grade($student->id)); + // Test the viewgrades capability - without mod/assign:grade. $this->setUser($student); diff --git a/mod/assign/upgrade.txt b/mod/assign/upgrade.txt index e6cef2135bb..aa0b6af782f 100644 --- a/mod/assign/upgrade.txt +++ b/mod/assign/upgrade.txt @@ -1,5 +1,8 @@ This files describes API changes in the assign code. +=== 3.4.5 === +* The function can_grade() now has optional $user parameter. + === 3.4 === * assign::add_attempt requires that set_most_recent_team_submission() be called if attempting to use this function with a team submission. From 4ac18c8480dafca87d097d534478090575a7521c Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Thu, 5 Apr 2018 01:56:56 +1000 Subject: [PATCH 7/8] MDL-58768 core: Fix a typo for callback name --- mod/upgrade.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod/upgrade.txt b/mod/upgrade.txt index 43849485386..abf49b52e3e 100644 --- a/mod/upgrade.txt +++ b/mod/upgrade.txt @@ -38,7 +38,7 @@ information provided here is intended especially for developers. MDL-55611 can be found at https://docs.moodle.org/dev/Calendar_API. The 3 new callbacks are: - mod__core_calendar_is_event_visible - mod__core_calendar_provide_event_action - - mod__core_calendar_event_action_show_items_acount + - mod__core_calendar_event_action_shows_item_count * Changes to the moodleform_mod class and its usage (MDL-58138): - the get_data() method has been overriden. The implementation calls parent::get_data() and a new data_postprocessing() method - new data_postprocessing() method added. Mods can override this in their mod_form subclass to modify the submit data. Previously From 723e5b7533462223c8b30d24a2131c0e1efabb43 Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Thu, 5 Apr 2018 18:17:36 +1000 Subject: [PATCH 8/8] MDL-58768 mod_assign: Added userid param to calendar callbacks The mod_assign_core_calendar_is_event_visible and mod_assign_core_calendar_provide_event_action functions now accept a new parameter ($userid) so they are not always dependet to the logged in user. --- mod/assign/lib.php | 31 ++++-- mod/assign/tests/lib_test.php | 197 ++++++++++++++++++++++++++++++++++ 2 files changed, 218 insertions(+), 10 deletions(-) diff --git a/mod/assign/lib.php b/mod/assign/lib.php index 5eeccf327e4..002404dd379 100644 --- a/mod/assign/lib.php +++ b/mod/assign/lib.php @@ -1826,20 +1826,25 @@ function assign_check_updates_since(cm_info $cm, $from, $filter = array()) { * the ASSIGN_EVENT_TYPE_GRADINGDUE event will not be shown to students on their calendar. * * @param calendar_event $event + * @param int $userid User id to use for all capability checks, etc. Set to 0 for current user (default). * @return bool Returns true if the event is visible to the current user, false otherwise. */ -function mod_assign_core_calendar_is_event_visible(calendar_event $event) { +function mod_assign_core_calendar_is_event_visible(calendar_event $event, $userid = 0) { global $CFG, $USER; require_once($CFG->dirroot . '/mod/assign/locallib.php'); - $cm = get_fast_modinfo($event->courseid)->instances['assign'][$event->instance]; + if (empty($userid)) { + $userid = $USER->id; + } + + $cm = get_fast_modinfo($event->courseid, $userid)->instances['assign'][$event->instance]; $context = context_module::instance($cm->id); $assign = new assign($context, $cm, null); if ($event->eventtype == ASSIGN_EVENT_TYPE_GRADINGDUE) { - return $assign->can_grade(); + return $assign->can_grade($userid); } else { return true; } @@ -1853,22 +1858,28 @@ function mod_assign_core_calendar_is_event_visible(calendar_event $event) { * * @param calendar_event $event * @param \core_calendar\action_factory $factory + * @param int $userid User id to use for all capability checks, etc. Set to 0 for current user (default). * @return \core_calendar\local\event\entities\action_interface|null */ function mod_assign_core_calendar_provide_event_action(calendar_event $event, - \core_calendar\action_factory $factory) { + \core_calendar\action_factory $factory, + $userid = 0) { global $CFG, $USER; require_once($CFG->dirroot . '/mod/assign/locallib.php'); - $cm = get_fast_modinfo($event->courseid)->instances['assign'][$event->instance]; + if (empty($userid)) { + $userid = $USER->id; + } + + $cm = get_fast_modinfo($event->courseid, $userid)->instances['assign'][$event->instance]; $context = context_module::instance($cm->id); $assign = new assign($context, $cm, null); // Apply overrides. - $assign->update_effective_access($USER->id); + $assign->update_effective_access($userid); if ($event->eventtype == ASSIGN_EVENT_TYPE_GRADINGDUE) { $name = get_string('grade'); @@ -1877,16 +1888,16 @@ function mod_assign_core_calendar_provide_event_action(calendar_event $event, 'action' => 'grader' ]); $itemcount = $assign->count_submissions_need_grading(); - $actionable = $assign->can_grade() && (time() >= $assign->get_instance()->allowsubmissionsfromdate); + $actionable = $assign->can_grade($userid) && (time() >= $assign->get_instance()->allowsubmissionsfromdate); } else { - $usersubmission = $assign->get_user_submission($USER->id, false); + $usersubmission = $assign->get_user_submission($userid, false); if ($usersubmission && $usersubmission->status === ASSIGN_SUBMISSION_STATUS_SUBMITTED) { // The user has already submitted. // We do not want to change the text to edit the submission, we want to remove the event from the Dashboard entirely. return null; } - $participant = $assign->get_participant($USER->id); + $participant = $assign->get_participant($userid); if (!$participant) { // If the user is not a participant in the assignment then they have @@ -1901,7 +1912,7 @@ function mod_assign_core_calendar_provide_event_action(calendar_event $event, 'action' => 'editsubmission' ]); $itemcount = 1; - $actionable = $assign->is_any_submission_plugin_enabled() && $assign->can_edit_submission($USER->id); + $actionable = $assign->is_any_submission_plugin_enabled() && $assign->can_edit_submission($userid, $userid); } return $factory->create_instance( diff --git a/mod/assign/tests/lib_test.php b/mod/assign/tests/lib_test.php index 9c758c1086a..4d47f855a94 100644 --- a/mod/assign/tests/lib_test.php +++ b/mod/assign/tests/lib_test.php @@ -426,6 +426,24 @@ class mod_assign_lib_testcase extends advanced_testcase { $this->assertTrue(mod_assign_core_calendar_is_event_visible($event)); } + public function test_assign_core_calendar_is_event_visible_duedate_event_for_teacher() { + $this->resetAfterTest(); + $course = $this->getDataGenerator()->create_course(); + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $assign = $this->create_instance($course); + + $this->setAdminUser(); + + // Create a calendar event. + $event = $this->create_action_event($course, $assign, ASSIGN_EVENT_TYPE_DUE); + + // Now, log out. + $this->setUser(); + + // The teacher should see the due date event. + $this->assertTrue(mod_assign_core_calendar_is_event_visible($event, $teacher->id)); + } + public function test_assign_core_calendar_is_event_visible_duedate_event_as_student() { $this->resetAfterTest(); $course = $this->getDataGenerator()->create_course(); @@ -443,6 +461,25 @@ class mod_assign_lib_testcase extends advanced_testcase { $this->assertTrue(mod_assign_core_calendar_is_event_visible($event)); } + public function test_assign_core_calendar_is_event_visible_duedate_event_for_student() { + $this->resetAfterTest(); + $course = $this->getDataGenerator()->create_course(); + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $student = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $assign = $this->create_instance($course, ['assignsubmission_onlinetext_enabled' => 1]); + + $this->setAdminUser(); + + // Create a calendar event. + $event = $this->create_action_event($course, $assign, ASSIGN_EVENT_TYPE_DUE); + + // Now, log out. + $this->setUser(); + + // The student should care about the due date event. + $this->assertTrue(mod_assign_core_calendar_is_event_visible($event, $student->id)); + } + public function test_assign_core_calendar_is_event_visible_gradingduedate_event_as_teacher() { $this->resetAfterTest(); $course = $this->getDataGenerator()->create_course(); @@ -458,6 +495,24 @@ class mod_assign_lib_testcase extends advanced_testcase { $this->assertTrue(mod_assign_core_calendar_is_event_visible($event)); } + + public function test_assign_core_calendar_is_event_visible_gradingduedate_event_for_teacher() { + $this->resetAfterTest(); + $course = $this->getDataGenerator()->create_course(); + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $assign = $this->create_instance($course); + + // Create a calendar event. + $this->setAdminUser(); + $event = $this->create_action_event($course, $assign, ASSIGN_EVENT_TYPE_GRADINGDUE); + + // Now, log out. + $this->setUser(); + + // The teacher should see the due date event. + $this->assertTrue(mod_assign_core_calendar_is_event_visible($event, $teacher->id)); + } + public function test_assign_core_calendar_is_event_visible_gradingduedate_event_as_student() { $this->resetAfterTest(); $course = $this->getDataGenerator()->create_course(); @@ -473,6 +528,24 @@ class mod_assign_lib_testcase extends advanced_testcase { $this->assertFalse(mod_assign_core_calendar_is_event_visible($event)); } + + public function test_assign_core_calendar_is_event_visible_gradingduedate_event_for_student() { + $this->resetAfterTest(); + $course = $this->getDataGenerator()->create_course(); + $student = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $assign = $this->create_instance($course); + + // Create a calendar event. + $this->setAdminUser(); + $event = $this->create_action_event($course, $assign, ASSIGN_EVENT_TYPE_GRADINGDUE); + + // Now, log out. + $this->setUser(); + + // The student should not see the due date event. + $this->assertFalse(mod_assign_core_calendar_is_event_visible($event, $student->id)); + } + public function test_assign_core_calendar_provide_event_action_duedate_as_teacher() { $this->resetAfterTest(); $course = $this->getDataGenerator()->create_course(); @@ -492,6 +565,27 @@ class mod_assign_lib_testcase extends advanced_testcase { $this->assertNull($actionevent); } + public function test_assign_core_calendar_provide_event_action_duedate_for_teacher() { + $this->resetAfterTest(); + $course = $this->getDataGenerator()->create_course(); + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $assign = $this->create_instance($course); + + // Create a calendar event. + $this->setAdminUser(); + $event = $this->create_action_event($course, $assign, ASSIGN_EVENT_TYPE_DUE); + + // Now, log out. + $this->setUser(); + + // Decorate action event for a teacher. + $factory = new \core_calendar\action_factory(); + $actionevent = mod_assign_core_calendar_provide_event_action($event, $factory, $teacher->id); + + // The teacher should not have an action for a due date event. + $this->assertNull($actionevent); + } + public function test_assign_core_calendar_provide_event_action_duedate_as_student() { $this->resetAfterTest(); $course = $this->getDataGenerator()->create_course(); @@ -515,6 +609,31 @@ class mod_assign_lib_testcase extends advanced_testcase { $this->assertTrue($actionevent->is_actionable()); } + public function test_assign_core_calendar_provide_event_action_duedate_for_student() { + $this->resetAfterTest(); + $course = $this->getDataGenerator()->create_course(); + $student = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $assign = $this->create_instance($course, ['assignsubmission_onlinetext_enabled' => 1]); + + // Create a calendar event. + $this->setAdminUser(); + $event = $this->create_action_event($course, $assign, ASSIGN_EVENT_TYPE_DUE); + + // Now, log out. + $this->setUser(); + + // Decorate action event for a student. + $factory = new \core_calendar\action_factory(); + $actionevent = mod_assign_core_calendar_provide_event_action($event, $factory, $student->id); + + // Confirm the event was decorated. + $this->assertInstanceOf('\core_calendar\local\event\value_objects\action', $actionevent); + $this->assertEquals(get_string('addsubmission', 'assign'), $actionevent->get_name()); + $this->assertInstanceOf('moodle_url', $actionevent->get_url()); + $this->assertEquals(1, $actionevent->get_item_count()); + $this->assertTrue($actionevent->is_actionable()); + } + public function test_assign_core_calendar_provide_event_action_gradingduedate_as_teacher() { $this->resetAfterTest(); $course = $this->getDataGenerator()->create_course(); @@ -537,6 +656,31 @@ class mod_assign_lib_testcase extends advanced_testcase { $this->assertTrue($actionevent->is_actionable()); } + public function test_assign_core_calendar_provide_event_action_gradingduedate_for_teacher() { + $this->resetAfterTest(); + $course = $this->getDataGenerator()->create_course(); + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $assign = $this->create_instance($course); + + // Create a calendar event. + $this->setAdminUser(); + $event = $this->create_action_event($course, $assign, ASSIGN_EVENT_TYPE_GRADINGDUE); + + // Now, log out. + $this->setUser(); + + // Decorate action event for a teacher. + $factory = new \core_calendar\action_factory(); + $actionevent = mod_assign_core_calendar_provide_event_action($event, $factory, $teacher->id); + + // Confirm the event was decorated. + $this->assertInstanceOf('\core_calendar\local\event\value_objects\action', $actionevent); + $this->assertEquals(get_string('grade'), $actionevent->get_name()); + $this->assertInstanceOf('moodle_url', $actionevent->get_url()); + $this->assertEquals(0, $actionevent->get_item_count()); + $this->assertTrue($actionevent->is_actionable()); + } + public function test_assign_core_calendar_provide_event_action_gradingduedate_as_student() { $this->resetAfterTest(); $course = $this->getDataGenerator()->create_course(); @@ -559,6 +703,31 @@ class mod_assign_lib_testcase extends advanced_testcase { $this->assertFalse($actionevent->is_actionable()); } + public function test_assign_core_calendar_provide_event_action_gradingduedate_for_student() { + $this->resetAfterTest(); + $course = $this->getDataGenerator()->create_course(); + $student = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $assign = $this->create_instance($course); + + // Create a calendar event. + $this->setAdminUser(); + $event = $this->create_action_event($course, $assign, ASSIGN_EVENT_TYPE_GRADINGDUE); + + // Now, log out. + $this->setUser(); + + // Decorate action event for a student. + $factory = new \core_calendar\action_factory(); + $actionevent = mod_assign_core_calendar_provide_event_action($event, $factory, $student->id); + + // Confirm the event was decorated. + $this->assertInstanceOf('\core_calendar\local\event\value_objects\action', $actionevent); + $this->assertEquals(get_string('grade'), $actionevent->get_name()); + $this->assertInstanceOf('moodle_url', $actionevent->get_url()); + $this->assertEquals(0, $actionevent->get_item_count()); + $this->assertFalse($actionevent->is_actionable()); + } + public function test_assign_core_calendar_provide_event_action_duedate_as_student_submitted() { $this->resetAfterTest(); $course = $this->getDataGenerator()->create_course(); @@ -584,6 +753,34 @@ class mod_assign_lib_testcase extends advanced_testcase { $this->assertNull($actionevent); } + public function test_assign_core_calendar_provide_event_action_duedate_for_student_submitted() { + $this->resetAfterTest(); + $course = $this->getDataGenerator()->create_course(); + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $student = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $assign = $this->create_instance($course, ['assignsubmission_onlinetext_enabled' => 1]); + + $this->setAdminUser(); + + // Create a calendar event. + $event = $this->create_action_event($course, $assign, ASSIGN_EVENT_TYPE_DUE); + + // Create an action factory. + $factory = new \core_calendar\action_factory(); + + // Submit as the student. + $this->add_submission($student, $assign); + $this->submit_for_grading($student, $assign); + + // Now, log out. + $this->setUser(); + + // Confirm there was no event to action. + $factory = new \core_calendar\action_factory(); + $actionevent = mod_assign_core_calendar_provide_event_action($event, $factory, $student->id); + $this->assertNull($actionevent); + } + /** * Creates an action event. *