From f5dda47ba2256e4b385463a091fa041e47751a2f Mon Sep 17 00:00:00 2001 From: Sara Arjona Date: Fri, 17 Oct 2025 12:58:49 +0200 Subject: [PATCH] MDL-86856 course: Replace course_delete_module with cmactions::delete --- .../monitor/tests/eventobservers_test.php | 2 +- .../classes/local/import_processor.php | 6 +- .../tool/recyclebin/tests/course_bin_test.php | 16 +- .../recyclebin/tests/event/events_test.php | 6 +- .../tests/externallib_test.php | 2 +- .../tests/observer_test.php | 2 +- public/calendar/tests/container_test.php | 2 +- public/competency/tests/hooks_test.php | 2 +- public/completion/tests/api_test.php | 2 +- .../classes/dndupload_ajax_processor.php | 4 +- .../classes/task/course_delete_modules.php | 5 +- public/course/externallib.php | 4 +- public/course/format/classes/base.php | 8 +- .../tests/local/sectionactions_test.php | 2 +- public/course/mod.php | 2 +- .../tests/course_delete_modules_test.php | 1 - public/course/tests/courselib_test.php | 278 +----------------- public/course/tests/modlib_test.php | 2 +- .../nofactivities/nofactivities_test.php | 6 +- .../field/number/tests/observer_test.php | 4 +- public/enrol/lti/tests/lib_test.php | 2 +- .../ltiadvantage/task/sync_members_test.php | 2 +- .../task/sync_tool_grades_test.php | 4 +- public/lib/moodlelib.php | 2 +- public/lib/tests/event/grade_deleted_test.php | 2 +- public/lib/tests/gradelib_test.php | 2 +- .../h5p_clean_orphaned_records_task_test.php | 2 +- .../tests/backup_restore_test.php | 2 +- .../tests/local/extension_test.php | 2 +- .../task/upgrade_recordings_task_test.php | 2 +- .../tests/task/cleanup_test.php | 2 +- public/mod/lti/tests/lib_test.php | 2 +- .../local/bank/question_bank_helper_test.php | 2 +- .../engine/simpledb/tests/engine_test.php | 2 +- .../search/engine/solr/tests/engine_test.php | 2 +- public/search/tests/base_activity_test.php | 2 +- public/search/tests/manager_test.php | 6 +- 37 files changed, 63 insertions(+), 333 deletions(-) diff --git a/public/admin/tool/monitor/tests/eventobservers_test.php b/public/admin/tool/monitor/tests/eventobservers_test.php index bd57cceff0e..252378263bb 100644 --- a/public/admin/tool/monitor/tests/eventobservers_test.php +++ b/public/admin/tool/monitor/tests/eventobservers_test.php @@ -631,7 +631,7 @@ final class eventobservers_test extends \advanced_testcase { $this->assertCount(20, $totalsubs); // Let us delete the user now. - course_delete_module($assign->cmid); + \core_courseformat\formatactions::cm($course1->id)->delete($assign->cmid); // Verify data after course delete. $totalrules = \tool_monitor\rule_manager::get_rules_by_plugin('test'); diff --git a/public/admin/tool/moodlenet/classes/local/import_processor.php b/public/admin/tool/moodlenet/classes/local/import_processor.php index 0d22128c74f..4a17253ab86 100644 --- a/public/admin/tool/moodlenet/classes/local/import_processor.php +++ b/public/admin/tool/moodlenet/classes/local/import_processor.php @@ -170,7 +170,8 @@ class import_processor { if (!$instanceid) { // Something has gone wrong - undo everything we can. - course_delete_module($cmid); + $coursecontext = \context_module::instance($cmid)->get_course_context(); + \core_courseformat\formatactions::cm($coursecontext->instanceid)->delete($cmid); throw new \moodle_exception('errorcreatingactivity', 'moodle', '', $this->handlerinfo->get_module_name()); } @@ -193,7 +194,7 @@ class import_processor { $info = get_fast_modinfo($this->course, $this->user->id); if (!isset($info->cms[$cmid])) { // The course module has not been properly created in the course - undo everything. - course_delete_module($cmid); + \core_courseformat\formatactions::cm($this->course->id)->delete($cmid); throw new \moodle_exception('errorcreatingactivity', 'moodle', '', $this->handlerinfo->get_module_name()); } $mod = $info->get_cm($cmid); @@ -203,4 +204,3 @@ class import_processor { $event->trigger(); } } - diff --git a/public/admin/tool/recyclebin/tests/course_bin_test.php b/public/admin/tool/recyclebin/tests/course_bin_test.php index 37e9f10a302..76b19844ba1 100644 --- a/public/admin/tool/recyclebin/tests/course_bin_test.php +++ b/public/admin/tool/recyclebin/tests/course_bin_test.php @@ -65,7 +65,7 @@ final class course_bin_test extends \advanced_testcase { $this->assertEquals(0, $DB->count_records('tool_recyclebin_course')); // Delete the course module. - course_delete_module($this->quiz->cmid); + \core_courseformat\formatactions::cm($this->course->id)->delete($this->quiz->cmid); // Check the course module is now in the recycle bin. $this->assertEquals(1, $DB->count_records('tool_recyclebin_course')); @@ -84,7 +84,7 @@ final class course_bin_test extends \advanced_testcase { $startcount = $DB->count_records('course_modules'); // Delete the course module. - course_delete_module($this->quiz->cmid); + \core_courseformat\formatactions::cm($this->course->id)->delete($this->quiz->cmid); // Try restoring. $recyclebin = new \tool_recyclebin\course_bin($this->course->id); @@ -106,7 +106,7 @@ final class course_bin_test extends \advanced_testcase { $startcount = $DB->count_records('course_modules'); // Delete the course module. - course_delete_module($this->quiz->cmid); + \core_courseformat\formatactions::cm($this->course->id)->delete($this->quiz->cmid); // Try purging. $recyclebin = new \tool_recyclebin\course_bin($this->course->id); @@ -128,7 +128,7 @@ final class course_bin_test extends \advanced_testcase { set_config('coursebinexpiry', WEEKSECS, 'tool_recyclebin'); // Delete the quiz. - course_delete_module($this->quiz->cmid); + \core_courseformat\formatactions::cm($this->course->id)->delete($this->quiz->cmid); // Set deleted date to the distant past. $recyclebin = new \tool_recyclebin\course_bin($this->course->id); @@ -141,7 +141,7 @@ final class course_bin_test extends \advanced_testcase { $book = $this->getDataGenerator()->get_plugin_generator('mod_book')->create_instance(array( 'course' => $this->course->id)); - course_delete_module($book->cmid); + \core_courseformat\formatactions::cm($this->course->id)->delete($book->cmid); // Should have 2 items now. $this->assertEquals(2, count($recyclebin->get_items())); @@ -211,7 +211,7 @@ final class course_bin_test extends \advanced_testcase { // Delete quiz. $cm = get_coursemodule_from_instance('quiz', $this->quiz->id); - course_delete_module($cm->id); + \core_courseformat\formatactions::cm($this->course->id)->delete($cm->id); $quizzes = get_coursemodules_in_course('quiz', $this->course->id); $this->assertEquals(0, count($quizzes)); @@ -246,7 +246,7 @@ final class course_bin_test extends \advanced_testcase { set_config('backup_auto_activities', false, 'backup'); // Delete the course module. - course_delete_module($this->quiz->cmid); + \core_courseformat\formatactions::cm($this->course->id)->delete($this->quiz->cmid); // Check there is no items in the recycle bin. $recyclebin = new \tool_recyclebin\course_bin($this->course->id); @@ -277,7 +277,7 @@ final class course_bin_test extends \advanced_testcase { // Delete quiz. $cm = get_coursemodule_from_instance('quiz', $this->quiz->id); - course_delete_module($cm->id); + \core_courseformat\formatactions::cm($this->course->id)->delete($cm->id); $quizzes = get_coursemodules_in_course('quiz', $this->course->id); $this->assertEquals(0, count($quizzes)); diff --git a/public/admin/tool/recyclebin/tests/event/events_test.php b/public/admin/tool/recyclebin/tests/event/events_test.php index c6d2e7daa1b..f7d71b81fe2 100644 --- a/public/admin/tool/recyclebin/tests/event/events_test.php +++ b/public/admin/tool/recyclebin/tests/event/events_test.php @@ -146,7 +146,7 @@ final class events_test extends \advanced_testcase { // Trigger and capture the event. $sink = $this->redirectEvents(); - course_delete_module($instance->cmid); + \core_courseformat\formatactions::cm($course->id)->delete($instance->cmid); $events = $sink->get_events(); $event = reset($events); @@ -174,7 +174,7 @@ final class events_test extends \advanced_testcase { $instance = $generator->create_instance(array('course' => $course->id)); // Delete the module. - course_delete_module($instance->cmid); + \core_courseformat\formatactions::cm($course->id)->delete($instance->cmid); // Get the item from the recycle bin. $rb = new \tool_recyclebin\course_bin($course->id); @@ -206,7 +206,7 @@ final class events_test extends \advanced_testcase { $generator = $this->getDataGenerator()->get_plugin_generator('mod_assign'); $instance = $generator->create_instance(array('course' => $course->id)); - course_delete_module($instance->cmid); + \core_courseformat\formatactions::cm($course->id)->delete($instance->cmid); // Get the item from the recycle bin. $rb = new \tool_recyclebin\course_bin($course->id); diff --git a/public/blocks/recentlyaccesseditems/tests/externallib_test.php b/public/blocks/recentlyaccesseditems/tests/externallib_test.php index 1a956aa6d83..21c81f92b12 100644 --- a/public/blocks/recentlyaccesseditems/tests/externallib_test.php +++ b/public/blocks/recentlyaccesseditems/tests/externallib_test.php @@ -112,7 +112,7 @@ final class externallib_test extends \core_external\tests\externallib_testcase { $this->assertCount((count($forum) + count($assign) + count($h5pactivity)) - 3, $result); // Delete a single course module should still return. - course_delete_module($forum[1]->cmid); + \core_courseformat\formatactions::cm($courses[1]->id)->delete($forum[1]->cmid); $result = \block_recentlyaccesseditems\external::get_recent_items(); $this->assertCount((count($forum) + count($assign) + count($h5pactivity)) - 4, $result); } diff --git a/public/blocks/recentlyaccesseditems/tests/observer_test.php b/public/blocks/recentlyaccesseditems/tests/observer_test.php index 8f3a155f70d..3f029f992c8 100644 --- a/public/blocks/recentlyaccesseditems/tests/observer_test.php +++ b/public/blocks/recentlyaccesseditems/tests/observer_test.php @@ -172,7 +172,7 @@ final class observer_test extends \advanced_testcase { $records = $DB->count_records($this->table, array('cmid' => $this->forum->cmid)); $this->assertEquals(2, $records); - course_delete_module($this->forum->cmid); + \core_courseformat\formatactions::cm($this->course->id)->delete($this->forum->cmid); $records = $DB->count_records($this->table, array('cmid' => $this->forum->cmid)); $this->assertEquals(0, $records); $records = $DB->count_records($this->table, ['cmid' => $this->assign->cmid]); diff --git a/public/calendar/tests/container_test.php b/public/calendar/tests/container_test.php index 40e7494965f..12dd8c9c8af 100644 --- a/public/calendar/tests/container_test.php +++ b/public/calendar/tests/container_test.php @@ -483,7 +483,7 @@ final class container_test extends \advanced_testcase { $this->create_event(['modulename' => $modname, 'instance' => $module->id, 'courseid' => $course->id]); // Delete module and make sure all events are deleted. - course_delete_module($module->cmid); + \core_courseformat\formatactions::cm($course->id)->delete($module->cmid); $this->assertEmpty($DB->get_record('event', ['modulename' => $modname, 'instance' => $module->id])); } } diff --git a/public/competency/tests/hooks_test.php b/public/competency/tests/hooks_test.php index 699ac012070..02a94a7bd4f 100644 --- a/public/competency/tests/hooks_test.php +++ b/public/competency/tests/hooks_test.php @@ -107,7 +107,7 @@ final class hooks_test extends \advanced_testcase { $ucc2b = $ccg->create_user_competency_course(['competencyid' => $comp2->get('id'), 'courseid' => $c2->id, 'userid' => $u1->id]); - course_delete_module($assign1b->cmid); + \core_courseformat\formatactions::cm($c1->id)->delete($assign1b->cmid); $this->assertEquals(2, course_competency::count_records(['courseid' => $c1->id])); $this->assertEquals(1, course_module_competency::count_records(['cmid' => $assign1a->cmid])); diff --git a/public/completion/tests/api_test.php b/public/completion/tests/api_test.php index cb56c992a58..6ece0da1ea2 100644 --- a/public/completion/tests/api_test.php +++ b/public/completion/tests/api_test.php @@ -68,7 +68,7 @@ final class api_test extends \advanced_testcase { require_once($CFG->dirroot . '/course/lib.php'); // Delete the module. - course_delete_module($assign->cmid); + \core_courseformat\formatactions::cm($course->id)->delete($assign->cmid); // Check we don't get a failure when called on a deleted module. \core_completion\api::update_completion_date_event($assign->cmid, 'assign', null, $time); diff --git a/public/course/classes/dndupload_ajax_processor.php b/public/course/classes/dndupload_ajax_processor.php index fabc6269b9e..a9cbb649fba 100644 --- a/public/course/classes/dndupload_ajax_processor.php +++ b/public/course/classes/dndupload_ajax_processor.php @@ -273,7 +273,7 @@ class dndupload_ajax_processor { if (!$instanceid) { // Something has gone wrong - undo everything we can. - course_delete_module($this->cm->id); + \core_courseformat\formatactions::cm($this->course->id)->delete($this->cm->id); throw new moodle_exception('errorcreatingactivity', 'moodle', '', $this->module->name); } @@ -297,7 +297,7 @@ class dndupload_ajax_processor { $info = get_fast_modinfo($this->course); if (!isset($info->cms[$this->cm->id])) { // The course module has not been properly created in the course - undo everything. - course_delete_module($this->cm->id); + \core_courseformat\formatactions::cm($this->course->id)->delete($this->cm->id); throw new moodle_exception('errorcreatingactivity', 'moodle', '', $this->module->name); } $mod = $info->get_cm($this->cm->id); diff --git a/public/course/classes/task/course_delete_modules.php b/public/course/classes/task/course_delete_modules.php index 0ce504414dd..8a55025d9e3 100644 --- a/public/course/classes/task/course_delete_modules.php +++ b/public/course/classes/task/course_delete_modules.php @@ -28,7 +28,7 @@ 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 + * This task supports an array of course module object as custom_data, and calls cmactions::delete() in synchronous deletion * mode for each of them. * This will: * 1. call any 'mod_xxx_pre_course_module_deleted' functions (e.g. Recycle bin) @@ -65,7 +65,8 @@ class course_delete_modules extends \core\task\adhoc_task { $cmsfailed = []; foreach ($cms as $key => $cm) { try { - course_delete_module($cm->id); + $coursecontext = \context_module::instance($cm->id)->get_course_context(); + \core_courseformat\formatactions::cm($coursecontext->instanceid)->delete($cm->id); } catch (\Exception $e) { // Keep the information instead of throw an exception and continue with next cms. $exceptions[] = ("The course module {$cm->id} could not be deleted. " diff --git a/public/course/externallib.php b/public/course/externallib.php index be64fd4fc52..c798b707a08 100644 --- a/public/course/externallib.php +++ b/public/course/externallib.php @@ -2438,7 +2438,7 @@ class core_course_external extends external_api { require_capability('moodle/course:manageactivities', $modcontext); // Delete the module. - course_delete_module($cm->id); + \core_courseformat\formatactions::cm($cm->course)->delete($cm->id); } } @@ -3764,7 +3764,7 @@ class core_course_external extends external_api { break; case 'delete': require_capability('moodle/course:manageactivities', $modcontext); - course_delete_module($cm->id, true); + \core_courseformat\formatactions::cm($course->id)->delete($cm->id, true); return ''; default: throw new coding_exception('Unrecognised action'); diff --git a/public/course/format/classes/base.php b/public/course/format/classes/base.php index 211d0a36152..41618a37667 100644 --- a/public/course/format/classes/base.php +++ b/public/course/format/classes/base.php @@ -1842,7 +1842,7 @@ abstract class base { // Delete all modules from the section. foreach (preg_split('/,/', $section->sequence, -1, PREG_SPLIT_NO_EMPTY) as $cmid) { - course_delete_module($cmid); + \core_courseformat\formatactions::cm($course->id)->delete($cmid); } // Delete section and it's format options. @@ -1867,16 +1867,16 @@ abstract class base { } /** - * Wrapper for course_delete_module method. + * Wrapper for delete method in course format cmactions. * - * Format plugins can override this method to provide their own implementation of course_delete_module. + * Format plugins can override this method to provide their own implementation of cmactions::delete. * * @param cm_info $cm the course module information * @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 */ public function delete_module(cm_info $cm, bool $async = false) { - course_delete_module($cm->id, $async); + \core_courseformat\formatactions::cm($cm->course)->delete($cm->id, $async); } /** diff --git a/public/course/format/tests/local/sectionactions_test.php b/public/course/format/tests/local/sectionactions_test.php index b1d2b62a7df..18c0f01ca85 100644 --- a/public/course/format/tests/local/sectionactions_test.php +++ b/public/course/format/tests/local/sectionactions_test.php @@ -477,7 +477,7 @@ final class sectionactions_test extends \advanced_testcase { // 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); + \core_courseformat\formatactions::cm($course->id)->delete($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. diff --git a/public/course/mod.php b/public/course/mod.php index 2e0e6fbf9a3..ab2fbb945f7 100644 --- a/public/course/mod.php +++ b/public/course/mod.php @@ -191,7 +191,7 @@ if (!empty($add)) { } // Delete the module. - course_delete_module($cm->id); + \core_courseformat\formatactions::cm($course->id)->delete($cm->id); redirect($return); } diff --git a/public/course/tests/course_delete_modules_test.php b/public/course/tests/course_delete_modules_test.php index cc402b7d9f8..630fe466b20 100644 --- a/public/course/tests/course_delete_modules_test.php +++ b/public/course/tests/course_delete_modules_test.php @@ -108,7 +108,6 @@ final class course_delete_modules_test extends \advanced_testcase { $this->assertInstanceOf(\coding_exception::class, $e); $errormsg = str_replace('\\', '/', $e->getMessage()); // Normalise dir separator. $this->assertStringContainsString('cannotdeletemodulemissinglib', $errormsg); - $this->assertStringContainsString('course/lib.php', $errormsg); $this->assertStringContainsString('Missing file mod/TestModuleToDelete/lib.php', $errormsg); // Get line numbers array which contains the exception name. $lines = array_keys(preg_grep("/cannotdeletemodulemissinglib/", file(dirname(__DIR__) . '/lib.php'))); diff --git a/public/course/tests/courselib_test.php b/public/course/tests/courselib_test.php index 379d811cdb8..fd1d2c0df95 100644 --- a/public/course/tests/courselib_test.php +++ b/public/course/tests/courselib_test.php @@ -631,20 +631,6 @@ final class courselib_test extends advanced_testcase { return $moduleinfo; } - /** - * Data provider for course_delete module - * - * @return array An array of arrays contain test data - */ - public static function provider_course_delete_module(): array { - $data = array(); - - $data['assign'] = array('assign', array('duedate' => time())); - $data['quiz'] = array('quiz', array('duedate' => time())); - - return $data; - } - /** * Test the create_course function */ @@ -1855,117 +1841,6 @@ final class courselib_test extends advanced_testcase { $this->assertEquals($pagecm->visible, 0); } - /** - * Tests the function that deletes a course module - * - * @param string $type The type of module for the test - * @param array $options The options for the module creation - * @dataProvider provider_course_delete_module - */ - public function test_course_delete_module($type, $options): void { - global $DB; - - $this->resetAfterTest(true); - $this->setAdminUser(); - - // Create course and modules. - $course = $this->getDataGenerator()->create_course(array('numsections' => 5)); - $options['course'] = $course->id; - - // Generate an assignment with due date (will generate a course event). - $module = $this->getDataGenerator()->create_module($type, $options); - - // Get the module context. - $modcontext = context_module::instance($module->cmid); - - $assocblog = $this->create_module_asscociated_blog($course, $modcontext); - - // Verify context exists. - $this->assertInstanceOf('context_module', $modcontext); - - // Make module specific messes. - switch ($type) { - case 'assign': - // Add some tags to this assignment. - core_tag_tag::set_item_tags('mod_assign', 'assign', $module->id, $modcontext, array('Tag 1', 'Tag 2', 'Tag 3')); - core_tag_tag::set_item_tags('core', 'course_modules', $module->cmid, $modcontext, array('Tag 3', 'Tag 4', 'Tag 5')); - - // Confirm the tag instances were added. - $criteria = array('component' => 'mod_assign', 'itemtype' => 'assign', 'contextid' => $modcontext->id); - $this->assertEquals(3, $DB->count_records('tag_instance', $criteria)); - $criteria = array('component' => 'core', 'itemtype' => 'course_modules', 'contextid' => $modcontext->id); - $this->assertEquals(3, $DB->count_records('tag_instance', $criteria)); - - // Verify event assignment event has been generated. - $eventcount = $DB->count_records('event', array('instance' => $module->id, 'modulename' => $type)); - $this->assertEquals(1, $eventcount); - - break; - case 'quiz': - $qgen = $this->getDataGenerator()->get_plugin_generator('core_question'); - $qcat = $qgen->create_question_category(array('contextid' => $modcontext->id)); - $qgen->create_question('shortanswer', null, array('category' => $qcat->id)); - $qgen->create_question('shortanswer', null, array('category' => $qcat->id)); - break; - default: - break; - } - - // Run delete.. - course_delete_module($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', array('id' => $module->cmid)); - $this->assertEmpty($cmcount); - - // Verify the blog_association record has been deleted. - $this->assertCount(0, $DB->get_records('blog_association', - array('contextid' => $modcontext->id))); - - // Verify the blog post record has been deleted. - $this->assertCount(0, $DB->get_records('post', - array('id' => $assocblog->id))); - - // Verify the tag instance record has been deleted. - $this->assertCount(0, $DB->get_records('tag_instance', - array('itemid' => $assocblog->id))); - - // Test clean up of module specific messes. - switch ($type) { - case 'assign': - // Verify event assignment events have been removed. - $eventcount = $DB->count_records('event', array('instance' => $module->id, 'modulename' => $type)); - $this->assertEmpty($eventcount); - - // Verify the tag instances were deleted. - $criteria = array('component' => 'mod_assign', 'contextid' => $modcontext->id); - $this->assertEquals(0, $DB->count_records('tag_instance', $criteria)); - - $criteria = array('component' => 'core', 'itemtype' => 'course_modules', 'contextid' => $modcontext->id); - $this->assertEquals(0, $DB->count_records('tag_instance', $criteria)); - break; - case 'quiz': - // Verify category deleted. - $criteria = array('contextid' => $modcontext->id); - $this->assertEquals(0, $DB->count_records('question_categories', $criteria)); - - // Verify questions deleted. - $criteria = [$qcat->id]; - $sql = 'SELECT COUNT(q.id) - FROM {question} q - JOIN {question_versions} qv ON qv.questionid = q.id - JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid - WHERE qbe.questioncategoryid = ?'; - $this->assertEquals(0, $DB->count_records_sql($sql, $criteria)); - break; - default: - break; - } - } - /** * Test that triggering a course_created event works as expected. */ @@ -2853,31 +2728,6 @@ final class courselib_test extends advanced_testcase { } - /** - * Tests for event related to course module delete. - */ - public function test_course_module_deleted_event(): void { - global $USER, $DB; - $this->resetAfterTest(); - - // Create and delete a module. - $sink = $this->redirectEvents(); - $modinfo = $this->create_specific_module_test('forum'); - $cm = $DB->get_record('course_modules', array('id' => $modinfo->coursemodule), '*', MUST_EXIST); - course_delete_module($modinfo->coursemodule); - $events = $sink->get_events(); - $event = array_pop($events); // delete module event.; - $sink->close(); - - // Validate event data. - $this->assertInstanceOf('\core\event\course_module_deleted', $event); - $this->assertEquals($cm->id, $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', $cm->id)); - } - /** * Tests for event validations related to course module deletion. */ @@ -3954,128 +3804,6 @@ final class courselib_test extends advanced_testcase { $this->assertFalse($updates->outcomes->updated); } - public function test_async_module_deletion_hook_implemented(): void { - // 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(): void { - // 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(): void { // 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, @@ -4101,7 +3829,7 @@ final class courselib_test extends advanced_testcase { // 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); + \core_courseformat\formatactions::cm($course->id)->delete($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. @@ -7270,7 +6998,9 @@ final class courselib_test extends advanced_testcase { $moduleinstances[] = $generator->create_module($module, array('course' => $course->id)); } - course_delete_module($moduleinstances[$indextodelete]->cmid, true); // Try to delete the instance asynchronously. + // Try to delete the instance asynchronously. + \core_courseformat\formatactions::cm($course->id)->delete($moduleinstances[$indextodelete]->cmid, true); + $this->assertEquals($expected, course_modules_pending_deletion($course->id, $gradable)); } diff --git a/public/course/tests/modlib_test.php b/public/course/tests/modlib_test.php index 7a488647fa3..79d4c802f58 100644 --- a/public/course/tests/modlib_test.php +++ b/public/course/tests/modlib_test.php @@ -253,7 +253,7 @@ final class modlib_test extends \advanced_testcase { $this->assertEquals($modulescm[3]->id, $moduleinfo[2]->beforemod); // Insert before a not existing module. - course_delete_module($modulescm[2]->id); + formatactions::cm($course->id)->delete($modulescm[2]->id); list($module, $context, $cw, $cm, $data) = prepare_new_moduleinfo_data($course, $labelmodule->name, $sectionnumber); $data->beforemod = $modulescm[2]->id; diff --git a/public/customfield/field/number/tests/local/numberproviders/nofactivities/nofactivities_test.php b/public/customfield/field/number/tests/local/numberproviders/nofactivities/nofactivities_test.php index 47a862a8d95..24e14b57901 100644 --- a/public/customfield/field/number/tests/local/numberproviders/nofactivities/nofactivities_test.php +++ b/public/customfield/field/number/tests/local/numberproviders/nofactivities/nofactivities_test.php @@ -101,7 +101,7 @@ final class nofactivities_test extends advanced_testcase { // Delete some assign module. $cm = get_coursemodule_from_instance('assign', $assign1->id); - course_delete_module($cm->id); + \core_courseformat\formatactions::cm($course1->id)->delete((int)$cm->id); $providers[0]->recalculate((int)$course1->id); $course1customfield = $DB->get_field('customfield_data', 'decvalue', ['instanceid' => $course1->id]); // Module is marked as deleted. @@ -184,8 +184,8 @@ final class nofactivities_test extends advanced_testcase { $this->assertSame('2', $data->export_value()); // Delete both modules, recalculate. - course_delete_module($assign1->cmid); - course_delete_module($assign2->cmid); + \core_courseformat\formatactions::cm($course1->id)->delete($assign1->cmid); + \core_courseformat\formatactions::cm($course1->id)->delete($assign2->cmid); (new \customfield_number\task\cron())->execute(); // Field1 (displaywhenzero='0') has the value zero. $data = $getdata($field1); diff --git a/public/customfield/field/number/tests/observer_test.php b/public/customfield/field/number/tests/observer_test.php index 7dac110bec9..7ce7a5a2f26 100644 --- a/public/customfield/field/number/tests/observer_test.php +++ b/public/customfield/field/number/tests/observer_test.php @@ -192,7 +192,7 @@ final class observer_test extends \advanced_testcase { $this->assertEquals([$course1->id => 3], $alldata); // Delete one module. - course_delete_module($assign1->cmid); + \core_courseformat\formatactions::cm($course1->id)->delete($assign1->cmid); // Execute scheduled ad-hoc tasks and it will update the data for the course. $this->ensure_number_adhoc_task_is_scheduled(true); @@ -239,7 +239,7 @@ final class observer_test extends \advanced_testcase { $this->ensure_number_adhoc_task_is_scheduled(false); // Delete one module. - course_delete_module($assign1->cmid); + \core_courseformat\formatactions::cm($course1->id)->delete($assign1->cmid); $this->ensure_number_adhoc_task_is_scheduled(false); } } diff --git a/public/enrol/lti/tests/lib_test.php b/public/enrol/lti/tests/lib_test.php index 5c3636a51a1..068b22b2e41 100644 --- a/public/enrol/lti/tests/lib_test.php +++ b/public/enrol/lti/tests/lib_test.php @@ -243,7 +243,7 @@ final class lib_test extends \lti_advantage_testcase { $this->assertEquals(ENROL_INSTANCE_ENABLED, $mod2instance->status); // Delete a module and verify the associated instance is disabled. - course_delete_module($mod->cmid); + \core_courseformat\formatactions::cm($course->id)->delete($mod->cmid); $modinstance = helper::get_lti_tool($tool->id); $mod2instance = helper::get_lti_tool($tool2->id); $this->assertEquals(ENROL_INSTANCE_DISABLED, $modinstance->status); diff --git a/public/enrol/lti/tests/local/ltiadvantage/task/sync_members_test.php b/public/enrol/lti/tests/local/ltiadvantage/task/sync_members_test.php index 526b393f2de..49065b594eb 100644 --- a/public/enrol/lti/tests/local/ltiadvantage/task/sync_members_test.php +++ b/public/enrol/lti/tests/local/ltiadvantage/task/sync_members_test.php @@ -634,7 +634,7 @@ final class sync_members_test extends \lti_advantage_testcase { // Delete the activity being shared by resource2, leaving resource 2 disabled as a result. $modcontext = \context::instance_by_id($resource2->contextid); - course_delete_module($modcontext->instanceid); + \core_courseformat\formatactions::cm($course->id)->delete($modcontext->instanceid); // Only the enabled resource 3 should sync members. $task = $this->get_mock_task_with_users(self::get_mock_members_with_ids(range(1, 1))); diff --git a/public/enrol/lti/tests/local/ltiadvantage/task/sync_tool_grades_test.php b/public/enrol/lti/tests/local/ltiadvantage/task/sync_tool_grades_test.php index db9c2ce2ae2..98ac824c807 100644 --- a/public/enrol/lti/tests/local/ltiadvantage/task/sync_tool_grades_test.php +++ b/public/enrol/lti/tests/local/ltiadvantage/task/sync_tool_grades_test.php @@ -432,7 +432,7 @@ final class sync_tool_grades_test extends \lti_advantage_testcase { // Delete the activity being shared by resource 2, leaving resource 2 disabled as a result. $modcontext = \context::instance_by_id($resource2->contextid); - course_delete_module($modcontext->instanceid); + \core_courseformat\formatactions::cm($course->id)->delete($modcontext->instanceid); // Only the enabled resource 3 should sync grades. $task = $this->get_task_with_mocked_grade_service(); @@ -464,7 +464,7 @@ final class sync_tool_grades_test extends \lti_advantage_testcase { // Delete the activity, then enable the enrolment method (it is disabled during activity deletion). $modcontext = \context::instance_by_id($resource->contextid); - course_delete_module($modcontext->instanceid); + \core_courseformat\formatactions::cm($course->id)->delete($modcontext->instanceid); $enrol = (object) ['id' => $resource->enrolid, 'status' => ENROL_INSTANCE_ENABLED]; $DB->update_record('enrol', $enrol); diff --git a/public/lib/moodlelib.php b/public/lib/moodlelib.php index 102e4afe6ac..727f687482f 100644 --- a/public/lib/moodlelib.php +++ b/public/lib/moodlelib.php @@ -4853,7 +4853,7 @@ function remove_course_contents($courseid, $showfeedback = true, ?array $options if ($instances) { foreach ($instances as $cm) { - // Warning! there is very similar code in course_delete_module. + // Warning! there is very similar code in cmactions::delete. // If you are changing this code, you probably need to change that too. if (function_exists($moddelete)) { // This purges all module data in related tables, extra user prefs, settings, etc. diff --git a/public/lib/tests/event/grade_deleted_test.php b/public/lib/tests/event/grade_deleted_test.php index bcd1f77590c..39661610560 100644 --- a/public/lib/tests/event/grade_deleted_test.php +++ b/public/lib/tests/event/grade_deleted_test.php @@ -58,7 +58,7 @@ final class grade_deleted_test extends \advanced_testcase { // Trigger the event. $sink = $this->redirectEvents(); - course_delete_module($quiz->cmid); + \core_courseformat\formatactions::cm($course->id)->delete($quiz->cmid); $events = $sink->get_events(); $event = $events[1]; $sink->close(); diff --git a/public/lib/tests/gradelib_test.php b/public/lib/tests/gradelib_test.php index 3d20c10f5e2..f0080ebaa83 100644 --- a/public/lib/tests/gradelib_test.php +++ b/public/lib/tests/gradelib_test.php @@ -393,7 +393,7 @@ final class gradelib_test extends \advanced_testcase { $g1->set_calculation('=[[a1]] + [[q1]]'); // Now delete one of the activities to break the calculation. - course_delete_module($assign->cmid); + \core_courseformat\formatactions::cm($course->id)->delete($assign->cmid); // Course grade item has needsupdate. $this->assertEquals(1, \grade_item::fetch_course_item($course->id)->needsupdate); diff --git a/public/lib/tests/h5p_clean_orphaned_records_task_test.php b/public/lib/tests/h5p_clean_orphaned_records_task_test.php index 066d423e140..320f3268b3d 100644 --- a/public/lib/tests/h5p_clean_orphaned_records_task_test.php +++ b/public/lib/tests/h5p_clean_orphaned_records_task_test.php @@ -55,7 +55,7 @@ final class h5p_clean_orphaned_records_task_test extends advanced_testcase { 'mod_h5pactivity', 'package'); // Delete activity. - course_delete_module($activity->cmid); + \core_courseformat\formatactions::cm($course->id)->delete($activity->cmid); $orphanedh5psql = "SELECT h5p.id, h5p.pathnamehash FROM {h5p} h5p diff --git a/public/mod/bigbluebuttonbn/tests/backup_restore_test.php b/public/mod/bigbluebuttonbn/tests/backup_restore_test.php index 2d666fffe12..93545265072 100644 --- a/public/mod/bigbluebuttonbn/tests/backup_restore_test.php +++ b/public/mod/bigbluebuttonbn/tests/backup_restore_test.php @@ -204,7 +204,7 @@ final class backup_restore_test extends restore_date_testcase { list($bbactivitycontext, $bbactivitycm, $bbactivity) = $this->create_instance($this->get_course(), ['type' => $type]); // Delete the course module. - course_delete_module($bbactivitycm->id); + \core_courseformat\formatactions::cm($this->course->id)->delete($bbactivitycm->id); // Now, run the course module deletion adhoc task. \phpunit_util::run_all_adhoc_tasks(); $currentinstances = instance::get_all_instances_in_course($this->course->id); diff --git a/public/mod/bigbluebuttonbn/tests/local/extension_test.php b/public/mod/bigbluebuttonbn/tests/local/extension_test.php index 37fbf54c495..9116de65b66 100644 --- a/public/mod/bigbluebuttonbn/tests/local/extension_test.php +++ b/public/mod/bigbluebuttonbn/tests/local/extension_test.php @@ -144,7 +144,7 @@ final class extension_test extends \advanced_testcase { $course = $this->getDataGenerator()->create_course(); $record = $this->getDataGenerator()->create_module('bigbluebuttonbn', ['course' => $course->id, 'newfield' => 2]); $cm = get_fast_modinfo($course)->instances['bigbluebuttonbn'][$record->id]; - course_delete_module($cm->id, false); + \core_courseformat\formatactions::cm($course->id)->delete($cm->id); $this->assertFalse($DB->get_field('bbbext_simple', 'newfield', ['bigbluebuttonbnid' => $record->id])); } diff --git a/public/mod/bigbluebuttonbn/tests/task/upgrade_recordings_task_test.php b/public/mod/bigbluebuttonbn/tests/task/upgrade_recordings_task_test.php index 0fa067750fc..87bde2edafc 100644 --- a/public/mod/bigbluebuttonbn/tests/task/upgrade_recordings_task_test.php +++ b/public/mod/bigbluebuttonbn/tests/task/upgrade_recordings_task_test.php @@ -300,7 +300,7 @@ final class upgrade_recordings_task_test extends advanced_testcase { $this->create_log_entries($groupinstance, $teacher->id, 15, $importedrecording); } $this->create_log_entries($deletedinstance, $teacher->id, 15, $importedrecording); - course_delete_module($deletedinstance->get_cm_id()); + \core_courseformat\formatactions::cm($this->course->id)->delete($deletedinstance->get_cm_id()); // Truncate the recordings table to reflect what it would have looked like before this version. $DB->delete_records('bigbluebuttonbn_recordings'); $this->groups = $groups; diff --git a/public/mod/lti/service/gradebookservices/tests/task/cleanup_test.php b/public/mod/lti/service/gradebookservices/tests/task/cleanup_test.php index c5345d2acbf..9e9ec7b5c9d 100644 --- a/public/mod/lti/service/gradebookservices/tests/task/cleanup_test.php +++ b/public/mod/lti/service/gradebookservices/tests/task/cleanup_test.php @@ -78,7 +78,7 @@ final class cleanup_test extends \advanced_testcase { $this->assertEquals(2, $DB->count_records('ltiservice_gradebookservices')); // Delete the first LTI activity. - course_delete_module($lti->cmid); + \core_courseformat\formatactions::cm($course->id)->delete($lti->cmid); // Run the task again. $task = new cleanup_task(); diff --git a/public/mod/lti/tests/lib_test.php b/public/mod/lti/tests/lib_test.php index c8044a1d2fa..35d6e606c60 100644 --- a/public/mod/lti/tests/lib_test.php +++ b/public/mod/lti/tests/lib_test.php @@ -103,7 +103,7 @@ final class lib_test extends \advanced_testcase { $cm = get_coursemodule_from_instance('lti', $lti->id); // Must not throw notices. - course_delete_module($cm->id); + \core_courseformat\formatactions::cm($course->id)->delete($cm->id); } public function test_lti_core_calendar_provide_event_action(): void { diff --git a/public/question/tests/local/bank/question_bank_helper_test.php b/public/question/tests/local/bank/question_bank_helper_test.php index dbf363caae8..83b692209df 100644 --- a/public/question/tests/local/bank/question_bank_helper_test.php +++ b/public/question/tests/local/bank/question_bank_helper_test.php @@ -442,7 +442,7 @@ final class question_bank_helper_test extends \advanced_testcase { } // Now delete one of the viewed bank modules and get the records again. - course_delete_module($banks[2]->cmid); + \core_courseformat\formatactions::cm($course1->id)->delete($banks[2]->cmid); $recentlyviewed = question_bank_helper::get_recently_used_open_banks($user->id); $this->assertCount(4, $recentlyviewed); diff --git a/public/search/engine/simpledb/tests/engine_test.php b/public/search/engine/simpledb/tests/engine_test.php index 8650384d1ee..ea67f63d6aa 100644 --- a/public/search/engine/simpledb/tests/engine_test.php +++ b/public/search/engine/simpledb/tests/engine_test.php @@ -364,7 +364,7 @@ final class engine_test extends \advanced_testcase { $this->assert_raw_index_contents('xyzzy', ['C1', 'C1P1', 'C1P2']); // Finally let's delete using Moodle functions to check that works. Single context first. - course_delete_module($course1page1->cmid); + \core_courseformat\formatactions::cm($course1->id)->delete($course1page1->cmid); $this->assert_raw_index_contents('xyzzy', ['C1', 'C1P2']); delete_course($course1, false); $this->assert_raw_index_contents('xyzzy', []); diff --git a/public/search/engine/solr/tests/engine_test.php b/public/search/engine/solr/tests/engine_test.php index 26951ebc1b8..91d16916665 100644 --- a/public/search/engine/solr/tests/engine_test.php +++ b/public/search/engine/solr/tests/engine_test.php @@ -1323,7 +1323,7 @@ final class engine_test extends \advanced_testcase { $this->assert_raw_solr_query_result('content:xyzzy', ['C1', 'C1P1', 'C1P2']); // Finally let's delete using Moodle functions to check that works. Single context first. - course_delete_module($course1page1->cmid); + \core_courseformat\formatactions::cm($course1->id)->delete($course1page1->cmid); $this->assert_raw_solr_query_result('content:xyzzy', ['C1', 'C1P2']); delete_course($course1, false); $this->assert_raw_solr_query_result('content:xyzzy', []); diff --git a/public/search/tests/base_activity_test.php b/public/search/tests/base_activity_test.php index 8c7bf1044f1..677b5f99890 100644 --- a/public/search/tests/base_activity_test.php +++ b/public/search/tests/base_activity_test.php @@ -313,7 +313,7 @@ final class base_activity_test extends \advanced_testcase { $this->setUser($user); // Delete forum 2 and set forum 3 hidden. - course_delete_module($this->forums[2]->cmid); + \core_courseformat\formatactions::cm($this->contexts['c1']->instanceid)->delete($this->forums[2]->cmid); set_coursemodule_visible($this->forums[3]->cmid, 0); // Call check access on all the first three. diff --git a/public/search/tests/manager_test.php b/public/search/tests/manager_test.php index d4ada2a2967..411931f76fe 100644 --- a/public/search/tests/manager_test.php +++ b/public/search/tests/manager_test.php @@ -1603,8 +1603,8 @@ final class manager_test extends \advanced_testcase { $search = \testable_core_search::instance(); // Delete two of the pages individually. - course_delete_module($page1->cmid); - course_delete_module($page3->cmid); + \core_courseformat\formatactions::cm($course1->id)->delete($page1->cmid); + \core_courseformat\formatactions::cm($course1->id)->delete($page3->cmid); // Delete the course with another two. delete_course($course1->id, false); @@ -1613,7 +1613,7 @@ final class manager_test extends \advanced_testcase { delete_user($user); // Delete the page from the other course. - course_delete_module($page5->cmid); + \core_courseformat\formatactions::cm($course2->id)->delete($page5->cmid); // It should have deleted the contexts and the course, but not the contexts in the course. $expected = [