From 048f909b0774a31bcd9d8b3484d5c7d746ad93de Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Wed, 2 Nov 2016 15:05:56 +0800 Subject: [PATCH 1/8] MDL-54751 core: introduce deletion flag for course modules Created a flag, 'deletioninprogress' on the course_modules table to be used when deleting course modules and course sections. Modified modinfolib caches such that, when rebuilt, these caches hide the relevant modules based on the value of the flag. --- lib/db/install.xml | 1 + lib/db/upgrade.php | 14 ++++++++++++++ lib/modinfolib.php | 21 +++++++++++++++++---- version.php | 2 +- 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/lib/db/install.xml b/lib/db/install.xml index af9e2dcd8b1..39bbd58abc4 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -300,6 +300,7 @@ + diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 7a2adeb3f8d..f7e6a77b669 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2322,5 +2322,19 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2016110300.00); } + if ($oldversion < 2016110400.02) { + // Define a field 'deletioninprogress' in the 'course_modules' table, to background deletion tasks. + $table = new xmldb_table('course_modules'); + $field = new xmldb_field('deletioninprogress', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0', 'availability'); + + // Conditionally launch add field 'deletioninprogress'. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Main savepoint reached. + upgrade_main_savepoint(true, 2016110400.02); + } + return true; } diff --git a/lib/modinfolib.php b/lib/modinfolib.php index d8d3c1ec484..a3664589392 100644 --- a/lib/modinfolib.php +++ b/lib/modinfolib.php @@ -554,8 +554,7 @@ class course_modinfo { // Get section data $sections = $DB->get_records('course_sections', array('course' => $course->id), 'section', - 'section, id, course, name, summary, summaryformat, sequence, visible, ' . - 'availability'); + 'section, id, course, name, summary, summaryformat, sequence, visible, availability'); $compressedsections = array(); $formatoptionsdef = course_get_format($course)->section_format_options(); @@ -753,6 +752,7 @@ class course_modinfo { * @property-read mixed $customdata Optional custom data stored in modinfo cache for this activity, or null if none * @property-read string $afterlink Extra HTML code to display after link - calculated on request * @property-read string $afterediticons Extra HTML code to display after editing icons (e.g. more icons) - calculated on request + * @property-read bool $deletioninprogress True if this course module is scheduled for deletion, false otherwise. */ class cm_info implements IteratorAggregate { /** @@ -1038,6 +1038,11 @@ class cm_info implements IteratorAggregate { */ private $afterediticons; + /** + * @var bool representing the deletion state of the module. True if the mod is scheduled for deletion. + */ + private $deletioninprogress; + /** * List of class read-only properties and their getter methods. * Used by magic functions __get(), __isset(), __empty() @@ -1089,6 +1094,7 @@ class cm_info implements IteratorAggregate { 'uservisible' => 'get_user_visible', 'visible' => false, 'visibleold' => false, + 'deletioninprogress' => false ); /** @@ -1505,7 +1511,7 @@ class cm_info implements IteratorAggregate { static $cmfields = array('id', 'course', 'module', 'instance', 'section', 'idnumber', 'added', 'score', 'indent', 'visible', 'visibleold', 'groupmode', 'groupingid', 'completion', 'completiongradeitemnumber', 'completionview', 'completionexpected', - 'showdescription', 'availability'); + 'showdescription', 'availability', 'deletioninprogress'); foreach ($cmfields as $key) { $cmrecord->$key = $this->$key; } @@ -1700,6 +1706,7 @@ class cm_info implements IteratorAggregate { $this->added = isset($mod->added) ? $mod->added : 0; $this->score = isset($mod->score) ? $mod->score : 0; $this->visibleold = isset($mod->visibleold) ? $mod->visibleold : 0; + $this->deletioninprogress = isset($mod->deletioninprogress) ? $mod->deletioninprogress : 0; // Note: it saves effort and database space to always include the // availability and completion fields, even if availability or completion @@ -1861,6 +1868,12 @@ class cm_info implements IteratorAggregate { } $this->uservisible = true; + // If the module is being deleted, set the uservisible state to false and return. + if ($this->deletioninprogress) { + $this->uservisible = false; + return null; + } + // If the user cannot access the activity set the uservisible flag to false. // Additional checks are required to determine whether the activity is entirely hidden or just greyed out. if ((!$this->visible or !$this->get_available()) and @@ -2465,7 +2478,7 @@ class section_info implements IteratorAggregate { 'summary' => '', 'summaryformat' => '1', // FORMAT_HTML, but must be a string 'visible' => '1', - 'availability' => null, + 'availability' => null ); /** diff --git a/version.php b/version.php index 84c1ef847e5..39211165821 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2016110400.01; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2016110400.02; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. From 37da62f3600144be33d261b67169aad172407aaf Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Wed, 2 Nov 2016 15:06:40 +0800 Subject: [PATCH 2/8] MDL-54751 core_course: Create adhoc task for course module deletion --- course/classes/task/course_delete_modules.php | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 course/classes/task/course_delete_modules.php diff --git a/course/classes/task/course_delete_modules.php b/course/classes/task/course_delete_modules.php new file mode 100644 index 00000000000..3770537451a --- /dev/null +++ b/course/classes/task/course_delete_modules.php @@ -0,0 +1,62 @@ +. + +/** + * Adhoc task handling course module deletion. + * + * @package core_course + * @copyright 2016 Jake Dallimore + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core_course\task; + +defined('MOODLE_INTERNAL') || die(); +/** + * Class handling course module deletion. + * + * This task supports an array of course module object as custom_data, and calls course_delete_module() in synchronous deletion + * mode for each of them. + * This will: + * 1. call any 'mod_xxx_pre_course_module_deleted' functions (e.g. Recycle bin) + * 2. delete the module + * 3. fire the deletion event + * + * @package core_course + * @copyright 2016 Jake Dallimore + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class course_delete_modules extends \core\task\adhoc_task { + + /** + * Run the deletion task. + * + * @throws \coding_exception if the module could not be removed. + */ + public function execute() { + global $CFG; + require_once($CFG->dirroot. '/course/lib.php'); + + $cms = $this->get_custom_data()->cms; + foreach ($cms as $cm) { + try { + course_delete_module($cm->id); + } catch (\Exception $e) { + throw new \coding_exception("The course module {$cm->id} could not be deleted. $e->getTraceAsString()"); + } + } + } +} From 3869d774113fdcf9475c27107d4d93d010d3606f Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Wed, 2 Nov 2016 15:07:55 +0800 Subject: [PATCH 3/8] MDL-54751 core_course: Edit course libs to support deletion flag Changed the course libs such that module and section deletion is now handed off to relevant functions in the util class and exempted modules scheduled for deletion from the course backups. --- admin/tool/recyclebin/lib.php | 11 + backup/util/dbops/backup_plan_dbops.class.php | 3 +- course/editsection.php | 2 +- course/lib.php | 235 +++++++++++++++++- course/rest.php | 2 +- lib/filestorage/file_storage.php | 7 +- 6 files changed, 244 insertions(+), 16 deletions(-) diff --git a/admin/tool/recyclebin/lib.php b/admin/tool/recyclebin/lib.php index ee2eb748633..d4e748bd1b6 100644 --- a/admin/tool/recyclebin/lib.php +++ b/admin/tool/recyclebin/lib.php @@ -152,6 +152,17 @@ function tool_recyclebin_pre_course_module_delete($cm) { } } +/** + * Hook called to check whether async course module deletion should be performed or not. + * + * @return true if background deletion is required (is the recyclebin is enabled), false otherwise. + */ +function tool_recyclebin_course_module_background_deletion_recommended() { + if (\tool_recyclebin\course_bin::is_enabled()) { + return true; + } +} + /** * Hook called before we delete a course. * diff --git a/backup/util/dbops/backup_plan_dbops.class.php b/backup/util/dbops/backup_plan_dbops.class.php index 530763752be..615f802fc6c 100644 --- a/backup/util/dbops/backup_plan_dbops.class.php +++ b/backup/util/dbops/backup_plan_dbops.class.php @@ -87,7 +87,8 @@ abstract class backup_plan_dbops extends backup_dbops { FROM {course_modules} cm JOIN {modules} m ON m.id = cm.module WHERE cm.course = ? - AND cm.section = ?", array($courseid, $sectionid)); + AND cm.section = ? + AND cm.deletioninprogress <> 1", array($courseid, $sectionid)); foreach (explode(',', $sequence) as $moduleid) { if (isset($modules[$moduleid])) { $module = array('id' => $modules[$moduleid]->id, 'modname' => $modules[$moduleid]->modname); diff --git a/course/editsection.php b/course/editsection.php index 14b74932488..231935f31be 100644 --- a/course/editsection.php +++ b/course/editsection.php @@ -50,7 +50,7 @@ if ($deletesection) { if (course_can_delete_section($course, $sectioninfo)) { $confirm = optional_param('confirm', false, PARAM_BOOL) && confirm_sesskey(); if ($confirm) { - course_delete_section($course, $sectioninfo, true); + course_delete_section($course, $sectioninfo, true, true); $courseurl = course_get_url($course, 0, array('sr' => $sectionreturn)); redirect($courseurl); } else { diff --git a/course/lib.php b/course/lib.php index 553632602e9..353845d3c99 100644 --- a/course/lib.php +++ b/course/lib.php @@ -430,6 +430,7 @@ function get_array_of_activities($courseid) { $mod[$seq]->completionexpected = $rawmods[$seq]->completionexpected; $mod[$seq]->showdescription = $rawmods[$seq]->showdescription; $mod[$seq]->availability = $rawmods[$seq]->availability; + $mod[$seq]->deletioninprogress = $rawmods[$seq]->deletioninprogress; $modname = $mod[$seq]->mod; $functionname = $modname."_get_coursemodule_info"; @@ -504,7 +505,7 @@ function get_array_of_activities($courseid) { foreach (array('idnumber', 'groupmode', 'groupingid', 'indent', 'completion', 'extra', 'extraclasses', 'iconurl', 'onclick', 'content', 'icon', 'iconcomponent', 'customdata', 'availability', 'completionview', - 'completionexpected', 'score', 'showdescription') as $property) { + 'completionexpected', 'score', 'showdescription', 'deletioninprogress') as $property) { if (property_exists($mod[$seq], $property) && empty($mod[$seq]->{$property})) { unset($mod[$seq]->{$property}); @@ -1072,9 +1073,25 @@ function set_coursemodule_name($id, $name) { * event to the DB. * * @param int $cmid the course module id + * @param bool $async whether or not to try to delete the module using an adhoc task. Async also depends on a plugin hook. + * @throws moodle_exception * @since Moodle 2.5 */ -function course_delete_module($cmid) { +function course_delete_module($cmid, $async = false) { + // Check the 'course_module_background_deletion_recommended' hook first. + // Only use asynchronous deletion if at least one plugin returns true and if async deletion has been requested. + // Both are checked because plugins should not be allowed to dictate the deletion behaviour, only support/decline it. + // It's up to plugins to handle things like whether or not they are enabled. + if ($async && $pluginsfunction = get_plugins_with_function('course_module_background_deletion_recommended')) { + foreach ($pluginsfunction as $plugintype => $plugins) { + foreach ($plugins as $pluginfunction) { + if ($pluginfunction()) { + return course_module_flag_for_async_deletion($cmid); + } + } + } + } + global $CFG, $DB; require_once($CFG->libdir.'/gradelib.php'); @@ -1192,6 +1209,104 @@ function course_delete_module($cmid) { rebuild_course_cache($cm->course, true); } +/** + * Schedule a course module for deletion in the background using an adhoc task. + * + * This method should not be called directly. Instead, please use course_delete_module($cmid, true), to denote async deletion. + * The real deletion of the module is handled by the task, which calls 'course_delete_module($cmid)'. + * + * @param int $cmid the course module id. + * @return bool whether the module was successfully scheduled for deletion. + * @throws \moodle_exception + */ +function course_module_flag_for_async_deletion($cmid) { + global $CFG, $DB; + require_once($CFG->libdir.'/gradelib.php'); + require_once($CFG->libdir.'/questionlib.php'); + require_once($CFG->dirroot.'/blog/lib.php'); + require_once($CFG->dirroot.'/calendar/lib.php'); + + // Get the course module. + if (!$cm = $DB->get_record('course_modules', array('id' => $cmid))) { + return true; + } + + // We need to be reasonably certain the deletion is going to succeed before we background the process. + // Make the necessary delete_instance checks, etc. before proceeding further. Throw exceptions if required. + + // Get the course module name. + $modulename = $DB->get_field('modules', 'name', array('id' => $cm->module), MUST_EXIST); + + // Get the file location of the delete_instance function for this module. + $modlib = "$CFG->dirroot/mod/$modulename/lib.php"; + + // Include the file required to call the delete_instance function for this module. + if (file_exists($modlib)) { + require_once($modlib); + } else { + throw new \moodle_exception('cannotdeletemodulemissinglib', '', '', null, + "Cannot delete this module as the file mod/$modulename/lib.php is missing."); + } + + $deleteinstancefunction = $modulename . '_delete_instance'; + + // Ensure the delete_instance function exists for this module. + if (!function_exists($deleteinstancefunction)) { + throw new \moodle_exception('cannotdeletemodulemissingfunc', '', '', null, + "Cannot delete this module as the function {$modulename}_delete_instance is missing in mod/$modulename/lib.php."); + } + + // We are going to defer the deletion as we can't be sure how long the module's pre_delete code will run for. + $cm->deletioninprogress = '1'; + $DB->update_record('course_modules', $cm); + + // Create an adhoc task for the deletion of the course module. The task takes an array of course modules for removal. + $removaltask = new \core_course\task\course_delete_modules(); + $removaltask->set_custom_data(array('cms' => array($cm))); + + // Queue the task for the next run. + \core\task\manager::queue_adhoc_task($removaltask); + + // Reset the course cache to hide the module. + rebuild_course_cache($cm->course, true); +} + +/** + * Checks whether the given course has any course modules scheduled for adhoc deletion. + * + * @param int $courseid the id of the course. + * @return bool true if the course contains any modules pending deletion, false otherwise. + */ +function course_modules_pending_deletion($courseid) { + if (empty($courseid)) { + return false; + } + $modinfo = get_fast_modinfo($courseid); + foreach ($modinfo->get_cms() as $module) { + if ($module->deletioninprogress == '1') { + return true; + } + } + return false; +} + +/** + * Checks whether the course module, as defined by modulename and instanceid, is scheduled for deletion within the given course. + * + * @param int $courseid the course id. + * @param string $modulename the module name. E.g. 'assign', 'book', etc. + * @param int $instanceid the module instance id. + * @return bool true if the course module is pending deletion, false otherwise. + */ +function course_module_instance_pending_deletion($courseid, $modulename, $instanceid) { + if (empty($courseid) || empty($modulename) || empty($instanceid)) { + return false; + } + $modinfo = get_fast_modinfo($courseid); + $instances = $modinfo->get_instances_of($modulename); + return isset($instances[$instanceid]) && $instances[$instanceid]->deletioninprogress; +} + function delete_mod_from_section($modid, $sectionid) { global $DB; @@ -1285,9 +1400,10 @@ function move_section_to($course, $section, $destination, $ignorenumsections = f * @param int|stdClass $course * @param int|stdClass|section_info $section * @param bool $forcedeleteifnotempty if set to false section will not be deleted if it has modules in it. + * @param bool $async whether or not to try to delete the section using an adhoc task. Async also depends on a plugin hook. * @return bool whether section was deleted */ -function course_delete_section($course, $section, $forcedeleteifnotempty = true) { +function course_delete_section($course, $section, $forcedeleteifnotempty = true, $async = false) { global $DB; // Prepare variables. @@ -1298,6 +1414,21 @@ function course_delete_section($course, $section, $forcedeleteifnotempty = true) // No section exists, can't proceed. return false; } + + // Check the 'course_module_background_deletion_recommended' hook first. + // Only use asynchronous deletion if at least one plugin returns true and if async deletion has been requested. + // Both are checked because plugins should not be allowed to dictate the deletion behaviour, only support/decline it. + // It's up to plugins to handle things like whether or not they are enabled. + if ($async && $pluginsfunction = get_plugins_with_function('course_module_background_deletion_recommended')) { + foreach ($pluginsfunction as $plugintype => $plugins) { + foreach ($plugins as $pluginfunction) { + if ($pluginfunction()) { + return course_delete_section_async($section, $forcedeleteifnotempty); + } + } + } + } + $format = course_get_format($course); $sectionname = $format->get_section_name($section); @@ -1308,22 +1439,102 @@ function course_delete_section($course, $section, $forcedeleteifnotempty = true) if ($result) { $context = context_course::instance($courseid); $event = \core\event\course_section_deleted::create( - array( - 'objectid' => $section->id, - 'courseid' => $courseid, - 'context' => $context, - 'other' => array( - 'sectionnum' => $section->section, - 'sectionname' => $sectionname, - ) + array( + 'objectid' => $section->id, + 'courseid' => $courseid, + 'context' => $context, + 'other' => array( + 'sectionnum' => $section->section, + 'sectionname' => $sectionname, ) - ); + ) + ); $event->add_record_snapshot('course_sections', $section); $event->trigger(); } return $result; } +/** + * Course section deletion, using an adhoc task for deletion of the modules it contains. + * 1. Schedule all modules within the section for adhoc removal. + * 2. Move all modules to course section 0. + * 3. Delete the resulting empty section. + * + * @param \stdClass $section the section to schedule for deletion. + * @param bool $forcedeleteifnotempty whether to force section deletion if it contains modules. + * @return bool true if the section was scheduled for deletion, false otherwise. + */ +function course_delete_section_async($section, $forcedeleteifnotempty = true) { + global $DB; + + // Objects only, and only valid ones. + if (!is_object($section) || empty($section->id)) { + return false; + } + + // Does the object currently exist in the DB for removal (check for stale objects). + $section = $DB->get_record('course_sections', array('id' => $section->id)); + if (!$section || !$section->section) { + // No section exists, or the section is 0. Can't proceed. + return false; + } + + // Check whether the section can be removed. + if (!$forcedeleteifnotempty && (!empty($section->sequence) || !empty($section->summary))) { + return false; + } + + $format = course_get_format($section->course); + $sectionname = $format->get_section_name($section); + + // Flag those modules having no existing deletion flag. Some modules may have been scheduled for deletion manually, and we don't + // want to create additional adhoc deletion tasks for these. Moving them to section 0 will suffice. + $affectedmods = $DB->get_records_select('course_modules', 'course = ? AND section = ? AND deletioninprogress <> ?', + [$section->course, $section->id, 1], '', 'id'); + $DB->set_field('course_modules', 'deletioninprogress', '1', ['course' => $section->course, 'section' => $section->id]); + + // Move all modules to section 0. + $modules = $DB->get_records('course_modules', ['section' => $section->id], ''); + $sectionzero = $DB->get_record('course_sections', ['course' => $section->course, 'section' => '0']); + foreach ($modules as $mod) { + moveto_module($mod, $sectionzero); + } + + // Create and queue an adhoc task for the deletion of the modules. + $removaltask = new \core_course\task\course_delete_modules(); + $data = array( + 'cms' => $affectedmods + ); + $removaltask->set_custom_data($data); + \core\task\manager::queue_adhoc_task($removaltask); + + // Delete the now empty section, passing in only the section number, which forces the function to fetch a new object. + // The refresh is needed because the section->sequence is now stale. + $result = $format->delete_section($section->section, $forcedeleteifnotempty); + + // Trigger an event for course section deletion. + if ($result) { + $context = \context_course::instance($section->course); + $event = \core\event\course_section_deleted::create( + array( + 'objectid' => $section->id, + 'courseid' => $section->course, + 'context' => $context, + 'other' => array( + 'sectionnum' => $section->section, + 'sectionname' => $sectionname, + ) + ) + ); + $event->add_record_snapshot('course_sections', $section); + $event->trigger(); + } + rebuild_course_cache($section->course, true); + + return $result; +} + /** * Updates the course section * diff --git a/course/rest.php b/course/rest.php index 31064e105d6..a63f6cef128 100644 --- a/course/rest.php +++ b/course/rest.php @@ -168,7 +168,7 @@ switch($requestmethod) { switch ($class) { case 'resource': require_capability('moodle/course:manageactivities', $modcontext); - course_delete_module($cm->id); + course_delete_module($cm->id, true); break; } break; diff --git a/lib/filestorage/file_storage.php b/lib/filestorage/file_storage.php index c3abedda152..c709c5f0dc2 100644 --- a/lib/filestorage/file_storage.php +++ b/lib/filestorage/file_storage.php @@ -2215,7 +2215,12 @@ class file_storage { mkdir($trashpath, $this->dirpermissions, true); } rename($contentfile, $trashfile); - chmod($trashfile, $this->filepermissions); // fix permissions if needed + + // Fix permissions, only if needed. + $currentperms = octdec(substr(decoct(fileperms($trashfile)), -4)); + if ((int)$this->filepermissions !== $currentperms) { + chmod($trashfile, $this->filepermissions); + } } /** From 45bd824e59d53ce7193352b6514e0e1a78f021e6 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Wed, 2 Nov 2016 15:08:35 +0800 Subject: [PATCH 4/8] MDL-54751 core_grades: Gradebook highlights modules pending deletion Added warning to all gradebook pages if any modules are pending deletion. Modified the return values for get_name, and is_locked for affected grade items to indicate their pending deletion. --- grade/lib.php | 7 +++++++ lang/en/grades.php | 2 ++ lib/grade/grade_item.php | 15 ++++++++++++++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/grade/lib.php b/grade/lib.php index 9dacc0b7123..7be114e25e2 100644 --- a/grade/lib.php +++ b/grade/lib.php @@ -970,6 +970,13 @@ function print_grade_page_head($courseid, $active_type, $active_plugin=null, $user = null) { global $CFG, $OUTPUT, $PAGE; + // Put a warning on all gradebook pages if the course has modules currently scheduled for background deletion. + require_once($CFG->dirroot . '/course/lib.php'); + if (course_modules_pending_deletion($courseid)) { + \core\notification::add(get_string('gradesmoduledeletionpendingwarning', 'grades'), + \core\output\notification::NOTIFY_WARNING); + } + if ($active_type === 'preferences') { // In Moodle 2.8 report preferences were moved under 'settings'. Allow backward compatibility for 3rd party grade reports. $active_type = 'settings'; diff --git a/lang/en/grades.php b/lang/en/grades.php index 41f91c3649f..0dfbe07defa 100644 --- a/lang/en/grades.php +++ b/lang/en/grades.php @@ -326,6 +326,8 @@ $string['grades'] = 'Grades'; $string['gradesforuser'] = 'Grades for {$a->user}'; $string['singleview'] = 'Single view for {$a}'; $string['gradesonly'] = 'Change to grades only'; +$string['gradesmoduledeletionpendingwarning'] = 'Warning: Activity deletion in progress! Some grades are about to be removed.'; +$string['gradesmoduledeletionprefix'] = '[Deletion in progress]'; $string['gradessettings'] = 'Grade settings'; $string['gradetype'] = 'Grade type'; $string['gradetype_help'] = 'There are 4 grade types: diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php index 79db6985143..881e7a6eeb7 100644 --- a/lib/grade/grade_item.php +++ b/lib/grade/grade_item.php @@ -524,6 +524,14 @@ class grade_item extends grade_object { * @return bool Locked state */ public function is_locked($userid=NULL) { + global $CFG; + + // Override for any grade items belonging to activities which are in the process of being deleted. + require_once($CFG->dirroot . '/course/lib.php'); + if (course_module_instance_pending_deletion($this->courseid, $this->itemmodule, $this->iteminstance)) { + return true; + } + if (!empty($this->locked)) { return true; } @@ -1393,7 +1401,12 @@ class grade_item extends grade_object { public function get_name($fulltotal=false) { if (strval($this->itemname) !== '') { // MDL-10557 - return format_string($this->itemname); + + // Make it obvious to users if the course module to which this grade item relates, is currently being removed. + $deletionpending = course_module_instance_pending_deletion($this->courseid, $this->itemmodule, $this->iteminstance); + $deletionnotice = get_string('gradesmoduledeletionprefix', 'grades'); + + return $deletionpending ? format_string($deletionnotice . ' ' . $this->itemname) : format_string($this->itemname); } else if ($this->is_course_item()) { return get_string('coursetotal', 'grades'); From 2f6e0d92b7b3fdc85e74e2054ed2097635dfa4af Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Wed, 2 Nov 2016 15:09:44 +0800 Subject: [PATCH 5/8] MDL-54751 core_availability: Module restrictions exclude deletions To make sure users can't add an access restriction based on a course module scheduled for deletion, or its respective grade items. --- availability/condition/completion/classes/frontend.php | 2 +- availability/condition/grade/classes/frontend.php | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/availability/condition/completion/classes/frontend.php b/availability/condition/completion/classes/frontend.php index 02dad76870d..8b425d73c24 100644 --- a/availability/condition/completion/classes/frontend.php +++ b/availability/condition/completion/classes/frontend.php @@ -64,7 +64,7 @@ class frontend extends \core_availability\frontend { foreach ($modinfo->cms as $id => $othercm) { // Add each course-module if it has completion turned on and is not // the one currently being edited. - if ($othercm->completion && (empty($cm) || $cm->id != $id)) { + if ($othercm->completion && (empty($cm) || $cm->id != $id) && !$othercm->deletioninprogress) { $cms[] = (object)array('id' => $id, 'name' => format_string($othercm->name, true, array('context' => $context))); } diff --git a/availability/condition/grade/classes/frontend.php b/availability/condition/grade/classes/frontend.php index 4a906c7d4a5..aee9f9f7739 100644 --- a/availability/condition/grade/classes/frontend.php +++ b/availability/condition/grade/classes/frontend.php @@ -42,6 +42,7 @@ class frontend extends \core_availability\frontend { \section_info $section = null) { global $DB, $CFG; require_once($CFG->libdir . '/gradelib.php'); + require_once($CFG->dirroot . '/course/lib.php'); // Get grades as basic associative array. $gradeoptions = array(); @@ -49,6 +50,10 @@ class frontend extends \core_availability\frontend { // For some reason the fetch_all things return null if none. $items = $items ? $items : array(); foreach ($items as $id => $item) { + // Don't include the grade item if it's linked with a module that is being deleted. + if (course_module_instance_pending_deletion($item->courseid, $item->itemmodule, $item->iteminstance)) { + continue; + } // Do not include grades for current item. if ($cm && $cm->instance == $item->iteminstance && $cm->modname == $item->itemmodule From 3704ff8cde502415d3faf9705f435403c32931b6 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Wed, 2 Nov 2016 15:10:59 +0800 Subject: [PATCH 6/8] MDL-54751 phpunit: Support adhoc module and section deletion in phpunit Created a new phpunit util function run_all_adhoc_tasks which runs any pending tasks, for use in unit tests. Added new recyclebin and course unit tests covering the new functionality. --- .../tool/recyclebin/tests/course_bin_test.php | 12 + course/tests/courselib_test.php | 284 ++++++++++++++++++ lib/phpunit/classes/util.php | 17 ++ 3 files changed, 313 insertions(+) diff --git a/admin/tool/recyclebin/tests/course_bin_test.php b/admin/tool/recyclebin/tests/course_bin_test.php index 1f83de31317..9a4e0c37f3a 100644 --- a/admin/tool/recyclebin/tests/course_bin_test.php +++ b/admin/tool/recyclebin/tests/course_bin_test.php @@ -71,6 +71,9 @@ class tool_recyclebin_course_bin_tests extends advanced_testcase { // Delete the course module. course_delete_module($this->quiz->cmid); + // Now, run the course module deletion adhoc task. + phpunit_util::run_all_adhoc_tasks(); + // Check the course module is now in the recycle bin. $this->assertEquals(1, $DB->count_records('tool_recyclebin_course')); @@ -112,6 +115,9 @@ class tool_recyclebin_course_bin_tests extends advanced_testcase { // Delete the course module. course_delete_module($this->quiz->cmid); + // Now, run the course module deletion adhoc task. + phpunit_util::run_all_adhoc_tasks(); + // Try purging. $recyclebin = new \tool_recyclebin\course_bin($this->course->id); foreach ($recyclebin->get_items() as $item) { @@ -134,6 +140,9 @@ class tool_recyclebin_course_bin_tests extends advanced_testcase { // Delete the quiz. course_delete_module($this->quiz->cmid); + // Now, run the course module deletion adhoc task. + phpunit_util::run_all_adhoc_tasks(); + // Set deleted date to the distant past. $recyclebin = new \tool_recyclebin\course_bin($this->course->id); foreach ($recyclebin->get_items() as $item) { @@ -147,6 +156,9 @@ class tool_recyclebin_course_bin_tests extends advanced_testcase { course_delete_module($book->cmid); + // Now, run the course module deletion adhoc task. + phpunit_util::run_all_adhoc_tasks(); + // Should have 2 items now. $this->assertEquals(2, count($recyclebin->get_items())); diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index f8bea359554..fd39dff0544 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -3379,4 +3379,288 @@ class core_course_courselib_testcase extends advanced_testcase { $this->assertFalse($updates->introfiles->updated); $this->assertFalse($updates->outcomes->updated); } + + public function test_async_module_deletion_hook_implemented() { + // Async module deletion depends on the 'true' being returned by at least one plugin implementing the hook, + // 'course_module_adhoc_deletion_recommended'. In core, is implemented by the course recyclebin, which will only return + // true if the recyclebin plugin is enabled. To make sure async deletion occurs, this test force-enables the recyclebin. + global $DB, $USER; + $this->resetAfterTest(true); + $this->setAdminUser(); + + // Ensure recyclebin is enabled. + set_config('coursebinenable', true, 'tool_recyclebin'); + + // Create course, module and context. + $course = $this->getDataGenerator()->create_course(['numsections' => 5]); + $module = $this->getDataGenerator()->create_module('assign', ['course' => $course->id]); + $modcontext = context_module::instance($module->cmid); + + // Verify context exists. + $this->assertInstanceOf('context_module', $modcontext); + + // Check events generated on the course_delete_module call. + $sink = $this->redirectEvents(); + + // Try to delete the module using the async flag. + course_delete_module($module->cmid, true); // Try to delete the module asynchronously. + + // Verify that no event has been generated yet. + $events = $sink->get_events(); + $event = array_pop($events); + $sink->close(); + $this->assertEmpty($event); + + // Grab the record, in it's final state before hard deletion, for comparison with the event snapshot. + // We need to do this because the 'deletioninprogress' flag has changed from '0' to '1'. + $cm = $DB->get_record('course_modules', ['id' => $module->cmid], '*', MUST_EXIST); + + // Verify the course_module is marked as 'deletioninprogress'. + $this->assertNotEquals($cm, false); + $this->assertEquals($cm->deletioninprogress, '1'); + + // Verify the context has not yet been removed. + $this->assertEquals($modcontext, context_module::instance($module->cmid, IGNORE_MISSING)); + + // Set up a sink to catch the 'course_module_deleted' event. + $sink = $this->redirectEvents(); + + // Now, run the adhoc task which performs the hard deletion. + phpunit_util::run_all_adhoc_tasks(); + + // Fetch and validate the event data. + $events = $sink->get_events(); + $event = array_pop($events); + $sink->close(); + $this->assertInstanceOf('\core\event\course_module_deleted', $event); + $this->assertEquals($module->cmid, $event->objectid); + $this->assertEquals($USER->id, $event->userid); + $this->assertEquals('course_modules', $event->objecttable); + $this->assertEquals(null, $event->get_url()); + $this->assertEquals($cm, $event->get_record_snapshot('course_modules', $module->cmid)); + + // Verify the context has been removed. + $this->assertFalse(context_module::instance($module->cmid, IGNORE_MISSING)); + + // Verify the course_module record has been deleted. + $cmcount = $DB->count_records('course_modules', ['id' => $module->cmid]); + $this->assertEmpty($cmcount); + } + + public function test_async_module_deletion_hook_not_implemented() { + // Only proceed if we are sure that no plugin is going to advocate async removal of a module. I.e. no plugin returns + // 'true' from the 'course_module_adhoc_deletion_recommended' hook. + // In the case of core, only recyclebin implements this hook, and it will only return true if enabled, so disable it. + global $DB, $USER; + $this->resetAfterTest(true); + $this->setAdminUser(); + set_config('coursebinenable', false, 'tool_recyclebin'); + + // Non-core plugins might implement the 'course_module_adhoc_deletion_recommended' hook and spoil this test. + // If at least one plugin still returns true, then skip this test. + if ($pluginsfunction = get_plugins_with_function('course_module_background_deletion_recommended')) { + foreach ($pluginsfunction as $plugintype => $plugins) { + foreach ($plugins as $pluginfunction) { + if ($pluginfunction()) { + $this->markTestSkipped(); + } + } + } + } + + // Create course, module and context. + $course = $this->getDataGenerator()->create_course(['numsections' => 5]); + $module = $this->getDataGenerator()->create_module('assign', ['course' => $course->id]); + $modcontext = context_module::instance($module->cmid); + $cm = $DB->get_record('course_modules', ['id' => $module->cmid], '*', MUST_EXIST); + + // Verify context exists. + $this->assertInstanceOf('context_module', $modcontext); + + // Check events generated on the course_delete_module call. + $sink = $this->redirectEvents(); + + // Try to delete the module using the async flag. + course_delete_module($module->cmid, true); // Try to delete the module asynchronously. + + // Fetch and validate the event data. + $events = $sink->get_events(); + $event = array_pop($events); + $sink->close(); + $this->assertInstanceOf('\core\event\course_module_deleted', $event); + $this->assertEquals($module->cmid, $event->objectid); + $this->assertEquals($USER->id, $event->userid); + $this->assertEquals('course_modules', $event->objecttable); + $this->assertEquals(null, $event->get_url()); + $this->assertEquals($cm, $event->get_record_snapshot('course_modules', $module->cmid)); + + // Verify the context has been removed. + $this->assertFalse(context_module::instance($module->cmid, IGNORE_MISSING)); + + // Verify the course_module record has been deleted. + $cmcount = $DB->count_records('course_modules', ['id' => $module->cmid]); + $this->assertEmpty($cmcount); + } + + public function test_async_section_deletion_hook_implemented() { + // Async section deletion (provided section contains modules), depends on the 'true' being returned by at least one plugin + // implementing the 'course_module_adhoc_deletion_recommended' hook. In core, is implemented by the course recyclebin, + // which will only return true if the plugin is enabled. To make sure async deletion occurs, this test enables recyclebin. + global $DB, $USER; + $this->resetAfterTest(true); + $this->setAdminUser(); + + // Ensure recyclebin is enabled. + set_config('coursebinenable', true, 'tool_recyclebin'); + + // Create course, module and context. + $generator = $this->getDataGenerator(); + $course = $generator->create_course(['numsections' => 4, 'format' => 'topics'], ['createsections' => true]); + $assign0 = $generator->create_module('assign', ['course' => $course, 'section' => 2]); + $assign1 = $generator->create_module('assign', ['course' => $course, 'section' => 2]); + $assign2 = $generator->create_module('assign', ['course' => $course, 'section' => 2]); + $assign3 = $generator->create_module('assign', ['course' => $course, 'section' => 0]); + + // Delete empty section. No difference from normal, synchronous behaviour. + $this->assertTrue(course_delete_section($course, 4, false, true)); + $this->assertEquals(3, course_get_format($course)->get_course()->numsections); + + // Delete a module in section 2 (using async). Need to verify this doesn't generate two tasks when we delete + // the section in the next step. + course_delete_module($assign2->cmid, true); + + // Confirm that the module is pending deletion in its current section. + $section = $DB->get_record('course_sections', ['course' => $course->id, 'section' => '2']); // For event comparison. + $this->assertEquals(true, $DB->record_exists('course_modules', ['id' => $assign2->cmid, 'deletioninprogress' => 1, + 'section' => $section->id])); + + // Now, delete section 2. + $this->assertFalse(course_delete_section($course, 2, false, true)); // Non-empty section, no forcedelete, so no change. + + $sink = $this->redirectEvents(); // To capture the event. + $this->assertTrue(course_delete_section($course, 2, true, true)); + + // Now, confirm that: + // a) the section's modules have been flagged for deletion and moved to section 0 and; + // b) the section has been deleted and; + // c) course_section_deleted event has been fired. The course_module_deleted events will only fire once they have been + // removed from section 0 via the adhoc task. + + // Modules should have been flagged for deletion and moved to section 0. + $sectionid = $DB->get_field('course_sections', 'id', ['course' => $course->id, 'section' => 0]); + $this->assertEquals(3, $DB->count_records('course_modules', ['section' => $sectionid, 'deletioninprogress' => 1])); + + // Confirm the section has been deleted. + $this->assertEquals(2, course_get_format($course)->get_course()->numsections); + + // Check event fired. + $events = $sink->get_events(); + $event = array_pop($events); + $sink->close(); + $this->assertInstanceOf('\core\event\course_section_deleted', $event); + $this->assertEquals($section->id, $event->objectid); + $this->assertEquals($USER->id, $event->userid); + $this->assertEquals('course_sections', $event->objecttable); + $this->assertEquals(null, $event->get_url()); + $this->assertEquals($section, $event->get_record_snapshot('course_sections', $section->id)); + + // Now, run the adhoc task to delete the modules from section 0. + $sink = $this->redirectEvents(); // To capture the events. + phpunit_util::run_all_adhoc_tasks(); + + // Confirm the modules have been deleted. + list($insql, $assignids) = $DB->get_in_or_equal([$assign0->cmid, $assign1->cmid, $assign2->cmid]); + $cmcount = $DB->count_records_select('course_modules', 'id ' . $insql, $assignids); + $this->assertEmpty($cmcount); + + // Confirm other modules in section 0 still remain. + $this->assertEquals(1, $DB->count_records('course_modules', ['id' => $assign3->cmid])); + + // Confirm that events were generated for all 3 of the modules. + $events = $sink->get_events(); + $sink->close(); + $count = 0; + while (!empty($events)) { + $event = array_pop($events); + if (in_array($event->objectid, [$assign0->cmid, $assign1->cmid, $assign2->cmid])) { + $count++; + } + } + $this->assertEquals(3, $count); + } + + public function test_async_section_deletion_hook_not_implemented() { + // If no plugins advocate async removal, then normal synchronous removal will take place. + // Only proceed if we are sure that no plugin is going to advocate async removal of a module. I.e. no plugin returns + // 'true' from the 'course_module_adhoc_deletion_recommended' hook. + // In the case of core, only recyclebin implements this hook, and it will only return true if enabled, so disable it. + global $DB, $USER; + $this->resetAfterTest(true); + $this->setAdminUser(); + set_config('coursebinenable', false, 'tool_recyclebin'); + + // Non-core plugins might implement the 'course_module_adhoc_deletion_recommended' hook and spoil this test. + // If at least one plugin still returns true, then skip this test. + if ($pluginsfunction = get_plugins_with_function('course_module_background_deletion_recommended')) { + foreach ($pluginsfunction as $plugintype => $plugins) { + foreach ($plugins as $pluginfunction) { + if ($pluginfunction()) { + $this->markTestSkipped(); + } + } + } + } + + // Create course, module and context. + $generator = $this->getDataGenerator(); + $course = $generator->create_course(['numsections' => 4, 'format' => 'topics'], ['createsections' => true]); + $assign0 = $generator->create_module('assign', ['course' => $course, 'section' => 2]); + $assign1 = $generator->create_module('assign', ['course' => $course, 'section' => 2]); + + // Delete empty section. No difference from normal, synchronous behaviour. + $this->assertTrue(course_delete_section($course, 4, false, true)); + $this->assertEquals(3, course_get_format($course)->get_course()->numsections); + + // Delete section in the middle (2). + $section = $DB->get_record('course_sections', ['course' => $course->id, 'section' => '2']); // For event comparison. + $this->assertFalse(course_delete_section($course, 2, false, true)); // Non-empty section, no forcedelete, so no change. + + $sink = $this->redirectEvents(); // To capture the event. + $this->assertTrue(course_delete_section($course, 2, true, true)); + + // Now, confirm that: + // a) The section's modules have deleted and; + // b) the section has been deleted and; + // c) course_section_deleted event has been fired and; + // d) course_module_deleted events have both been fired. + + // Confirm modules have been deleted. + list($insql, $assignids) = $DB->get_in_or_equal([$assign0->cmid, $assign1->cmid]); + $cmcount = $DB->count_records_select('course_modules', 'id ' . $insql, $assignids); + $this->assertEmpty($cmcount); + + // Confirm the section has been deleted. + $this->assertEquals(2, course_get_format($course)->get_course()->numsections); + + // Confirm the course_section_deleted event has been generated. + $events = $sink->get_events(); + $event = array_pop($events); + $sink->close(); + $this->assertInstanceOf('\core\event\course_section_deleted', $event); + $this->assertEquals($section->id, $event->objectid); + $this->assertEquals($USER->id, $event->userid); + $this->assertEquals('course_sections', $event->objecttable); + $this->assertEquals(null, $event->get_url()); + $this->assertEquals($section, $event->get_record_snapshot('course_sections', $section->id)); + + // Confirm that the course_module_deleted events have both been generated. + $count = 0; + while (!empty($events)) { + $event = array_pop($events); + if (in_array($event->objectid, [$assign0->cmid, $assign1->cmid])) { + $count++; + } + } + $this->assertEquals(2, $count); + } } diff --git a/lib/phpunit/classes/util.php b/lib/phpunit/classes/util.php index d91528cf55c..397a611c9e8 100644 --- a/lib/phpunit/classes/util.php +++ b/lib/phpunit/classes/util.php @@ -819,4 +819,21 @@ class phpunit_util extends testing_util { return 'en_AU.UTF-8'; } } + + /** + * Executes all adhoc tasks in the queue. Useful for testing asynchronous behaviour. + * + * @return void + */ + public static function run_all_adhoc_tasks() { + $now = time(); + while (($task = \core\task\manager::get_next_adhoc_task($now)) !== null) { + try { + $task->execute(); + \core\task\manager::adhoc_task_complete($task); + } catch (Exception $e) { + \core\task\manager::adhoc_task_failed($task); + } + } + } } From ff4230d88ecd24d1f68e1efec3d221fbc8cdfe67 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Wed, 2 Nov 2016 15:11:22 +0800 Subject: [PATCH 7/8] MDL-54751 behat: Support for adhoc module and section deletion in behat Introduced new behat step for running all pending adhoc tasks and modified the relavant behat tests. --- .../tests/behat/backup_user_data.feature | 1 + .../tests/behat/basic_functionality.feature | 2 + backup/util/loggers/base_logger.class.php | 3 +- .../tests/behat/structural_changes.feature | 1 + lib/tests/behat/behat_general.php | 37 +++++++++++++++++++ 5 files changed, 43 insertions(+), 1 deletion(-) diff --git a/admin/tool/recyclebin/tests/behat/backup_user_data.feature b/admin/tool/recyclebin/tests/behat/backup_user_data.feature index 500a2730b2f..c0bc961a89e 100644 --- a/admin/tool/recyclebin/tests/behat/backup_user_data.feature +++ b/admin/tool/recyclebin/tests/behat/backup_user_data.feature @@ -58,6 +58,7 @@ Feature: Backup user data And I follow "Course 1" And I turn editing mode on And I delete "Quiz 1" activity + And I run all adhoc tasks And I navigate to "Recycle bin" node in "Course administration" And I should see "Quiz 1" And I click on "Restore" "link" in the "region-main" "region" diff --git a/admin/tool/recyclebin/tests/behat/basic_functionality.feature b/admin/tool/recyclebin/tests/behat/basic_functionality.feature index ded5e99f349..b10af91f9e4 100644 --- a/admin/tool/recyclebin/tests/behat/basic_functionality.feature +++ b/admin/tool/recyclebin/tests/behat/basic_functionality.feature @@ -69,6 +69,7 @@ Feature: Basic recycle bin functionality | Assignment name | Test assign | | Description | Test | And I delete "Test assign" activity + And I run all adhoc tasks And I navigate to "Recycle bin" node in "Course administration" When I click on "Delete" "link" Then I should see "Are you sure you want to delete the selected item from the recycle bin?" @@ -92,6 +93,7 @@ Feature: Basic recycle bin functionality | Description | Test 2 | And I delete "Test assign 1" activity And I delete "Test assign 2" activity + And I run all adhoc tasks And I navigate to "Recycle bin" node in "Course administration" And I should see "Test assign 1" And I should see "Test assign 2" diff --git a/backup/util/loggers/base_logger.class.php b/backup/util/loggers/base_logger.class.php index 32b0c06c939..26bb05cec4a 100644 --- a/backup/util/loggers/base_logger.class.php +++ b/backup/util/loggers/base_logger.class.php @@ -114,7 +114,8 @@ abstract class base_logger implements checksumable { public final function process($message, $level, $options = null) { $result = true; - if ($this->level != backup::LOG_NONE && $this->level >= $level) { // Perform action conditionally + if ($this->level != backup::LOG_NONE && $this->level >= $level + && !(defined('BEHAT_TEST') && BEHAT_TEST)) { // Perform action conditionally. $result = $this->action($message, $level, $options); } if ($result === false) { // Something was wrong, stop the chain diff --git a/blocks/recent_activity/tests/behat/structural_changes.feature b/blocks/recent_activity/tests/behat/structural_changes.feature index d66978a4be6..5914281d57b 100644 --- a/blocks/recent_activity/tests/behat/structural_changes.feature +++ b/blocks/recent_activity/tests/behat/structural_changes.feature @@ -194,6 +194,7 @@ Feature: View structural changes in recent activity block And I follow "Course 1" And I turn editing mode on And I delete "ForumUpdated" activity + And I run all adhoc tasks And I log out And I wait "1" seconds # Students 1 and 2 see that forum was deleted diff --git a/lib/tests/behat/behat_general.php b/lib/tests/behat/behat_general.php index edc4cca71a1..a6be6f4a3b8 100644 --- a/lib/tests/behat/behat_general.php +++ b/lib/tests/behat/behat_general.php @@ -1001,6 +1001,43 @@ class behat_general extends behat_base { } } + /** + * Runs all ad-hoc tasks in the queue. + * + * This is faster and more reliable than running cron (running cron won't + * work more than once in the same test, for instance). However it is + * a little less 'realistic'. + * + * While the task is running, we suppress mtrace output because it makes + * the Behat result look ugly. + * + * @Given /^I run all adhoc tasks$/ + * @throws DriverException + */ + public function i_run_all_adhoc_tasks() { + // Do setup for cron task. + cron_setup_user(); + + // Run tasks. Locking is handled by get_next_adhoc_task. + $now = time(); + ob_start(); // Discard task output as not appropriate for Behat output! + while (($task = \core\task\manager::get_next_adhoc_task($now)) !== null) { + + try { + $task->execute(); + + // Mark task complete. + \core\task\manager::adhoc_task_complete($task); + } catch (Exception $e) { + // Mark task failed and throw exception. + \core\task\manager::adhoc_task_failed($task); + ob_end_clean(); + throw new DriverException('An adhoc task failed', 0, $e); + } + } + ob_end_clean(); + } + /** * Checks that an element and selector type exists in another element and selector type on the current page. * From f939a6d45208a27f1ca1b8364ca6cac666c2a5a1 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Fri, 4 Nov 2016 08:28:38 +0800 Subject: [PATCH 8/8] MDL-54751 core_completion: Exclude activities pending deletion --- lib/completionlib.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/completionlib.php b/lib/completionlib.php index cba3ed9707a..80c854242db 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -1063,7 +1063,7 @@ class completion_info { $modinfo = get_fast_modinfo($this->course); $result = array(); foreach ($modinfo->get_cms() as $cm) { - if ($cm->completion != COMPLETION_TRACKING_NONE) { + if ($cm->completion != COMPLETION_TRACKING_NONE && !$cm->deletioninprogress) { $result[$cm->id] = $cm; } }