From 9c98546da64ea9a2b8ce394cc847be152165e84f Mon Sep 17 00:00:00 2001 From: Cameron Ball Date: Tue, 2 May 2017 14:45:58 +0800 Subject: [PATCH 1/6] MDL-58777 core: Use component_callback in refresh events task Previously we did not require the lib.php files for each module in this ad-hoc task, which meant that the *_refresh_events functions would only be called if some lucky sequence of events led to the lib.php file for the module being required. Instead we should use the component_callback function which can handle that stuff automagically. --- lib/classes/task/refresh_mod_calendar_events_task.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/classes/task/refresh_mod_calendar_events_task.php b/lib/classes/task/refresh_mod_calendar_events_task.php index ca5a63ee25d..34dd6d28f30 100644 --- a/lib/classes/task/refresh_mod_calendar_events_task.php +++ b/lib/classes/task/refresh_mod_calendar_events_task.php @@ -59,10 +59,9 @@ class refresh_mod_calendar_events_task extends adhoc_task { continue; } // Check if the plugin implements *_refresh_events() and call it when it does. - $refresheventsfunction = $plugin->name . '_refresh_events'; - if (function_exists($refresheventsfunction)) { - mtrace('Calling ' . $refresheventsfunction); - call_user_func($refresheventsfunction); + if (component_callback_exists('mod_' . $plugin->name, 'refresh_events')) { + mtrace('Refreshing events for ' . $plugin->name); + component_callback('mod_' . $plugin->name, 'refresh_events'); } } } From 4133df6fd63cd05852f7a52c59b753118135a8d4 Mon Sep 17 00:00:00 2001 From: Cameron Ball Date: Fri, 5 May 2017 16:42:12 +0800 Subject: [PATCH 2/6] MDL-58777 mod_quiz: Explicitly sort records and set event priority The quiz update events code depends on the "old" events in the DB being returned in the same order as they were originally made, however there was no guarantee that this would be the case. There were also situations where the priority would not be explicitly set (e.g., when creating the "original" event). --- mod/quiz/lib.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index 7a9300a2d99..de6d1de0def 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -1205,14 +1205,17 @@ function quiz_update_events($quiz, $override = null) { $conds['groupid'] = $override->groupid; } } - $oldevents = $DB->get_records('event', $conds); + $oldevents = $DB->get_records('event', $conds, 'id ASC'); // Now make a to-do list of all that needs to be updated. if (empty($override)) { - // We are updating the primary settings for the lesson, so we need to add all the overrides. - $overrides = $DB->get_records('quiz_overrides', array('quiz' => $quiz->id)); - // As well as the original quiz (empty override). - $overrides[] = new stdClass(); + // We are updating the primary settings for the quiz, so we need to add all the overrides. + $overrides = $DB->get_records('quiz_overrides', array('quiz' => $quiz->id), 'id ASC'); + // It is necessary to add an empty stdClass to the beginning of the array as the $oldevents + // list contains the original (non-override) event for the module. If this is not included + // the logic below will end up updating the wrong row when we try to reconcile this $overrides + // list against the $oldevents list. + array_unshift($overrides, new stdClass()); } else { // Just do the one override. $overrides = array($override); @@ -1251,6 +1254,7 @@ function quiz_update_events($quiz, $override = null) { $event->timesort = $timeopen; $event->visible = instance_is_visible('quiz', $quiz); $event->eventtype = QUIZ_EVENT_TYPE_OPEN; + $event->priority = null; // Determine the event name and priority. if ($groupid) { From 8dada7e334af76f2a9dc529ec3e07c55e47a86d2 Mon Sep 17 00:00:00 2001 From: Cameron Ball Date: Fri, 5 May 2017 16:44:52 +0800 Subject: [PATCH 3/6] MDL-58777 mod_lesson: Explicitly sort records and set event priority The lesson update events code depends on the "old" events in the DB being returned in the same order as they were originally made, however there was no guarantee that this would be the case. There were also situations where the priority would not be explicitly set (e.g., when creating the "original" event). --- mod/lesson/lib.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/mod/lesson/lib.php b/mod/lesson/lib.php index 7a6ab2500e8..e838f5d3be3 100644 --- a/mod/lesson/lib.php +++ b/mod/lesson/lib.php @@ -121,14 +121,17 @@ function lesson_update_events($lesson, $override = null) { $conds['groupid'] = $override->groupid; } } - $oldevents = $DB->get_records('event', $conds); + $oldevents = $DB->get_records('event', $conds, 'id ASC'); // Now make a to-do list of all that needs to be updated. if (empty($override)) { // We are updating the primary settings for the lesson, so we need to add all the overrides. - $overrides = $DB->get_records('lesson_overrides', array('lessonid' => $lesson->id)); - // As well as the original lesson (empty override). - $overrides[] = new stdClass(); + $overrides = $DB->get_records('lesson_overrides', array('lessonid' => $lesson->id), 'id ASC'); + // It is necessary to add an empty stdClass to the beginning of the array as the $oldevents + // list contains the original (non-override) event for the module. If this is not included + // the logic below will end up updating the wrong row when we try to reconcile this $overrides + // list against the $oldevents list. + array_unshift($overrides, new stdClass()); } else { // Just do the one override. $overrides = array($override); @@ -167,6 +170,7 @@ function lesson_update_events($lesson, $override = null) { $event->timesort = $available; $event->visible = instance_is_visible('lesson', $lesson); $event->eventtype = LESSON_EVENT_TYPE_OPEN; + $event->priority = null; // Determine the event name and priority. if ($groupid) { From 56d1ddd775a81b0527316786d4809689c964afd2 Mon Sep 17 00:00:00 2001 From: Cameron Ball Date: Fri, 5 May 2017 16:46:22 +0800 Subject: [PATCH 4/6] MDL-58777 mod_assign: Explicitly sort records and set event priority The assign update events code depends on the "old" events in the DB being returned in the same order as they were originally made, however there was no guarantee that this would be the case. There were also situations where the priority would not be explicitly set (e.g., when creating the "original" event). --- mod/assign/lib.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/mod/assign/lib.php b/mod/assign/lib.php index 30c472c9ff0..860f5ad3624 100644 --- a/mod/assign/lib.php +++ b/mod/assign/lib.php @@ -231,14 +231,17 @@ function assign_update_events($assign, $override = null) { $conds['groupid'] = $override->groupid; } } - $oldevents = $DB->get_records('event', $conds); + $oldevents = $DB->get_records('event', $conds, 'id ASC'); // Now make a to-do list of all that needs to be updated. if (empty($override)) { - // We are updating the primary settings for the assign, so we need to add all the overrides. - $overrides = $DB->get_records('assign_overrides', array('assignid' => $assigninstance->id)); - // As well as the original assign (empty override). - $overrides[] = new stdClass(); + // We are updating the primary settings for the assignment, so we need to add all the overrides. + $overrides = $DB->get_records('assign_overrides', array('assignid' => $assigninstance->id), 'id ASC'); + // It is necessary to add an empty stdClass to the beginning of the array as the $oldevents + // list contains the original (non-override) event for the module. If this is not included + // the logic below will end up updating the wrong row when we try to reconcile this $overrides + // list against the $oldevents list. + array_unshift($overrides, new stdClass()); } else { // Just do the one override. $overrides = array($override); @@ -272,6 +275,7 @@ function assign_update_events($assign, $override = null) { $event->timesort = $event->timestart + $event->timeduration; $event->visible = instance_is_visible('assign', $assigninstance); $event->eventtype = ASSIGN_EVENT_TYPE_DUE; + $event->priority = null; // Determine the event name and priority. if ($groupid) { From 9eb5a0417c00701246d277f7bddbd49f735ca904 Mon Sep 17 00:00:00 2001 From: Cameron Ball Date: Mon, 8 May 2017 11:37:12 +0800 Subject: [PATCH 5/6] MDL-58777 mod_assign: Remove unnecessary upgrade step This upgrade step was setting the priority field for assign events in the events table. This is redundant since we queue a task to refresh all events. This patch also removes a redundant refresh_events task that is queued from assign. --- mod/assign/db/upgrade.php | 41 --------------------------------------- 1 file changed, 41 deletions(-) diff --git a/mod/assign/db/upgrade.php b/mod/assign/db/upgrade.php index 5afc8889b68..bc8f955fdff 100644 --- a/mod/assign/db/upgrade.php +++ b/mod/assign/db/upgrade.php @@ -253,35 +253,6 @@ function xmldb_assign_upgrade($oldversion) { upgrade_mod_savepoint(true, 2017021500, 'assign'); } - if ($oldversion < 2017031000) { - // Set priority of assign user overrides. - $params = [ - 'modulename' => 'assign', - 'courseid' => 0, - 'groupid' => 0, - 'repeatid' => 0 - ]; - // CALENDAR_EVENT_USER_OVERRIDE_PRIORITY has a value of 9999999. - $DB->set_field('event', 'priority', 9999999, $params); - - // Set priority for group overrides for existing assign events. - $where = 'groupid IS NOT NULL'; - $assignoverridesrs = $DB->get_recordset_select('assign_overrides', $where, null, '', 'id, assignid, groupid, sortorder'); - foreach ($assignoverridesrs as $record) { - $params = [ - 'modulename' => 'assign', - 'instance' => $record->assignid, - 'groupid' => $record->groupid, - 'repeatid' => 0 - ]; - $DB->set_field('event', 'priority', $record->sortorder, $params); - } - $assignoverridesrs->close(); - - // Assign savepoint reached. - upgrade_mod_savepoint(true, 2017031000, 'assign'); - } - if ($oldversion < 2017031300) { // Add a 'gradingduedate' field to the 'assign' table. $table = new xmldb_table('assign'); @@ -318,18 +289,6 @@ function xmldb_assign_upgrade($oldversion) { // Execute DB update for assign instances. $DB->execute($sql, $params); - // Create adhoc task for upgrading of existing mod_assign calendar events. - $task = new \stdClass(); - $task->classname = "\\core\\task\\refresh_mod_calendar_events_task"; - $task->component = 'core'; - - // Next run time based from nextruntime computation in \core\task\manager::queue_adhoc_task(). - $nextruntime = time() - 1; - $task->nextruntime = $nextruntime; - // Indicate to the adhoc task that only the assignment module will be refreshed. - $task->customdata = json_encode(['plugins' => ['assign']]); - $DB->insert_record('task_adhoc', $task); - // Assign savepoint reached. upgrade_mod_savepoint(true, 2017042800, 'assign'); } From 79b80ee540bba0facd13e9d5a0e81d87f740c7c8 Mon Sep 17 00:00:00 2001 From: Cameron Ball Date: Tue, 2 May 2017 14:51:57 +0800 Subject: [PATCH 6/6] MDL-58777 core: Remove redundant refresh events tasks There were 4 instances of the calendar refresh events task being queued, which is redundant as we now have the "one true task" to fix the events table, and it needs to be run for everybody. --- lib/db/upgrade.php | 62 +++++++++++----------------------------------- version.php | 2 +- 2 files changed, 16 insertions(+), 48 deletions(-) diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 8e08d74eae1..21b4ebe58ab 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2566,19 +2566,6 @@ function xmldb_main_upgrade($oldversion) { $dbman->add_field($table, $field); } - // Create adhoc task for upgrading of existing calendar events. - $record = new \stdClass(); - $record->classname = "\\core\\task\\refresh_mod_calendar_events_task"; - $record->component = 'core'; - // Next run time based from nextruntime computation in \core\task\manager::queue_adhoc_task(). - $nextruntime = time() - 1; - $record->nextruntime = $nextruntime; - $DB->insert_record('task_adhoc', $record); - - // This same task is queued again in a later step, but if we already queue it here - // then there is no need to queue it again. We use this flag in the second step. - $refresheventsadhocadded = true; - // Main savepoint reached. upgrade_main_savepoint(true, 2017030700.00); } @@ -2671,21 +2658,6 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2017040402.00); } - if ($oldversion < 2017040403.00) { - // Create adhoc task for upgrading of existing calendar events. - $record = new \stdClass(); - $record->classname = "\\core\\task\\refresh_mod_calendar_events_task"; - $record->component = 'core'; - - // Next run time based from nextruntime computation in \core\task\manager::queue_adhoc_task(). - $nextruntime = time() - 1; - $record->nextruntime = $nextruntime; - $DB->insert_record('task_adhoc', $record); - - // Main savepoint reached. - upgrade_main_savepoint(true, 2017040403.00); - } - if ($oldversion < 2017040700.01) { // Define table oauth2_issuer to be created. @@ -2838,25 +2810,6 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2017041801.00); } - if ($oldversion < 2017042600.01) { - // If the previous step didn't execute and queue the task. - if (!isset($refresheventsadhocadded)) { - // Create adhoc task for upgrading of existing calendar events. - $record = new \stdClass(); - $record->classname = "\\core\\task\\refresh_mod_calendar_events_task"; - $record->component = 'core'; - - // Next run time based from nextruntime computation in \core\task\manager::queue_adhoc_task(). - $nextruntime = time() - 1; - $record->nextruntime = $nextruntime; - $DB->insert_record('task_adhoc', $record); - - } - - // Main savepoint reached. - upgrade_main_savepoint(true, 2017042600.01); - } - if ($oldversion < 2017050500.01) { // Get the list of parent event IDs. $sql = "SELECT DISTINCT repeatid @@ -2894,5 +2847,20 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2017050500.02); } + if ($oldversion < 2017050900.01) { + // Create adhoc task for upgrading of existing calendar events. + $record = new \stdClass(); + $record->classname = '\core\task\refresh_mod_calendar_events_task'; + $record->component = 'core'; + + // Next run time based from nextruntime computation in \core\task\manager::queue_adhoc_task(). + $nextruntime = time() - 1; + $record->nextruntime = $nextruntime; + $DB->insert_record('task_adhoc', $record); + + // Main savepoint reached. + upgrade_main_savepoint(true, 2017050900.01); + } + return true; } diff --git a/version.php b/version.php index 0b7efa7f9c7..4f31d564ec5 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2017050900.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2017050900.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.