From 0582765407d0fa027a262807d153957467a2a469 Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Tue, 8 Nov 2016 08:30:57 +0800 Subject: [PATCH 1/2] MDL-56120 calendar: Fix disabled modules showing events. calendar_get_events now check s for disabled activities and only shows events for active modules. --- calendar/lib.php | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/calendar/lib.php b/calendar/lib.php index d93396dee05..78de07a3155 100644 --- a/calendar/lib.php +++ b/calendar/lib.php @@ -737,12 +737,12 @@ function calendar_get_events($tstart, $tend, $users, $groups, $courses, $withdur // Events from a number of users if(!empty($whereclause)) $whereclause .= ' OR'; list($insqlusers, $inparamsusers) = $DB->get_in_or_equal($users, SQL_PARAMS_NAMED); - $whereclause .= " (userid $insqlusers AND courseid = 0 AND 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 if(!empty($whereclause)) $whereclause .= ' OR'; - $whereclause .= ' (userid != 0 AND courseid = 0 AND groupid = 0)'; + $whereclause .= ' (e.userid != 0 AND e.courseid = 0 AND e.groupid = 0)'; } else if($users === false) { // No user at all, do nothing } @@ -751,24 +751,24 @@ function calendar_get_events($tstart, $tend, $users, $groups, $courses, $withdur // Events from a number of groups if(!empty($whereclause)) $whereclause .= ' OR'; list($insqlgroups, $inparamsgroups) = $DB->get_in_or_equal($groups, SQL_PARAMS_NAMED); - $whereclause .= " groupid $insqlgroups "; + $whereclause .= " e.groupid $insqlgroups "; $params = array_merge($params, $inparamsgroups); } else if($groups === true) { // Events from ALL groups if(!empty($whereclause)) $whereclause .= ' OR '; - $whereclause .= ' groupid != 0'; + $whereclause .= ' e.groupid != 0'; } // boolean false (no groups at all): we don't need to do anything 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 .= " (groupid = 0 AND courseid $insqlcourses)"; + $whereclause .= " (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 .= ' (groupid = 0 AND courseid != 0)'; + $whereclause .= ' (e.groupid = 0 AND e.courseid != 0)'; } // Security check: if, by now, we have NOTHING in $whereclause, then it means @@ -780,10 +780,10 @@ function calendar_get_events($tstart, $tend, $users, $groups, $courses, $withdur } if($withduration) { - $timeclause = '(timestart >= '.$tstart.' OR timestart + timeduration > '.$tstart.') AND timestart <= '.$tend; + $timeclause = '(e.timestart >= '.$tstart.' OR e.timestart + e.timeduration > '.$tstart.') AND e.timestart <= '.$tend; } else { - $timeclause = 'timestart >= '.$tstart.' AND timestart <= '.$tend; + $timeclause = 'e.timestart >= '.$tstart.' AND e.timestart <= '.$tend; } if(!empty($whereclause)) { // We have additional constraints @@ -795,10 +795,17 @@ function calendar_get_events($tstart, $tend, $users, $groups, $courses, $withdur } if ($ignorehidden) { - $whereclause .= ' AND visible = 1'; + $whereclause .= ' AND e.visible = 1'; } - $events = $DB->get_records_select('event', $whereclause, $params, 'timestart'); + $sql = "SELECT e.* + 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(); } From 197554d49a8247bc0821c8675d127ce8591a0d08 Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Tue, 8 Nov 2016 08:32:09 +0800 Subject: [PATCH 2/2] MDL-56120 calendar: Unit tests for getting events. Added a unit test for checking that calendar_get_events will only return events for activated activities. --- calendar/tests/lib_test.php | 60 +++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/calendar/tests/lib_test.php b/calendar/tests/lib_test.php index 2eef505a4a8..1d74d01ff6f 100644 --- a/calendar/tests/lib_test.php +++ b/calendar/tests/lib_test.php @@ -111,4 +111,64 @@ class core_calendar_lib_testcase extends advanced_testcase { $this->expectOutputRegex('/Error updating calendar subscription: The given iCal URL is invalid/'); calendar_cron(); } + + /** + * Test the calendar_get_events() function only returns activity + * events that are enabled. + */ + 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 + ] + ]; + + foreach ($events as $event) { + calendar_event::create($event, false); + } + + $timestart = time() - 60; + $timeend = time() + 60; + + // Get all events. + $events = calendar_get_events($timestart, $timeend, true, 0, true); + $this->assertCount(2, $events); + + // Disable the lesson module. + $modulerecord = $DB->get_record('modules', ['name' => 'lesson']); + $modulerecord->visible = 0; + $DB->update_record('modules', $modulerecord); + + // Check that we only return the assign event. + $events = calendar_get_events($timestart, $timeend, true, 0, true); + $this->assertCount(1, $events); + $event = reset($events); + $this->assertEquals('assign', $event->modulename); + } }