From 7aa3925c9a9d0aa8930a5bdaada33a1113744049 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Wed, 26 Apr 2017 10:32:32 +0800 Subject: [PATCH] MDL-56251 format_weeks: do not use caches in event observers --- course/format/weeks/classes/observer.php | 27 +++++----- course/format/weeks/lib.php | 59 ++++++++++++++++----- course/format/weeks/tests/observer_test.php | 20 +++---- 3 files changed, 69 insertions(+), 37 deletions(-) diff --git a/course/format/weeks/classes/observer.php b/course/format/weeks/classes/observer.php index d3595808c79..7be6b63c0b2 100644 --- a/course/format/weeks/classes/observer.php +++ b/course/format/weeks/classes/observer.php @@ -39,11 +39,10 @@ class format_weeks_observer { * @param \core\event\course_updated $event */ public static function course_updated(\core\event\course_updated $event) { - $format = course_get_format($event->courseid); - - // Only want to update the end date if the course format has been set as weeks. - if ($format->get_format() == 'weeks') { - $format->update_end_date(); + if (class_exists('format_weeks', false)) { + // If class format_weeks was never loaded, this is definitely not a course in 'weeks' format. + // Course may still be in another format but format_weeks::update_end_date() will check it. + format_weeks::update_end_date($event->courseid); } } @@ -53,11 +52,10 @@ class format_weeks_observer { * @param \core\event\course_section_created $event */ public static function course_section_created(\core\event\course_section_created $event) { - $format = course_get_format($event->courseid); - - // Only want to update the end date if the course format has been set as weeks. - if ($format->get_format() == 'weeks') { - $format->update_end_date(); + if (class_exists('format_weeks', false)) { + // If class format_weeks was never loaded, this is definitely not a course in 'weeks' format. + // Course may still be in another format but format_weeks::update_end_date() will check it. + format_weeks::update_end_date($event->courseid); } } @@ -67,11 +65,10 @@ class format_weeks_observer { * @param \core\event\course_section_deleted $event */ public static function course_section_deleted(\core\event\course_section_deleted $event) { - $format = course_get_format($event->courseid); - - // Only want to update the end date if the course format has been set as weeks. - if ($format->get_format() == 'weeks') { - $format->update_end_date(); + if (class_exists('format_weeks', false)) { + // If class format_weeks was never loaded, this is definitely not a course in 'weeks' format. + // Course may still be in another format but format_weeks::update_end_date() will check it. + format_weeks::update_end_date($event->courseid); } } } diff --git a/course/format/weeks/lib.php b/course/format/weeks/lib.php index 8789fbf2ad6..9031c13645c 100644 --- a/course/format/weeks/lib.php +++ b/course/format/weeks/lib.php @@ -504,25 +504,60 @@ class format_weeks extends format_base { } /** - * Updates the end date for a course. + * Updates the end date for a course in weeks format if option automaticenddate is set. + * + * This method is called from event observers and it can not use any modinfo or format caches because + * events are triggered before the caches are reset. + * + * @param int $courseid */ - public function update_end_date() { - global $DB; + public static function update_end_date($courseid) { + global $DB, $COURSE; + + // Use one DB query to retrieve necessary fields in course, value for automaticenddate and number of the last + // section. This query will also validate that the course is indeed in 'weeks' format. + $sql = "SELECT c.id, c.format, c.startdate, c.enddate, fo.value AS automaticenddate, MAX(s.section) AS lastsection + FROM {course} c + LEFT JOIN {course_format_options} fo + ON fo.courseid = c.id + AND fo.format = c.format + AND fo.name = :optionname + AND fo.sectionid = 0 + LEFT JOIN {course_sections} s + ON s.course = c.id + WHERE c.format = :format + AND c.id = :courseid + GROUP BY c.id, c.format, c.startdate, c.enddate, fo.value"; + $course = $DB->get_record_sql($sql, + ['optionname' => 'automaticenddate', 'format' => 'weeks', 'courseid' => $courseid]); + + if (!$course) { + // Looks like it is a course in a different format, nothing to do here. + return; + } - $options = $this->get_format_options(); - // Check that the course format for setting an automatic date is set. - if (!empty($options['automaticenddate'])) { - // Now, check how many sections for this course were created. - $numsections = $DB->count_records('course_sections', array('course' => $this->get_courseid())); + // Create an instance of this class and mock the course object. + $format = new format_weeks('weeks', $courseid); + $format->course = $course; - // The first section is not a week related section, it is the 'General' section which can not be deleted. - $numsections--; + // If automaticenddate is not specified take the default value. + if (!isset($course->automaticenddate)) { + $defaults = $format->course_format_options(); + $course->automaticenddate = $defaults['automaticenddate']; + } + // Check that the course format for setting an automatic date is set. + if (!empty($course->automaticenddate)) { // Get the final week's last day. - $dates = $this->get_section_dates($numsections); + $dates = $format->get_section_dates((int)$course->lastsection); // Set the course end date. - $DB->set_field('course', 'enddate', $dates->end, array('id' => $this->get_courseid())); + if ($course->enddate != $dates->end) { + $DB->set_field('course', 'enddate', $dates->end, array('id' => $course->id)); + if (isset($COURSE->id) && $COURSE->id == $courseid) { + $COURSE->enddate = $dates->end; + } + } } } } diff --git a/course/format/weeks/tests/observer_test.php b/course/format/weeks/tests/observer_test.php index 2bf8454c387..d0ebf9ceafa 100644 --- a/course/format/weeks/tests/observer_test.php +++ b/course/format/weeks/tests/observer_test.php @@ -56,16 +56,16 @@ class format_weeks_observer_testcase extends advanced_testcase { 'automaticenddate' => 1)); // Ok, let's update the course start date. - $course->startdate = $startdate + WEEKSECS; - update_course($course); + $newstartdate = $startdate + WEEKSECS; + update_course((object)['id' => $course->id, 'startdate' => $newstartdate]); // Get the updated course end date. $enddate = $DB->get_field('course', 'enddate', array('id' => $course->id)); $format = course_get_format($course->id); - $dates = $format->get_section_dates($numsections, $course->startdate); - - // Confirm the end date is the number of weeks ahead of the start date. + $this->assertEquals($numsections, $format->get_last_section_number()); + $this->assertEquals($newstartdate, $format->get_course()->startdate); + $dates = $format->get_section_dates($numsections); $this->assertEquals($dates->end, $enddate); } @@ -86,7 +86,7 @@ class format_weeks_observer_testcase extends advanced_testcase { $createenddate = $DB->get_field('course', 'enddate', array('id' => $course->id)); // Ok, let's update the course - but actually not change anything. - update_course($course); + update_course((object)['id' => $course->id]); // Get the updated course end date. $updateenddate = $DB->get_field('course', 'enddate', array('id' => $course->id)); @@ -112,8 +112,8 @@ class format_weeks_observer_testcase extends advanced_testcase { 'automaticenddate' => 0)); // Ok, let's update the course start date. - $course->startdate = $startdate + WEEKSECS; - update_course($course); + $newstartdate = $startdate + WEEKSECS; + update_course((object)['id' => $course->id, 'startdate' => $newstartdate]); // Get the updated course end date. $updateenddate = $DB->get_field('course', 'enddate', array('id' => $course->id)); @@ -142,7 +142,7 @@ class format_weeks_observer_testcase extends advanced_testcase { $enddate = $DB->get_field('course', 'enddate', array('id' => $course->id)); $format = course_get_format($course->id); - $dates = $format->get_section_dates($numsections + 1, $course->startdate); + $dates = $format->get_section_dates($numsections + 1); // Confirm end date was updated. $this->assertEquals($enddate, $dates->end); @@ -169,7 +169,7 @@ class format_weeks_observer_testcase extends advanced_testcase { $enddate = $DB->get_field('course', 'enddate', array('id' => $course->id)); $format = course_get_format($course->id); - $dates = $format->get_section_dates($numsections - 1, $course->startdate); + $dates = $format->get_section_dates($numsections - 1); // Confirm end date was updated. $this->assertEquals($enddate, $dates->end); -- 2.17.1