diff --git a/course/lib.php b/course/lib.php index c3a4165df14..8626b427934 100644 --- 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 c96441491fb..a59351b4485 100644 --- 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"); + } + } + } + }