Merge branch 'MDL-58736-master' of git://github.com/cameron1729/moodle
authorAndrew Nicols <andrew@nicols.co.uk>
Thu, 4 May 2017 06:33:27 +0000 (14:33 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Thu, 4 May 2017 06:33:27 +0000 (14:33 +0800)
calendar/lib.php
calendar/tests/lib_test.php

index b3ffe7f..f75cc9e 100644 (file)
@@ -1076,186 +1076,91 @@ class calendar_information {
  * @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) {
-    // We have a new implementation of this function in the calendar API class, which has slightly different behaviour
-    // so the old implementation must remain here.
     global $DB;
-    $params = array();
 
+    $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.
+        // Events from a number of users
+        if(!empty($whereclause)) $whereclause .= ' OR';
         list($insqlusers, $inparamsusers) = $DB->get_in_or_equal($users, SQL_PARAMS_NAMED);
-        $filters[] = "(e.userid $insqlusers AND e.courseid = 0 AND e.groupid = 0)";
+        $whereclause .= " (e.userid $insqlusers AND e.courseid = 0 AND e.groupid = 0)";
         $params = array_merge($params, $inparamsusers);
-    } else if ($users === true) {
-        // Events from ALL users.
-        $filters[] = "(e.userid != 0 AND e.courseid = 0 AND e.groupid = 0)";
+    } 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
     }
 
-    // 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.
+        // Events from a number of groups
+        if(!empty($whereclause)) $whereclause .= ' OR';
         list($insqlgroups, $inparamsgroups) = $DB->get_in_or_equal($groups, SQL_PARAMS_NAMED);
-        $filters[] = "e.groupid $insqlgroups";
+        $whereclause .= " e.groupid $insqlgroups ";
         $params = array_merge($params, $inparamsgroups);
-    } else if ($groups === true) {
-        // Events from ALL groups.
-        $filters[] = "e.groupid != 0";
+    } else if($groups === true) {
+        // Events from ALL groups
+        if(!empty($whereclause)) $whereclause .= ' OR ';
+        $whereclause .= ' 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);
-        $filters[] = "(e.groupid = 0 AND e.courseid $insqlcourses)";
+        $whereclause .= " (e.groupid = 0 AND e.courseid $insqlcourses)";
         $params = array_merge($params, $inparamscourses);
     } else if ($courses === true) {
-        // Events from ALL courses.
-        $filters[] = "(e.groupid = 0 AND e.courseid != 0)";
+        // Events from ALL courses
+        if(!empty($whereclause)) $whereclause .= ' OR';
+        $whereclause .= ' (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($filters)) {
+    if(empty($whereclause)) {
         return array();
     }
 
-    // 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;
-    }
-    $whereconditions[] = $timeclause;
-
-    // Show visible only.
-    if ($ignorehidden) {
-        $whereconditions[] = "(e.visible = 1)";
-    }
-
-    // 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;
-                }
-            }
-        }
-    }
-
-    // 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($withduration) {
+        $timeclause = '(e.timestart >= '.$tstart.' OR e.timestart + e.timeduration > '.$tstart.') AND e.timestart <= '.$tend;
     }
-
-    // Get courses to be used for the subquery.
-    $subquerycourses = [];
-    if (is_array($courses)) {
-        $subquerycourses = $courses;
-    } else if (is_numeric($courses)) {
-        $subquerycourses[] = $courses;
+    else {
+        $timeclause = 'e.timestart >= '.$tstart.' AND e.timestart <= '.$tend;
     }
-
-    // Merge with user courses, if necessary.
-    if (!empty($usercourses)) {
-        $subquerycourses = array_merge($subquerycourses, $usercourses);
-        // Make sure we remove duplicate values.
-        $subquerycourses = array_unique($subquerycourses);
+    if(!empty($whereclause)) {
+        // We have additional constraints
+        $whereclause = $timeclause.' AND ('.$whereclause.')';
     }
-
-    // Set subquery filter condition for the courses.
-    if (!empty($subquerycourses)) {
-        list($incourses, $incoursesparams) = $DB->get_in_or_equal($subquerycourses, SQL_PARAMS_NAMED);
-        $subqueryconditions[] = "(ev.groupid = 0 AND ev.courseid $incourses)";
-        $subqueryparams = array_merge($subqueryparams, $incoursesparams);
+    else {
+        // Just basic time filtering
+        $whereclause = $timeclause;
     }
 
-    // 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);
+    if ($ignorehidden) {
+        $whereclause .= ' AND e.visible = 1';
     }
 
-    // Sub-query that fetches the list of unique events that were filtered based on priority.
-    $subquery = "SELECT ev.modulename,
-                            ev.instance,
-                            ev.eventtype,
-                            MIN(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
-            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";
+              FROM {event} e
+         LEFT JOIN {modules} m ON e.modulename = m.name
+                -- Non visible modules will have a value of 0.
+             WHERE (m.visible = 1 OR m.visible IS NULL) AND $whereclause
+          ORDER BY e.timestart";
     $events = $DB->get_records_sql($sql, $params);
 
     if ($events === false) {
         $events = array();
     }
-
     return $events;
 }
 
index 837692e..2f627ed 100644 (file)
@@ -105,172 +105,6 @@ class core_calendar_lib_testcase extends advanced_testcase {
         $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 A 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' => $event->id,
-                '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' => $event->id,
-                '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);
-    }
-
     public function test_get_course_cached() {
         // Setup some test courses.
         $course1 = $this->getDataGenerator()->create_course();