MDL-41181 function moveto_module does not modify properties of object

instead it returns the new value for module visibility

diff --git a/course/lib.php b/course/lib.php
index e1e1240..a0b5f5a 100644 (file)
--- 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 4bae0b0..79a1152 100644 (file)
--- 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 5a543da..28837fc 100644 (file)
--- a/course/tests/courselib_test.php
+++ b/course/tests/courselib_test.php
@@ -881,8 +881,7 @@ 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);
 
         $cms = get_fast_modinfo($course)->get_cms();
         $cm = reset($cms);
@@ -1193,9 +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);
+        // 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);
 
@@ -1221,24 +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));
 
         $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));
 
-        // 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);
@@ -1276,10 +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));
 
-        // 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 113c8e2..dcbbbaf 100644 (file)
--- 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