From e2cd526456069fb0438f39ab35237b36cabc47e7 Mon Sep 17 00:00:00 2001 From: Simey Lameze Date: Thu, 7 Jul 2022 10:36:52 +0800 Subject: [PATCH 1/3] MDL-74925 phpunit: add missing course/format test suite --- phpunit.xml.dist | 3 +++ 1 file changed, 3 insertions(+) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index f2ef3acee87..34dd79c3558 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -103,6 +103,9 @@ course/tests + + course/format/tests + privacy/tests From 45dfb5ac84f7bd7fc8abd7363386b08f5272fd72 Mon Sep 17 00:00:00 2001 From: Simey Lameze Date: Thu, 7 Jul 2022 10:37:46 +0800 Subject: [PATCH 2/3] MDL-74925 course: use the correct 'remove' action --- course/format/classes/stateactions.php | 4 ++-- course/format/classes/stateupdates.php | 20 ++++++++++++++++++- .../tests/external/update_course_test.php | 2 +- .../format_theunittest_stateactions.php | 2 +- course/format/tests/stateactions_test.php | 6 +++--- course/format/tests/stateupdates_test.php | 14 ++++++------- 6 files changed, 33 insertions(+), 15 deletions(-) diff --git a/course/format/classes/stateactions.php b/course/format/classes/stateactions.php index 935c6e6690b..31c3b0968ff 100644 --- a/course/format/classes/stateactions.php +++ b/course/format/classes/stateactions.php @@ -247,11 +247,11 @@ class stateactions { if (!empty($modinfo->sections[$section->section])) { foreach ($modinfo->sections[$section->section] as $modnumber) { $cm = $modinfo->cms[$modnumber]; - $updates->add_cm_delete($cm->id); + $updates->add_cm_remove($cm->id); } } course_delete_section($course, $section, true, true); - $updates->add_section_delete($sectionid); + $updates->add_section_remove($sectionid); } // Removing a section affects the full course structure. diff --git a/course/format/classes/stateupdates.php b/course/format/classes/stateupdates.php index a3921da0082..e66931f3848 100644 --- a/course/format/classes/stateupdates.php +++ b/course/format/classes/stateupdates.php @@ -134,6 +134,15 @@ class stateupdates implements JsonSerializable { $this->add_update('section', 'remove', (object)['id' => $sectionid]); } + /** + * Add track about a section removed. + * + * @param int $sectionid The affected section id. + */ + public function add_section_remove(int $sectionid): void { + $this->add_update('section', 'remove', (object)['id' => $sectionid]); + } + /** * Add track about a course module state update. * @@ -184,11 +193,20 @@ class stateupdates implements JsonSerializable { $this->add_update('cm', 'remove', (object)['id' => $cmid]); } + /** + * Add track about a course module removed. + * + * @param int $cmid the affected course module id + */ + public function add_cm_remove(int $cmid): void { + $this->add_update('cm', 'remove', (object)['id' => $cmid]); + } + /** * Add a valid update message to the update list. * * @param string $name the update name - * @param string $action the update action (usually update, create, delete) + * @param string $action the update action (usually update, create, remove) * @param stdClass $fields the object fields */ protected function add_update(string $name, string $action, stdClass $fields): void { diff --git a/course/format/tests/external/update_course_test.php b/course/format/tests/external/update_course_test.php index 355e14923ba..56d64f889bd 100644 --- a/course/format/tests/external/update_course_test.php +++ b/course/format/tests/external/update_course_test.php @@ -133,7 +133,7 @@ class update_course_test extends \externallib_advanced_testcase { 'action' => 'format_do_something', 'expected' => [ 'count' => 1, - 'action' => 'delete', + 'action' => 'remove', 'visible' => null, ], 'expectexception' => false, diff --git a/course/format/tests/fixtures/format_theunittest_stateactions.php b/course/format/tests/fixtures/format_theunittest_stateactions.php index 207f83acfb0..379fd929f64 100644 --- a/course/format/tests/fixtures/format_theunittest_stateactions.php +++ b/course/format/tests/fixtures/format_theunittest_stateactions.php @@ -65,7 +65,7 @@ class stateactions extends core_actions { ?int $targetcmid = null ): void { - $updates->add_cm_delete(array_pop($ids)); + $updates->add_cm_remove(array_pop($ids)); } /** diff --git a/course/format/tests/stateactions_test.php b/course/format/tests/stateactions_test.php index 56c1f14c057..b0733c9885b 100644 --- a/course/format/tests/stateactions_test.php +++ b/course/format/tests/stateactions_test.php @@ -143,7 +143,7 @@ class stateactions_test extends \advanced_testcase { 'cm' => [], 'count' => 0, ], - 'delete' => [ + 'remove' => [ 'course' => [], 'section' => [], 'cm' => [], @@ -236,10 +236,10 @@ class stateactions_test extends \advanced_testcase { // Format results in a way we can compare easily. $results = $this->summarize_updates($updates); - // The state actions does not use create or delete actions because they are designed + // The state actions does not use create or remove actions because they are designed // to refresh parts of the state. $this->assertEquals(0, $results['create']['count']); - $this->assertEquals(0, $results['delete']['count']); + $this->assertEquals(0, $results['remove']['count']); // Validate we have all the expected entries. $expectedtotal = count($expectedresults['course']) + count($expectedresults['section']) + count($expectedresults['cm']); diff --git a/course/format/tests/stateupdates_test.php b/course/format/tests/stateupdates_test.php index d3eca691e60..e15f6415363 100644 --- a/course/format/tests/stateupdates_test.php +++ b/course/format/tests/stateupdates_test.php @@ -186,7 +186,7 @@ class stateupdates_test extends \advanced_testcase { $this->assertEquals($action, $update->action); $this->assertEquals('section', $update->name); // Delete does not provide all fields. - if ($action == 'delete') { + if ($action == 'remove') { $this->assertEquals($section->id, $update->fields->id); } else { $this->assertEquals($expected, $update->fields); @@ -203,7 +203,7 @@ class stateupdates_test extends \advanced_testcase { return array_merge( $this->add_section_provider_helper('put'), $this->add_section_provider_helper('create'), - $this->add_section_provider_helper('delete'), + $this->add_section_provider_helper('remove'), ); } @@ -215,7 +215,7 @@ class stateupdates_test extends \advanced_testcase { */ private function add_section_provider_helper(string $action): array { // Delete does not depends on user permissions. - if ($action == 'delete') { + if ($action == 'remove') { $studentsections = [0, 1, 2]; } else { $studentsections = [0, 1]; @@ -247,7 +247,7 @@ class stateupdates_test extends \advanced_testcase { * @dataProvider add_cm_provider * @covers ::add_cm_update * @covers ::add_cm_create - * @covers ::add_cm_delete + * @covers ::add_cm_remove * * @param string $action the action name * @param string $role the user role name @@ -330,7 +330,7 @@ class stateupdates_test extends \advanced_testcase { $this->assertEquals($action, $update->action); $this->assertEquals('cm', $update->name); // Delete does not provide all fields. - if ($action == 'delete') { + if ($action == 'remove') { $this->assertEquals($cm->id, $update->fields->id); } else { $this->assertEquals($expected, $update->fields); @@ -347,7 +347,7 @@ class stateupdates_test extends \advanced_testcase { return array_merge( $this->add_cm_provider_helper('put'), $this->add_cm_provider_helper('create'), - $this->add_cm_provider_helper('delete'), + $this->add_cm_provider_helper('remove'), ); } @@ -359,7 +359,7 @@ class stateupdates_test extends \advanced_testcase { */ private function add_cm_provider_helper(string $action): array { // Delete does not depends on user permissions. - if ($action == 'delete') { + if ($action == 'remove') { $studentcms = [0, 1, 2, 3]; } else { $studentcms = [0]; From 5294f81c27b1e1bb580ca4ff1ff0d08603e0ee37 Mon Sep 17 00:00:00 2001 From: Simey Lameze Date: Wed, 13 Jul 2022 12:07:27 +0800 Subject: [PATCH 3/3] MDL-74925 core_course: initial deprecation of '_delete' methods --- course/format/classes/stateupdates.php | 6 ++++++ course/format/upgrade.txt | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/course/format/classes/stateupdates.php b/course/format/classes/stateupdates.php index e66931f3848..a1b095bf74c 100644 --- a/course/format/classes/stateupdates.php +++ b/course/format/classes/stateupdates.php @@ -128,9 +128,12 @@ class stateupdates implements JsonSerializable { /** * Add track about a section deleted. * + * @deprecated since Moodle 4.1 MDL-74925 - please call add_section_remove() instead. * @param int $sectionid The affected section id. */ public function add_section_delete(int $sectionid): void { + debugging('add_section_delete() is deprecated. Please use add_section_remove() instead.', DEBUG_DEVELOPER); + $this->add_update('section', 'remove', (object)['id' => $sectionid]); } @@ -187,9 +190,12 @@ class stateupdates implements JsonSerializable { /** * Add track about a course module deleted. * + * @deprecated since Moodle 4.1 MDL-74925 - please call add_cm_remove() instead. * @param int $cmid the affected course module id */ public function add_cm_delete(int $cmid): void { + debugging('add_cm_delete() is deprecated. Please use add_cm_remove() instead.', DEBUG_DEVELOPER); + $this->add_update('cm', 'remove', (object)['id' => $cmid]); } diff --git a/course/format/upgrade.txt b/course/format/upgrade.txt index 8707452361c..e284e13ba24 100644 --- a/course/format/upgrade.txt +++ b/course/format/upgrade.txt @@ -2,6 +2,10 @@ This files describes API changes for course formats Overview of this plugin type at http://docs.moodle.org/dev/Course_formats +=== 4.1 === +* New \core_courseformat\stateupdates methods add_section_remove() and add_cm_remove() have been added to replace + the deprecated methods add_section_delete() and add_cm_delete(). + === 4.0 === * New core_courseformat\base::uses_course_index() to define whether the course format uses course index or not. * New core_courseformat\base::supports_components() to specify if the format is compatible with reactive components.