From 031cf251c11ed6ee3017d151a38df61958729636 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Thu, 21 Feb 2019 16:21:58 +0800 Subject: [PATCH 1/2] MDL-64878 grades: Rebuild cache on import When the course module cache is out of date during a gradebook re-calculcation it throws throws exceptions because the module cannot be found. This prevents access to gradebook or any type of grading functions until the cache is rebuilt. When the cache still has no module record we log an error and return the course context. --- backup/moodle2/restore_stepslib.php | 3 +++ lib/grade/grade_item.php | 18 ++++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index b3d5322f70d..149f606ce46 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -485,6 +485,9 @@ class restore_gradebook_structure_step extends restore_structure_step { // Freeze gradebook calculations if needed. $this->gradebook_calculation_freeze(); + // Ensure the module cache is current when recalculating grades. + rebuild_course_cache($this->get_courseid(), true); + // Restore marks items as needing update. Update everything now. grade_regrade_final_grades($this->get_courseid()); } diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php index 60edaf60b92..769f69dfd3c 100644 --- a/lib/grade/grade_item.php +++ b/lib/grade/grade_item.php @@ -2495,8 +2495,22 @@ class grade_item extends grade_object { */ public function get_context() { if ($this->itemtype == 'mod') { - $cm = get_fast_modinfo($this->courseid)->instances[$this->itemmodule][$this->iteminstance]; - $context = \context_module::instance($cm->id); + $modinfo = get_fast_modinfo($this->courseid); + // Sometimes the course module cache is out of date and needs to be rebuilt. + if (!isset($modinfo->instances[$this->itemmodule][$this->iteminstance])) { + rebuild_course_cache($this->courseid, true); + $modinfo = get_fast_modinfo($this->courseid); + } + // Even with a rebuilt cache the module does not exist. This means the + // database is in an invalid state - we will log an error and return + // the course context but the calling code should be updated. + if (!isset($modinfo->instances[$this->itemmodule][$this->iteminstance])) { + mtrace(get_string('moduleinstancedoesnotexist', 'error')); + $context = \context_course::instance($this->courseid); + } else { + $cm = $modinfo->instances[$this->itemmodule][$this->iteminstance]; + $context = \context_module::instance($cm->id); + } } else { $context = \context_course::instance($this->courseid); } From 40fe1538160d61765ac98ad54bf88491e8e00bcf Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Wed, 27 Feb 2019 10:35:42 +0800 Subject: [PATCH 2/2] MDL-64878 grades: Update delete_instance When grades are deleted, it needs to know the context so any files in the gradebook can be deleted. This means module delete_instance functions must delete the grade_item before they delete the module record. --- mod/assign/locallib.php | 1 + mod/assignment/lib.php | 5 +++-- mod/data/lib.php | 7 ++++--- mod/forum/lib.php | 5 +++-- mod/lesson/locallib.php | 4 +++- mod/lti/lib.php | 1 + mod/quiz/lib.php | 1 + mod/scorm/lib.php | 6 ++++-- mod/workshop/lib.php | 7 ++++--- 9 files changed, 24 insertions(+), 13 deletions(-) diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index cd9f4c11fbc..a3302e02bdc 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -813,6 +813,7 @@ class assign { } // Delete the instance. + // We must delete the module record after we delete the grade item. $DB->delete_records('assign', array('id'=>$this->get_instance()->id)); return $result; diff --git a/mod/assignment/lib.php b/mod/assignment/lib.php index 9b125863bc7..40917e88372 100644 --- a/mod/assignment/lib.php +++ b/mod/assignment/lib.php @@ -71,12 +71,13 @@ function assignment_delete_instance($id){ $result = false; } + grade_update('mod/assignment', $assignment->course, 'mod', 'assignment', $assignment->id, 0, NULL, array('deleted'=>1)); + + // We must delete the module record after we delete the grade item. if (! $DB->delete_records('assignment', array('id'=>$assignment->id))) { $result = false; } - grade_update('mod/assignment', $assignment->course, 'mod', 'assignment', $assignment->id, 0, NULL, array('deleted'=>1)); - return $result; } diff --git a/mod/data/lib.php b/mod/data/lib.php index 0fe8e85ac26..eace6e75fb8 100644 --- a/mod/data/lib.php +++ b/mod/data/lib.php @@ -1153,12 +1153,13 @@ function data_delete_instance($id) { // takes the dataid $event->delete(); } - // Delete the instance itself - $result = $DB->delete_records('data', array('id'=>$id)); - // cleanup gradebook data_grade_item_delete($data); + // Delete the instance itself + // We must delete the module record after we delete the grade item. + $result = $DB->delete_records('data', array('id'=>$id)); + return $result; } diff --git a/mod/forum/lib.php b/mod/forum/lib.php index 830e051a4f9..80c06f28719 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -305,12 +305,13 @@ function forum_delete_instance($id) { forum_tp_delete_read_records(-1, -1, -1, $forum->id); + forum_grade_item_delete($forum); + + // We must delete the module record after we delete the grade item. if (!$DB->delete_records('forum', array('id'=>$forum->id))) { $result = false; } - forum_grade_item_delete($forum); - return $result; } diff --git a/mod/lesson/locallib.php b/mod/lesson/locallib.php index 5cb65daf753..4d55cb4465b 100644 --- a/mod/lesson/locallib.php +++ b/mod/lesson/locallib.php @@ -1642,6 +1642,9 @@ class lesson extends lesson_base { $this->delete_all_overrides(); + grade_update('mod/lesson', $this->properties->course, 'mod', 'lesson', $this->properties->id, 0, null, array('deleted'=>1)); + + // We must delete the module record after we delete the grade item. $DB->delete_records("lesson", array("id"=>$this->properties->id)); $DB->delete_records("lesson_pages", array("lessonid"=>$this->properties->id)); $DB->delete_records("lesson_answers", array("lessonid"=>$this->properties->id)); @@ -1662,7 +1665,6 @@ class lesson extends lesson_base { $fs = get_file_storage(); $fs->delete_area_files($context->id); - grade_update('mod/lesson', $this->properties->course, 'mod', 'lesson', $this->properties->id, 0, null, array('deleted'=>1)); return true; } diff --git a/mod/lti/lib.php b/mod/lti/lib.php index 886519338f5..5becf387a9d 100644 --- a/mod/lti/lib.php +++ b/mod/lti/lib.php @@ -200,6 +200,7 @@ function lti_delete_instance($id) { $cm = get_coursemodule_from_instance('lti', $id); \core_completion\api::update_completion_date_event($cm->id, 'lti', $id, null); + // We must delete the module record after we delete the grade item. return $DB->delete_records("lti", array("id" => $basiclti->id)); } diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index 3ffd67e8e8c..d4843a08cd2 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -203,6 +203,7 @@ function quiz_delete_instance($id) { } quiz_grade_item_delete($quiz); + // We must delete the module record after we delete the grade item. $DB->delete_records('quiz', array('id' => $quiz->id)); return true; diff --git a/mod/scorm/lib.php b/mod/scorm/lib.php index 234a33e7871..ae9672e885e 100644 --- a/mod/scorm/lib.php +++ b/mod/scorm/lib.php @@ -301,6 +301,10 @@ function scorm_delete_instance($id) { } $DB->delete_records('scorm_scoes', array('scorm' => $scorm->id)); } + + scorm_grade_item_delete($scorm); + + // We must delete the module record after we delete the grade item. if (! $DB->delete_records('scorm', array('id' => $scorm->id))) { $result = false; } @@ -327,8 +331,6 @@ function scorm_delete_instance($id) { $result = false; }*/ - scorm_grade_item_delete($scorm); - return $result; } diff --git a/mod/workshop/lib.php b/mod/workshop/lib.php index 8cdf52b3c05..73d05e2e40e 100644 --- a/mod/workshop/lib.php +++ b/mod/workshop/lib.php @@ -290,13 +290,14 @@ function workshop_delete_instance($id) { $event->delete(); } - // finally remove the workshop record itself - $DB->delete_records('workshop', array('id' => $workshop->id)); - // gradebook cleanup grade_update('mod/workshop', $workshop->course, 'mod', 'workshop', $workshop->id, 0, null, array('deleted' => true)); grade_update('mod/workshop', $workshop->course, 'mod', 'workshop', $workshop->id, 1, null, array('deleted' => true)); + // finally remove the workshop record itself + // We must delete the module record after we delete the grade item. + $DB->delete_records('workshop', array('id' => $workshop->id)); + return true; }