MDL-56251 format_weeks: do not use caches in event observers

diff --git a/course/format/weeks/classes/observer.php b/course/format/weeks/classes/observer.php
index d359580..7be6b63 100644 (file)
--- 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 8789fbf..9031c13 100644 (file)
--- 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 2bf8454..d0ebf9c 100644 (file)
--- 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);