Merge branch 'MDL-36417-master' of git://github.com/FMCorz/moodle

diff --git a/course/lib.php b/course/lib.php
index c3a4165..8626b42 100644 (file)
--- a/course/lib.php
+++ b/course/lib.php
@@ -1272,7 +1272,16 @@ function set_section_visible($courseid, $sectionnumber, $visibility) {
         if (!empty($section->sequence)) {
             $modules = explode(",", $section->sequence);
             foreach ($modules as $moduleid) {
-                set_coursemodule_visible($moduleid, $visibility, true);
+                if ($cm = $DB->get_record('course_modules', array('id' => $moduleid), 'visible, visibleold')) {
+                    if ($visibility) {
+                        // As we unhide the section, we use the previously saved visibility stored in visibleold.
+                        set_coursemodule_visible($moduleid, $cm->visibleold);
+                    } else {
+                        // We hide the section, so we hide the module but we store the original state in visibleold.
+                        set_coursemodule_visible($moduleid, 0);
+                        $DB->set_field('course_modules', 'visibleold', $cm->visible, array('id' => $moduleid));
+                    }
+                }
             }
         }
         rebuild_course_cache($courseid, true);
@@ -2805,16 +2814,26 @@ function set_coursemodule_idnumber($id, $idnumber) {
 }
 
 /**
-* $prevstateoverrides = true will set the visibility of the course module
-* to what is defined in visibleold. This enables us to remember the current
-* visibility when making a whole section hidden, so that when we toggle
-* that section back to visible, we are able to return the visibility of
-* the course module back to what it was originally.
-*/
-function set_coursemodule_visible($id, $visible, $prevstateoverrides=false) {
+ * Set the visibility of a module and inherent properties.
+ *
+ * From 2.4 the parameter $prevstateoverrides has been removed, the logic it triggered
+ * has been moved to {@link set_section_visible()} which was the only place from which
+ * the parameter was used.
+ *
+ * @param int $id of the module
+ * @param int $visible state of the module
+ * @return bool false when the module was not found, true otherwise
+ */
+function set_coursemodule_visible($id, $visible) {
     global $DB, $CFG;
     require_once($CFG->libdir.'/gradelib.php');
 
+    // Trigger developer's attention when using the previously removed argument.
+    if (func_num_args() > 2) {
+        debugging('Wrong number of arguments passed to set_coursemodule_visible(), $prevstateoverrides
+            has been removed.', DEBUG_DEVELOPER);
+    }
+
     if (!$cm = $DB->get_record('course_modules', array('id'=>$id))) {
         return false;
     }
@@ -2839,17 +2858,14 @@ function set_coursemodule_visible($id, $visible, $prevstateoverrides=false) {
         }
     }
 
-    if ($prevstateoverrides) {
-        if ($visible == '0') {
-            // Remember the current visible state so we can toggle this back.
-            $DB->set_field('course_modules', 'visibleold', $cm->visible, array('id'=>$id));
-        } else {
-            // Get the previous saved visible states.
-            $DB->set_field('course_modules', 'visible', $cm->visibleold, array('id'=>$id));
-        }
-    } else {
-        $DB->set_field("course_modules", "visible", $visible, array("id"=>$id));
-    }
+    // Updating visible and visibleold to keep them in sync. Only changing a section visibility will
+    // affect visibleold to allow for an original visibility restore. See set_section_visible().
+    $cminfo = new stdClass();
+    $cminfo->id = $id;
+    $cminfo->visible = $visible;
+    $cminfo->visibleold = $visible;
+    $DB->update_record('course_modules', $cminfo);
+
     rebuild_course_cache($cm->course, true);
     return true;
 }

diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php
index c964414..a59351b 100644 (file)
--- a/course/tests/courselib_test.php
+++ b/course/tests/courselib_test.php
@@ -314,4 +314,125 @@ class courselib_testcase extends advanced_testcase {
         $this->assertTrue(empty($modinfo->sections[0]));
         $this->assertFalse(empty($modinfo->sections[3]));
     }
+
+    public function test_module_visibility() {
+        $this->setAdminUser();
+        $this->resetAfterTest(true);
+
+        // Create course and modules.
+        $course = $this->getDataGenerator()->create_course(array('numsections' => 5));
+        $forum = $this->getDataGenerator()->create_module('forum', array('course' => $course->id));
+        $assign = $this->getDataGenerator()->create_module('assign', array('duedate' => time(), 'course' => $course->id));
+        $modules = compact('forum', 'assign');
+
+        // Hiding the modules.
+        foreach ($modules as $mod) {
+            set_coursemodule_visible($mod->cmid, 0);
+            $this->check_module_visibility($mod, 0, 0);
+        }
+
+        // Showing the modules.
+        foreach ($modules as $mod) {
+            set_coursemodule_visible($mod->cmid, 1);
+            $this->check_module_visibility($mod, 1, 1);
+        }
+    }
+
+    public function test_section_visibility() {
+        $this->setAdminUser();
+        $this->resetAfterTest(true);
+
+        // Create course.
+        $course = $this->getDataGenerator()->create_course(array('numsections' => 3), array('createsections' => true));
+
+        // Testing an empty section.
+        $sectionnumber = 1;
+        set_section_visible($course->id, $sectionnumber, 0);
+        $section_info = get_fast_modinfo($course->id)->get_section_info($sectionnumber);
+        $this->assertEquals($section_info->visible, 0);
+        set_section_visible($course->id, $sectionnumber, 1);
+        $section_info = get_fast_modinfo($course->id)->get_section_info($sectionnumber);
+        $this->assertEquals($section_info->visible, 1);
+
+        // Testing a section with visible modules.
+        $sectionnumber = 2;
+        $forum = $this->getDataGenerator()->create_module('forum', array('course' => $course->id),
+                array('section' => $sectionnumber));
+        $assign = $this->getDataGenerator()->create_module('assign', array('duedate' => time(),
+                'course' => $course->id), array('section' => $sectionnumber));
+        $modules = compact('forum', 'assign');
+        set_section_visible($course->id, $sectionnumber, 0);
+        $section_info = get_fast_modinfo($course->id)->get_section_info($sectionnumber);
+        $this->assertEquals($section_info->visible, 0);
+        foreach ($modules as $mod) {
+            $this->check_module_visibility($mod, 0, 1);
+        }
+        set_section_visible($course->id, $sectionnumber, 1);
+        $section_info = get_fast_modinfo($course->id)->get_section_info($sectionnumber);
+        $this->assertEquals($section_info->visible, 1);
+        foreach ($modules as $mod) {
+            $this->check_module_visibility($mod, 1, 1);
+        }
+
+        // Testing a section with hidden modules, which should stay hidden.
+        $sectionnumber = 3;
+        $forum = $this->getDataGenerator()->create_module('forum', array('course' => $course->id),
+                array('section' => $sectionnumber));
+        $assign = $this->getDataGenerator()->create_module('assign', array('duedate' => time(),
+                'course' => $course->id), array('section' => $sectionnumber));
+        $modules = compact('forum', 'assign');
+        foreach ($modules as $mod) {
+            set_coursemodule_visible($mod->cmid, 0);
+            $this->check_module_visibility($mod, 0, 0);
+        }
+        set_section_visible($course->id, $sectionnumber, 0);
+        $section_info = get_fast_modinfo($course->id)->get_section_info($sectionnumber);
+        $this->assertEquals($section_info->visible, 0);
+        foreach ($modules as $mod) {
+            $this->check_module_visibility($mod, 0, 0);
+        }
+        set_section_visible($course->id, $sectionnumber, 1);
+        $section_info = get_fast_modinfo($course->id)->get_section_info($sectionnumber);
+        $this->assertEquals($section_info->visible, 1);
+        foreach ($modules as $mod) {
+            $this->check_module_visibility($mod, 0, 0);
+        }
+    }
+
+    /**
+     * Helper function to assert that a module has correctly been made visible, or hidden.
+     *
+     * @param stdClass $mod module information
+     * @param int $visibility the current state of the module
+     * @param int $visibleold the current state of the visibleold property
+     * @return void
+     */
+    public function check_module_visibility($mod, $visibility, $visibleold) {
+        global $DB;
+        $cm = get_fast_modinfo($mod->course)->get_cm($mod->cmid);
+        $this->assertEquals($visibility, $cm->visible);
+        $this->assertEquals($visibleold, $cm->visibleold);
+
+        // Check the module grade items.
+        $grade_items = grade_item::fetch_all(array('itemtype' => 'mod', 'itemmodule' => $cm->modname,
+                'iteminstance' => $cm->instance, 'courseid' => $cm->course));
+        if ($grade_items) {
+            foreach ($grade_items as $grade_item) {
+                if ($visibility) {
+                    $this->assertFalse($grade_item->is_hidden(), "$cm->modname grade_item not visible");
+                } else {
+                    $this->assertTrue($grade_item->is_hidden(), "$cm->modname grade_item not hidden");
+                }
+            }
+        }
+
+        // Check the events visibility.
+        if ($events = $DB->get_records('event', array('instance' => $cm->instance, 'modulename' => $cm->modname))) {
+            foreach ($events as $event) {
+                $calevent = new calendar_event($event);
+                $this->assertEquals($visibility, $calevent->visible, "$cm->modname calendar_event visibility");
+            }
+        }
+    }
+
 }