diff --git a/course/lib.php b/course/lib.php index e1e12409f75..a0b5f5a3f4a 100644 --- a/course/lib.php +++ b/course/lib.php @@ -1736,35 +1736,44 @@ function reorder_sections($sections, $origin_position, $target_position) { * Move the module object $mod to the specified $section * If $beforemod exists then that is the module * before which $modid should be inserted - * All parameters are objects + * + * @param stdClass|cm_info $mod + * @param stdClass|section_info $section + * @param int|stdClass $beforemod id or object with field id corresponding to the module + * before which the module needs to be included. Null for inserting in the + * end of the section + * @return int new value for module visibility (0 or 1) */ function moveto_module($mod, $section, $beforemod=NULL) { global $OUTPUT, $DB; -/// Remove original module from original section + // Current module visibility state - return value of this function. + $modvisible = $mod->visible; + + // Remove original module from original section. if (! delete_mod_from_section($mod->id, $mod->section)) { echo $OUTPUT->notification("Could not delete module from existing section"); } - // if moving to a hidden section then hide module + // If moving to a hidden section then hide module. if ($mod->section != $section->id) { if (!$section->visible && $mod->visible) { - // Set this in the object because it is sent as a response to ajax calls. - $mod->visible = 0; + // Module was visible but must become hidden after moving to hidden section. + $modvisible = 0; set_coursemodule_visible($mod->id, 0); // Set visibleold to 1 so module will be visible when section is made visible. $DB->set_field('course_modules', 'visibleold', 1, array('id' => $mod->id)); } if ($section->visible && !$mod->visible) { + // Hidden module was moved to the visible section, restore the module visibility from visibleold. set_coursemodule_visible($mod->id, $mod->visibleold); - // Set this in the object because it is sent as a response to ajax calls. - $mod->visible = $mod->visibleold; + $modvisible = $mod->visibleold; } } -/// Add the module into the new section + // Add the module into the new section. course_add_cm_to_section($section->course, $mod->id, $section->section, $beforemod); - return true; + return $modvisible; } /** diff --git a/course/rest.php b/course/rest.php index 4bae0b0e596..79a1152b0b1 100644 --- a/course/rest.php +++ b/course/rest.php @@ -133,8 +133,8 @@ switch($requestmethod) { $beforemod = NULL; } - moveto_module($cm, $section, $beforemod); - echo json_encode(array('visible' => $cm->visible)); + $isvisible = moveto_module($cm, $section, $beforemod); + echo json_encode(array('visible' => $isvisible)); break; case 'gettitle': require_capability('moodle/course:manageactivities', $modcontext); diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index 01c4bfc697c..28837fc76b2 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -856,8 +856,6 @@ class core_course_courselib_testcase extends advanced_testcase { // Perform the move moveto_module($cm, $newsection); - // reset of get_fast_modinfo is usually called the code calling moveto_module so call it here - get_fast_modinfo(0, 0, true); $cms = get_fast_modinfo($course)->get_cms(); $cm = reset($cms); @@ -883,11 +881,8 @@ class core_course_courselib_testcase extends advanced_testcase { // Perform a second move as some issues were only seen on the second move $newsection = get_fast_modinfo($course)->get_section_info(2); $oldsectionid = $cm->section; - $result = moveto_module($cm, $newsection); - $this->assertTrue($result); + moveto_module($cm, $newsection); - // reset of get_fast_modinfo is usually called the code calling moveto_module so call it here - get_fast_modinfo(0, 0, true); $cms = get_fast_modinfo($course)->get_cms(); $cm = reset($cms); @@ -1197,12 +1192,9 @@ class core_course_courselib_testcase extends advanced_testcase { $forumcm = $modinfo->cms[$forum->cmid]; $pagecm = $modinfo->cms[$page->cmid]; - // Move the forum and the page to a hidden section. - moveto_module($forumcm, $hiddensection); - moveto_module($pagecm, $hiddensection); - - // Reset modinfo cache. - get_fast_modinfo(0, 0, true); + // Move the forum and the page to a hidden section, make sure moveto_module returns 0 as new visibility state. + $this->assertEquals(0, moveto_module($forumcm, $hiddensection)); + $this->assertEquals(0, moveto_module($pagecm, $hiddensection)); $modinfo = get_fast_modinfo($course); @@ -1228,29 +1220,26 @@ class core_course_courselib_testcase extends advanced_testcase { $this->assertEquals($quizcm->visible, 1); // Move forum and page back to visible section. + // Make sure the visibility is restored to the original value (visible for forum and hidden for page). $visiblesection = $modinfo->get_section_info(2); - moveto_module($forumcm, $visiblesection); - moveto_module($pagecm, $visiblesection); + $this->assertEquals(1, moveto_module($forumcm, $visiblesection)); + $this->assertEquals(0, moveto_module($pagecm, $visiblesection)); - // Reset modinfo cache. - get_fast_modinfo(0, 0, true); $modinfo = get_fast_modinfo($course); - // Verify that forum has been made visible. + // Double check that forum has been made visible. $forumcm = $modinfo->cms[$forum->cmid]; $this->assertEquals($forumcm->visible, 1); - // Verify that page has stayed invisible. + // Double check that page has stayed invisible. $pagecm = $modinfo->cms[$page->cmid]; $this->assertEquals($pagecm->visible, 0); - // Move the page in the same section (this is what mod duplicate does_ - moveto_module($pagecm, $visiblesection, $forumcm); + // Move the page in the same section (this is what mod duplicate does). + // Visibility of page remains 0. + $this->assertEquals(0, moveto_module($pagecm, $visiblesection, $forumcm)); - // Reset modinfo cache. - get_fast_modinfo(0, 0, true); - - // Verify that the the page is still hidden + // Double check that the the page is still hidden. $modinfo = get_fast_modinfo($course); $pagecm = $modinfo->cms[$page->cmid]; $this->assertEquals($pagecm->visible, 0); @@ -1288,13 +1277,10 @@ class core_course_courselib_testcase extends advanced_testcase { $this->assertEquals($section->id, $pagecm->section); - // Move the forum and the page to a hidden section. - moveto_module($pagecm, $section, $forumcm); + // Move the page inside the hidden section. Make sure it is hidden. + $this->assertEquals(0, moveto_module($pagecm, $section, $forumcm)); - // Reset modinfo cache. - get_fast_modinfo(0, 0, true); - - // Verify that the the page is still hidden + // Double check that the the page is still hidden. $modinfo = get_fast_modinfo($course); $pagecm = $modinfo->cms[$page->cmid]; $this->assertEquals($pagecm->visible, 0); diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 113c8e253eb..dcbbbaf3b89 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -18,6 +18,7 @@ information provided here is intended especially for developers. * The ability to use an 'insecure' rc4encrypt/rc4decrypt key has been removed. * Use $CFG->debugdeveloper instead of debugging('', DEBUG_DEVELOPER). * Use set_debugging(DEBUG_xxx) when changing debugging level for current request. +* Function moveto_module() does not modify $mod argument and instead now returns the new module visibility value. DEPRECATIONS: Various previously deprecated functions have now been altered to throw DEBUG_DEVELOPER debugging notices