Skip to content

Commit 9175069

Browse files
nathan-catalystitnathan-catalystit
committed
Fix section checking
1 parent 19a4d33 commit 9175069

2 files changed

Lines changed: 113 additions & 8 deletions

File tree

‎classes/actions.php‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -270,21 +270,21 @@ public static function duplicate_to_course(array $modules, int $targetcourseid,
270270
$sourcemodinfo = get_fast_modinfo($sourcecourseid);
271271
$targetmodinfo = get_fast_modinfo($targetcourseid);
272272
$targetformat = course_get_format($targetmodinfo->get_course());
273-
$targetsectionnum = $targetformat->get_last_section_number();
273+
$lastsectionnum = $targetformat->get_last_section_number();
274274

275275
$filtersectionshook = new filter_sections_different_course($targetcourseid,
276276
array_keys($targetmodinfo->get_section_info_all()));
277277
\core\di::get(\core\hook\manager::class)->dispatch($filtersectionshook);
278278
$filteredsections = $filtersectionshook->get_sectionnums();
279279

280-
if ($targetsectionnum == -1 && !$filtersectionshook->is_originsectionkept()) {
280+
if ($sectionnum == -1 && !$filtersectionshook->is_originsectionkept()) {
281281
// The course modules should be in the same section number as in the original course. However, the hook listener(s)
282282
// disabled this option, so we cancel the operation.
283283
// This is only a security measure and should not happen unless someone manipulates the UI.
284284
return;
285285
}
286286

287-
if (!in_array($targetsectionnum, $filteredsections)) {
287+
if (($sectionnum >= 0) && ($sectionnum <= $lastsectionnum) && !in_array($sectionnum, $filteredsections)) {
288288
// The target section number has been filtered by a hook callback, thus must not be used.
289289
// This is only a security measure and should not happen unless someone manipulates the UI.
290290
return;
@@ -293,16 +293,16 @@ public static function duplicate_to_course(array $modules, int $targetcourseid,
293293
$canaddsection = has_capability('moodle/course:update', context_course::instance($targetcourseid))
294294
&& $filtersectionshook->is_makesectionallowed();
295295

296-
// If a new section (that means that $sectionnum of the user is higher than $targetsectionnum), we create one.
297-
if ($sectionnum > $targetsectionnum) {
296+
// If a new section (that means that $sectionnum of the user is higher than $lastsectionnum), we create one.
297+
if ($sectionnum > $lastsectionnum) {
298298
// No permissions to add section.
299299
if (!$canaddsection) {
300300
return;
301301
}
302302

303303
$targetformatopt = $targetformat->get_format_options();
304304
// No course format setting or no orphaned sections exist.
305-
if (!isset($targetformatopt['numsections']) || !($targetformatopt['numsections'] < $targetsectionnum)) {
305+
if (!isset($targetformatopt['numsections']) || !($targetformatopt['numsections'] < $lastsectionnum)) {
306306
course_create_section($targetcourseid);
307307
}
308308

@@ -312,7 +312,7 @@ public static function duplicate_to_course(array $modules, int $targetcourseid,
312312
}
313313

314314
// Make sure new sectionnum is set accurately.
315-
$sectionnum = $targetsectionnum + 1;
315+
$sectionnum = $lastsectionnum + 1;
316316
}
317317

318318
if ($sectionnum == -1) {
@@ -323,7 +323,7 @@ public static function duplicate_to_course(array $modules, int $targetcourseid,
323323
}, $modules));
324324

325325
// If target course needs sections added but user does not have permission.
326-
if ($srcmaxsectionnum > $targetsectionnum && !$canaddsection) {
326+
if ($srcmaxsectionnum > $lastsectionnum && !$canaddsection) {
327327
return; // No permission to add section.
328328
}
329329

‎tests/massaction_test.php‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
use base_plan_exception;
2121
use base_setting_exception;
2222
use block_massaction;
23+
use block_massaction\hook\filter_sections_different_course;
2324
use coding_exception;
25+
use core\di;
2426
use core\event\course_module_updated;
2527
use core\task\manager;
2628
use dml_exception;
@@ -860,4 +862,107 @@ private function shuffle_modules(): void {
860862
moveto_module(get_fast_modinfo($this->course->id)->get_cm(get_fast_modinfo($this->course->id)->get_sections()[3][3]),
861863
get_fast_modinfo($this->course->id)->get_section_info(3));
862864
}
865+
866+
/**
867+
* Tests the duplication of modules to a course with filter_sections hook.
868+
*
869+
* @covers \block_massaction\actions::duplicate_to_course
870+
* @return void
871+
*/
872+
public function test_duplicate_to_course_with_filter_sections_different_course_hook(): void {
873+
$this->resetAfterTest();
874+
875+
// Callback for filter_sections_different_course hook.
876+
$testcallback = function(filter_sections_different_course $hook) {
877+
foreach ($hook->get_sectionnums() as $sectionnum) {
878+
// Restrict section 3 onward.
879+
if ($sectionnum >= 3) {
880+
$hook->remove_sectionnum($sectionnum);
881+
}
882+
}
883+
884+
// Disable the options to keep the original section.
885+
$hook->disable_originsectionkept();
886+
887+
// Disable the option to create a new section.
888+
$hook->disable_makesection();
889+
};
890+
$this->redirectHook(filter_sections_different_course::class, $testcallback);
891+
892+
// Source course.
893+
$sourcecourseid = $this->course->id;
894+
$sourcecoursemodinfo = get_fast_modinfo($sourcecourseid);
895+
896+
// Move modules around so that they are not in id order.
897+
$this->shuffle_modules();
898+
899+
// Select some random course modules from different sections to be duplicated.
900+
$selectedmoduleids[] = $sourcecoursemodinfo->get_sections()[1][0];
901+
$selectedmoduleids[] = $sourcecoursemodinfo->get_sections()[1][1];
902+
$selectedmoduleids[] = $sourcecoursemodinfo->get_sections()[3][0];
903+
$selectedmoduleids[] = $sourcecoursemodinfo->get_sections()[3][2];
904+
905+
$selectedmodules = array_filter($this->get_test_course_modules(), function($module) use ($selectedmoduleids) {
906+
return in_array($module->id, $selectedmoduleids);
907+
});
908+
909+
// Target course.
910+
$targetcourseid = $this->setup_target_course_for_duplicating(3);
911+
$targetcoursemodinfo = get_fast_modinfo($targetcourseid);
912+
// Four sections (0 1 2 3).
913+
$this->assertCount(4, $targetcoursemodinfo->get_section_info_all());
914+
// There is no module.
915+
$this->assertEmpty($targetcoursemodinfo->get_cms());
916+
917+
// Test keep origin section.
918+
actions::duplicate_to_course($selectedmodules, $targetcourseid, -1);
919+
$targetcoursemodinfo = get_fast_modinfo($targetcourseid);
920+
// Four sections (0 1 2 3).
921+
$this->assertCount(4, $targetcoursemodinfo->get_section_info_all());
922+
// There is no module as we cannot keep origin section.
923+
$this->assertEmpty($targetcoursemodinfo->get_cms());
924+
925+
// Test create new section.
926+
actions::duplicate_to_course($selectedmodules, $targetcourseid, 4);
927+
$targetcoursemodinfo = get_fast_modinfo($targetcourseid);
928+
// Still four sections (0 1 2 3).
929+
$this->assertCount(4, $targetcoursemodinfo->get_section_info_all());
930+
// And still no module.
931+
$this->assertEmpty($targetcoursemodinfo->get_cms());
932+
933+
// Duplicate to section 3, which is restricted by the hook.
934+
actions::duplicate_to_course($selectedmodules, $targetcourseid, 3);
935+
$targetcoursemodinfo = get_fast_modinfo($targetcourseid);
936+
// Still four sections (0 1 2 3).
937+
$this->assertCount(4, $targetcoursemodinfo->get_section_info_all());
938+
// And still no module.
939+
$this->assertEmpty($targetcoursemodinfo->get_cms());
940+
941+
// Duplicate to section 1, this should work as normal.
942+
actions::duplicate_to_course($selectedmodules, $targetcourseid, 1);
943+
$targetcoursemodinfo = get_fast_modinfo($targetcourseid);
944+
$this->assertCount(4, $targetcoursemodinfo->get_section_info_all());
945+
// There should be 4 modules now.
946+
$this->assertCount(4, $targetcoursemodinfo->get_cms());
947+
// These module ids should be in section 1.
948+
$duplicatedmoduleids = $targetcoursemodinfo->get_sections()[1];
949+
$this->assertCount(4, $duplicatedmoduleids);
950+
951+
// Sort name of the modules to be able to compare them.
952+
$sourcemodulenames = [];
953+
foreach ($selectedmoduleids as $selectedmoduleid) {
954+
$sourcemodulenames[] = $sourcecoursemodinfo->get_cm($selectedmoduleid)->name;
955+
}
956+
sort($sourcemodulenames);
957+
958+
// Sort name of the duplicated modules to be able to compare them.
959+
$duplicatedmodulenames = [];
960+
foreach ($duplicatedmoduleids as $moduleid) {
961+
$duplicatedmodulenames[] = $targetcoursemodinfo->get_cm($moduleid)->name;
962+
}
963+
sort($duplicatedmodulenames);
964+
965+
// The names of the duplicated modules should be the same as the source module names.
966+
$this->assertEquals($sourcemodulenames, $duplicatedmodulenames);
967+
}
863968
}

0 commit comments

Comments
 (0)