From 1a3faa11b778d31a5f95cb8fe3e5139372a51c97 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 859198ea0e0..84bf0de8e8b 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 7cd7f202020acf12edbde0f36b3e379e7d1b5df8 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 90160f60799..1dfeb6f0053 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -783,6 +783,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 7b59ce86671..95d53dd196b 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 5b847da7294..755767a7e26 100644 --- a/mod/lesson/locallib.php +++ b/mod/lesson/locallib.php @@ -1637,6 +1637,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)); @@ -1657,7 +1660,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; }