Commit f52459bb authored by jun's avatar jun
Browse files

MDL-55956 calendar: Show only one of duplicate events relevant to user

If there are multiple, non-repeating events with the same module name,
instance and event type. The most specific event or the event with the
highest priority will be shown.
The ordering of event priorities:
  User override events > Group override events > Course events.
If there are no user override events and there are multiple group
overrides for an event, then the one with the highest priority will be
shown.
parent 98239b2d
......@@ -728,85 +728,164 @@ function calendar_add_event_metadata($event) {
* @param boolean $ignorehidden whether to select only visible events or all events
* @return array $events of selected events or an empty array if there aren't any (or there was an error)
*/
function calendar_get_events($tstart, $tend, $users, $groups, $courses, $withduration=true, $ignorehidden=true) {
function calendar_get_events($tstart, $tend, $users, $groups, $courses, $withduration = true, $ignorehidden = true) {
global $DB;
$whereclause = '';
$params = array();
// Quick test.
if (empty($users) && empty($groups) && empty($courses)) {
return array();
}
// Array of filter conditions. To be concatenated by the OR operator.
$filters = [];
// User filter.
if ((is_array($users) && !empty($users)) or is_numeric($users)) {
// Events from a number of users
if(!empty($whereclause)) $whereclause .= ' OR';
list($insqlusers, $inparamsusers) = $DB->get_in_or_equal($users, SQL_PARAMS_NAMED);
$whereclause .= " (e.userid $insqlusers AND e.courseid = 0 AND e.groupid = 0)";
$filters[] = "(e.userid $insqlusers AND e.courseid = 0 AND e.groupid = 0)";
$params = array_merge($params, $inparamsusers);
} else if($users === true) {
} else if ($users === true) {
// Events from ALL users
if(!empty($whereclause)) $whereclause .= ' OR';
$whereclause .= ' (e.userid != 0 AND e.courseid = 0 AND e.groupid = 0)';
} else if($users === false) {
// No user at all, do nothing
$filters[] = "(e.userid != 0 AND e.courseid = 0 AND e.groupid = 0)";
}
// 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)) {
// Events from a number of groups
if(!empty($whereclause)) $whereclause .= ' OR';
list($insqlgroups, $inparamsgroups) = $DB->get_in_or_equal($groups, SQL_PARAMS_NAMED);
$whereclause .= " e.groupid $insqlgroups ";
$filters[] = "e.groupid $insqlgroups";
$params = array_merge($params, $inparamsgroups);
} else if($groups === true) {
} else if ($groups === true) {
// Events from ALL groups
if(!empty($whereclause)) $whereclause .= ' OR ';
$whereclause .= ' e.groupid != 0';
$filters[] = "e.groupid != 0";
}
// boolean false (no groups at all): we don't need to do anything
// 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(!empty($whereclause)) $whereclause .= ' OR';
list($insqlcourses, $inparamscourses) = $DB->get_in_or_equal($courses, SQL_PARAMS_NAMED);
$whereclause .= " (e.groupid = 0 AND e.courseid $insqlcourses)";
$filters[] = "(e.groupid = 0 AND e.courseid $insqlcourses)";
$params = array_merge($params, $inparamscourses);
} else if ($courses === true) {
// Events from ALL courses
if(!empty($whereclause)) $whereclause .= ' OR';
$whereclause .= ' (e.groupid = 0 AND e.courseid != 0)';
$filters[] = "(e.groupid = 0 AND e.courseid != 0)";
}
// Security check: if, by now, we have NOTHING in $whereclause, then it means
// that NO event-selecting clauses were defined. Thus, we won't be returning ANY
// events no matter what. Allowing the code to proceed might return a completely
// valid query with only time constraints, thus selecting ALL events in that time frame!
if(empty($whereclause)) {
if (empty($filters)) {
return array();
}
if($withduration) {
$timeclause = '(e.timestart >= '.$tstart.' OR e.timestart + e.timeduration > '.$tstart.') AND e.timestart <= '.$tend;
// Build our clause for the filters.
$filterclause = implode(' OR ', $filters);
// Array of where conditions for our query. To be concatenated by the AND operator.
$whereconditions = ["($filterclause)"];
// Time clause.
if ($withduration) {
$timeclause = "((e.timestart >= :tstart1 OR e.timestart + e.timeduration > :tstart2) AND e.timestart <= :tend)";
$params['tstart1'] = $tstart;
$params['tstart2'] = $tstart;
$params['tend'] = $tend;
} else {
$timeclause = "(e.timestart >= :tstart AND e.timestart <= :tend)";
$params['tstart'] = $tstart;
$params['tend'] = $tend;
}
else {
$timeclause = 'e.timestart >= '.$tstart.' AND e.timestart <= '.$tend;
$whereconditions[] = $timeclause;
// Show visible only.
if ($ignorehidden) {
$whereconditions[] = "(e.visible = 1)";
}
if(!empty($whereclause)) {
// We have additional constraints
$whereclause = $timeclause.' AND ('.$whereclause.')';
// Build the main query's WHERE clause.
$whereclause = implode(' AND ', $whereconditions);
// 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();
// Set calendar filters.
list($usercourses, $usergroups, $user) = calendar_set_filters($usercourses, true);
$subqueryparams = [];
// Flag to indicate whether the query needs to exclude group overrides.
$viewgroupsonly = false;
if ($user) {
// Set filter condition for the user's events.
$subqueryconditions[] = "(ev.userid = :user AND ev.courseid = 0 AND ev.groupid = 0)";
$subqueryparams['user'] = $user;
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;
}
}
}
}
else {
// Just basic time filtering
$whereclause = $timeclause;
// 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);
}
if ($ignorehidden) {
$whereclause .= ' AND e.visible = 1';
// Set filter condition for the user's courses.
if (!empty($usercourses)) {
list($inusercourses, $inusercoursesparams) = $DB->get_in_or_equal($usercourses, SQL_PARAMS_NAMED);
$subqueryconditions[] = "(ev.groupid = 0 AND ev.courseid $inusercourses)";
$subqueryparams = array_merge($subqueryparams, $inusercoursesparams);
}
// Build the WHERE condition for the sub-query.
if (!empty($subqueryconditions)) {
$subquerywhere = 'WHERE ' . implode(" OR ", $subqueryconditions);
}
// Merge subquery parameters to the parameters of the main query.
if (!empty($subqueryparams)) {
$params = array_merge($params, $subqueryparams);
}
// Sub-query that fetches the list of unique events that were filtered based on priority.
$subquery = "SELECT ev.modulename,
ev.instance,
ev.eventtype,
MAX(ev.priority) as priority
FROM {event} ev
$subquerywhere
GROUP BY ev.modulename, ev.instance, ev.eventtype";
// Build the main query.
$sql = "SELECT e.*
FROM {event} e
LEFT JOIN {modules} m ON e.modulename = m.name
-- Non visible modules will have a value of 0.
INNER JOIN ($subquery) fe
ON e.modulename = fe.modulename
AND e.instance = fe.instance
AND e.eventtype = fe.eventtype
AND (e.priority = fe.priority OR (e.priority IS NULL AND fe.priority IS NULL))
LEFT JOIN {modules} m
ON e.modulename = m.name
WHERE (m.visible = 1 OR m.visible IS NULL) AND $whereclause
ORDER BY e.timestart";
$events = $DB->get_records_sql($sql, $params);
......
......@@ -62,7 +62,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase {
*/
public static function create_calendar_event($name, $userid = 0, $type = 'user', $repeats = 0, $timestart = null, $prop = null) {
global $CFG, $DB, $USER, $SITE;
global $CFG, $DB, $SITE;
require_once("$CFG->dirroot/calendar/lib.php");
if (!empty($prop)) {
......@@ -100,6 +100,26 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase {
if (!isset($prop->courseid)) {
$prop->courseid = $SITE->id;
}
// Determine event priority.
if ($prop->courseid == 0 && isset($prop->groupid) && $prop->groupid == 0 && !empty($prop->userid)) {
// User override event.
$prop->priority = CALENDAR_EVENT_USER_OVERRIDE_PRIORITY;
} else if ($prop->courseid != $SITE->id && !empty($prop->groupid)) {
// Group override event.
$priorityparams = ['courseid' => $prop->courseid, 'groupid' => $prop->groupid];
// Group override event with the highest priority.
$groupevents = $DB->get_records('event', $priorityparams, 'priority DESC', 'id, priority', 0, 1);
$priority = 1;
if (!empty($groupevents)) {
$event = reset($groupevents);
if (!empty($event->priority)) {
$priority = $event->priority + 1;
}
}
$prop->priority = $priority;
}
$event = new calendar_event($prop);
return $event->create($prop);
}
......@@ -266,6 +286,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase {
global $DB, $USER;
$this->resetAfterTest(true);
set_config('calendar_adminseesall', 1);
$this->setAdminUser();
// Create a few stuff to test with.
......@@ -296,13 +317,19 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase {
$record = new stdClass();
$record->courseid = $course->id;
$record->groupid = 0;
$record->description = array(
'format' => FORMAT_HTML,
'text' => 'Text with img <img src="@@PLUGINFILE@@/fakeimage.png">',
'itemid' => $draftidfile
);
$courseevent = $this->create_calendar_event('course', $USER->id, 'course', 2, time(), $record);
$userevent = $this->create_calendar_event('user', $USER->id);
$record = new stdClass();
$record->courseid = 0;
$record->groupid = 0;
$userevent = $this->create_calendar_event('user', $USER->id, 'user', 0, time(), $record);
$record = new stdClass();
$record->courseid = $course->id;
$record->groupid = $group->id;
......@@ -335,6 +362,13 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase {
$this->assertEquals(2, $withdescription);
// Let's play around with caps.
// Create user event for the user $user.
$record = new stdClass();
$record->courseid = 0;
$record->groupid = 0;
$this->create_calendar_event('user', $user->id, 'user', 0, time(), $record);
$this->setUser($user);
$events = core_calendar_external::get_calendar_events($paramevents, $options);
$events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events);
......
......@@ -119,35 +119,42 @@ class core_calendar_lib_testcase extends advanced_testcase {
public function test_calendar_get_events_with_disabled_module() {
global $DB;
$course = $this->getDataGenerator()->create_course();
$events = [[
'name' => 'Start of assignment',
'description' => '',
'format' => 1,
'courseid' => $course->id,
'groupid' => 0,
'userid' => 2,
'modulename' => 'assign',
'instance' => 1,
'eventtype' => 'due',
'timestart' => time(),
'timeduration' => 86400,
'visible' => 1
], [
'name' => 'Start of lesson',
'description' => '',
'format' => 1,
'courseid' => $course->id,
'groupid' => 0,
'userid' => 2,
'modulename' => 'lesson',
'instance' => 1,
'eventtype' => 'end',
'timestart' => time(),
'timeduration' => 86400,
'visible' => 1
]
];
$generator = $this->getDataGenerator();
$course = $generator->create_course();
$student = $generator->create_user();
$generator->enrol_user($student->id, $course->id, 'student');
$this->setUser($student);
$events = [
[
'name' => 'Start of assignment',
'description' => '',
'format' => 1,
'courseid' => $course->id,
'groupid' => 0,
'userid' => 2,
'modulename' => 'assign',
'instance' => 1,
'eventtype' => 'due',
'timestart' => time(),
'timeduration' => 86400,
'visible' => 1
], [
'name' => 'Start of lesson',
'description' => '',
'format' => 1,
'courseid' => $course->id,
'groupid' => 0,
'userid' => 2,
'modulename' => 'lesson',
'instance' => 1,
'eventtype' => 'end',
'timestart' => time(),
'timeduration' => 86400,
'visible' => 1
]
];
foreach ($events as $event) {
calendar_event::create($event, false);
......@@ -171,4 +178,186 @@ class core_calendar_lib_testcase extends advanced_testcase {
$event = reset($events);
$this->assertEquals('assign', $event->modulename);
}
/**
* Test for calendar_get_events() when there are user and group overrides.
*/
public function test_calendar_get_events_with_overrides() {
global $DB;
$generator = $this->getDataGenerator();
$course = $generator->create_course();
$plugingenerator = $this->getDataGenerator()->get_plugin_generator('mod_assign');
if (!isset($params['course'])) {
$params['course'] = $course->id;
}
$instance = $plugingenerator->create_instance($params);
// Create users.
$useroverridestudent = $generator->create_user();
$group1student = $generator->create_user();
$group2student = $generator->create_user();
$group12student = $generator->create_user();
$nogroupstudent = $generator->create_user();
// Enrol users.
$generator->enrol_user($useroverridestudent->id, $course->id, 'student');
$generator->enrol_user($group1student->id, $course->id, 'student');
$generator->enrol_user($group2student->id, $course->id, 'student');
$generator->enrol_user($group12student->id, $course->id, 'student');
$generator->enrol_user($nogroupstudent->id, $course->id, 'student');
// Create groups.
$group1 = $generator->create_group(['courseid' => $course->id]);
$group2 = $generator->create_group(['courseid' => $course->id]);
// Add members to groups.
$generator->create_group_member(['groupid' => $group1->id, 'userid' => $group1student->id]);
$generator->create_group_member(['groupid' => $group2->id, 'userid' => $group2student->id]);
$generator->create_group_member(['groupid' => $group1->id, 'userid' => $group12student->id]);
$generator->create_group_member(['groupid' => $group2->id, 'userid' => $group12student->id]);
$now = time();
// Events with the same module name, instance and event type.
$events = [
[
'name' => 'Assignment 1 due date',
'description' => '',
'format' => 0,
'courseid' => $course->id,
'groupid' => 0,
'userid' => 2,
'modulename' => 'assign',
'instance' => $instance->id,
'eventtype' => 'due',
'timestart' => $now,
'timeduration' => 0,
'visible' => 1
], [
'name' => 'Assignment 1 due date - User override',
'description' => '',
'format' => 1,
'courseid' => 0,
'groupid' => 0,
'userid' => $useroverridestudent->id,
'modulename' => 'assign',
'instance' => $instance->id,
'eventtype' => 'due',
'timestart' => $now + 86400,
'timeduration' => 0,
'visible' => 1,
'priority' => CALENDAR_EVENT_USER_OVERRIDE_PRIORITY
], [
'name' => 'Assignment 1 due date - Group A override',
'description' => '',
'format' => 1,
'courseid' => $course->id,
'groupid' => $group1->id,
'userid' => 2,
'modulename' => 'assign',
'instance' => $instance->id,
'eventtype' => 'due',
'timestart' => $now + (2 * 86400),
'timeduration' => 0,
'visible' => 1,
'priority' => 1,
], [
'name' => 'Assignment 1 due date - Group B override',
'description' => '',
'format' => 1,
'courseid' => $course->id,
'groupid' => $group2->id,
'userid' => 2,
'modulename' => 'assign',
'instance' => $instance->id,
'eventtype' => 'due',
'timestart' => $now + (3 * 86400),
'timeduration' => 0,
'visible' => 1,
'priority' => 2,
],
];
foreach ($events as $event) {
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 = calendar_get_events($timestart, $timeend, $useroverridestudent->id, $groups, $course->id);
$this->assertCount(1, $events);
$event = reset($events);
$this->assertEquals('Assignment 1 due date - User override', $event->name);
// Get event for user with override but with the timestart and timeend parameters only covering the original event.
$events = calendar_get_events($timestart, $now, $useroverridestudent->id, $groups, $course->id);
$this->assertCount(0, $events);
// Get events for user that does not belong to any group and has no user override events.
$this->setUser($nogroupstudent);
$events = calendar_get_events($timestart, $timeend, $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 groups A and B and has no user override events.
$this->setUser($group12student);
$events = calendar_get_events($timestart, $timeend, $group12student->id, $groups, $course->id);
$this->assertCount(1, $events);
$event = reset($events);
$this->assertEquals('Assignment 1 due date - Group B override', $event->name);
// Get events for user that belongs to group A and has no user override events.
$this->setUser($group1student);
$events = calendar_get_events($timestart, $timeend, $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 = [
[
'name' => 'Repeating site event',
'description' => '',
'format' => 1,
'courseid' => SITEID,
'groupid' => 0,
'userid' => 2,
'repeatid' => 1,
'modulename' => '0',
'instance' => 0,
'eventtype' => 'site',
'timestart' => $now + 86400,
'timeduration' => 0,
'visible' => 1,
],
[
'name' => 'Repeating site event',
'description' => '',
'format' => 1,
'courseid' => SITEID,
'groupid' => 0,
'userid' => 2,
'repeatid' => 1,
'modulename' => '0',
'instance' => 0,
'eventtype' => 'site',
'timestart' => $now + (2 * 86400),
'timeduration' => 0,
'visible' => 1,
],
];
foreach ($repeatingevents as $event) {
calendar_event::create($event, false);
}
// Make sure repeating events are not filtered out.
$events = calendar_get_events($timestart, $timeend, true, true, true);
$this->assertCount(3, $events);
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment