MDL-27809 navigation: Fixed up a couple of minor issues after peer-review

diff --git a/lib/navigationlib.php b/lib/navigationlib.php
index b091379..3efae16 100644 (file)
--- a/lib/navigationlib.php
+++ b/lib/navigationlib.php
@@ -1354,13 +1354,6 @@ class global_navigation extends navigation_node {
             $categoryselect = '';
         }
 
-        if (count($this->addedcategories) > 0) {
-            list($courseselect, $courseparams) = $DB->get_in_or_equal(array_keys($this->addedcourses), SQL_PARAMS_NAMED, 'course', false);
-            $params += $courseparams;
-        } else {
-            $courseselect = '';
-        }
-
         list($ccselect, $ccjoin) = context_instance_preload_sql('c.id', CONTEXT_COURSE, 'ctx');
         list($courseids, $courseparams) = $DB->get_in_or_equal(array_keys($this->addedcourses) + array(SITEID), SQL_PARAMS_NAMED, 'lcourse', false);
         $sql = "SELECT c.id, c.sortorder, c.visible, c.fullname, c.shortname, c.category, cat.path AS categorypath $ccselect
@@ -1430,7 +1423,7 @@ class global_navigation extends navigation_node {
         } else {
             // This category hasn't been loaded yet so we need to fetch it, work out its category path
             // and load this category plus all its parents and subcategories
-            $category = $DB->get_record('course_categories', array('id'=>$categoryid), 'path', MUST_EXIST);
+            $category = $DB->get_record('course_categories', array('id' => $categoryid), 'path', MUST_EXIST);
             $coursestoload = explode('/', trim($category->path, '/'));
             list($select, $params) = $DB->get_in_or_equal($coursestoload);
             $select = 'id '.$select.' OR parent '.$select;
@@ -1963,9 +1956,7 @@ class global_navigation extends navigation_node {
 
             // Check the number of nodes in the report node... if there are none remove
             // the node
-            if (count($reporttab->children)===0) {
-                $usernode->remove_child($reporttab);
-            }
+            $reporttab->trim_if_empty();
         }
 
         // If the user is the current user add the repositories for the current user
@@ -2128,13 +2119,17 @@ class global_navigation extends navigation_node {
             $url = new moodle_url('/course/view.php', array('id'=>$course->id));
         }
 
-        if (!$ismycourse && !$issite && !empty($CFG->navshowcategories) && !empty($course->category)) {
-            // We need to load the category structure for this course
-            $this->load_all_categories($course->category);
-            $parent = $this->addedcategories[$course->category];
-            // This could lead to the course being created so we should check whether it is the case again
-            if (!$forcegeneric && array_key_exists($course->id, $this->addedcourses)) {
-                return $this->addedcourses[$course->id];
+        if (!$ismycourse && !$issite && !empty($course->category)) {
+            if (!empty($CFG->navshowcategories)) {
+                // We need to load the category structure for this course
+                $this->load_all_categories($course->category);
+            }
+            if (array_key_exists($course->category, $this->addedcategories)) {
+                $parent = $this->addedcategories[$course->category];
+                // This could lead to the course being created so we should check whether it is the case again
+                if (!$forcegeneric && array_key_exists($course->id, $this->addedcourses)) {
+                    return $this->addedcourses[$course->id];
+                }
             }
         }
 
@@ -3990,7 +3985,7 @@ class navigation_json {
         $attributes['hidden'] = ($child->hidden);
         $attributes['haschildren'] = ($child->children->count()>0 || $child->type == navigation_node::TYPE_CATEGORY);
 
-        if (count($child->children)>0) {
+        if ($child->children->count() > 0) {
             $attributes['children'] = array();
             foreach ($child->children as $subchild) {
                 $attributes['children'][] = $this->convert_child($subchild, $depth+1);

diff --git a/lib/simpletest/testnavigationlib.php b/lib/simpletest/testnavigationlib.php
index 6622440..fdb5f4f 100644 (file)
--- a/lib/simpletest/testnavigationlib.php
+++ b/lib/simpletest/testnavigationlib.php
@@ -316,14 +316,10 @@ class global_navigation_test extends UnitTestCase {
      * @var global_navigation
      */
     public $node;
-    protected $cache;
-    protected $modinfo5 = 'O:6:"object":6:{s:8:"courseid";s:1:"5";s:6:"userid";s:1:"2";s:8:"sections";a:1:{i:0;a:1:{i:0;s:3:"288";}}s:3:"cms";a:1:{i:288;O:6:"object":17:{s:2:"id";s:3:"288";s:8:"instance";s:2:"19";s:6:"course";s:1:"5";s:7:"modname";s:5:"forum";s:4:"name";s:10:"News forum";s:7:"visible";s:1:"1";s:10:"sectionnum";s:1:"0";s:9:"groupmode";s:1:"0";s:10:"groupingid";s:1:"0";s:16:"groupmembersonly";s:1:"0";s:6:"indent";s:1:"0";s:10:"completion";s:1:"0";s:5:"extra";s:0:"";s:4:"icon";s:0:"";s:11:"uservisible";b:1;s:9:"modplural";s:6:"Forums";s:9:"available";b:1;}}s:9:"instances";a:1:{s:5:"forum";a:1:{i:19;R:8;}}s:6:"groups";N;}';
-    protected $coursesections5 = 'a:5:{i:0;O:8:"stdClass":6:{s:7:"section";s:1:"0";s:2:"id";s:2:"14";s:6:"course";s:1:"5";s:7:"summary";N;s:8:"sequence";s:3:"288";s:7:"visible";s:1:"1";}i:1;O:8:"stdClass":6:{s:7:"section";s:1:"1";s:2:"id";s:2:"97";s:6:"course";s:1:"5";s:7:"summary";s:0:"";s:8:"sequence";N;s:7:"visible";s:1:"1";}i:2;O:8:"stdClass":6:{s:7:"section";s:1:"2";s:2:"id";s:2:"98";s:6:"course";s:1:"5";s:7:"summary";s:0:"";s:8:"sequence";N;s:7:"visible";s:1:"1";}i:3;O:8:"stdClass":6:{s:7:"section";s:1:"3";s:2:"id";s:2:"99";s:6:"course";s:1:"5";s:7:"summary";s:0:"";s:8:"sequence";N;s:7:"visible";s:1:"1";}i:4;O:8:"stdClass":6:{s:7:"section";s:1:"4";s:2:"id";s:3:"100";s:6:"course";s:1:"5";s:7:"summary";s:0:"";s:8:"sequence";N;s:7:"visible";s:1:"1";}}';
     public static $includecoverage = array('./lib/navigationlib.php');
     public static $excludecoverage = array();
 
     public function setUp() {
-        $this->cache = new navigation_cache('simpletest_nav');
         $this->node = new exposed_global_navigation();
         // Create an initial tree structure to work with
         $cat1 = $this->node->add('category 1', null, navigation_node::TYPE_CATEGORY, null, 'cat1');
@@ -335,33 +331,17 @@ class global_navigation_test extends UnitTestCase {
         $course1 = $sub2->add('course 1', null, navigation_node::TYPE_COURSE, null, 'course1');
         $course2 = $sub2->add('course 2', null, navigation_node::TYPE_COURSE, null, 'course2');
         $course3 = $sub2->add('course 3', null, navigation_node::TYPE_COURSE, null, 'course3');
-        $section1 = $course2->add('section 1', null, navigation_node::TYPE_COURSE, null, 'sec1');
-        $section2 = $course2->add('section 2', null, navigation_node::TYPE_COURSE, null, 'sec2');
-        $section3 = $course2->add('section 3', null, navigation_node::TYPE_COURSE, null, 'sec3');
+        $section1 = $course2->add('section 1', null, navigation_node::TYPE_SECTION, null, 'sec1');
+        $section2 = $course2->add('section 2', null, navigation_node::TYPE_SECTION, null, 'sec2');
+        $section3 = $course2->add('section 3', null, navigation_node::TYPE_SECTION, null, 'sec3');
         $act1 = $section2->add('activity 1', null, navigation_node::TYPE_ACTIVITY, null, 'act1');
         $act2 = $section2->add('activity 2', null, navigation_node::TYPE_ACTIVITY, null, 'act2');
         $act3 = $section2->add('activity 3', null, navigation_node::TYPE_ACTIVITY, null, 'act3');
         $res1 = $section2->add('resource 1', null, navigation_node::TYPE_RESOURCE, null, 'res1');
         $res2 = $section2->add('resource 2', null, navigation_node::TYPE_RESOURCE, null, 'res2');
         $res3 = $section2->add('resource 3', null, navigation_node::TYPE_RESOURCE, null, 'res3');
-
-        $this->cache->clear();
-        $this->cache->modinfo5 = unserialize($this->modinfo5);
-        $this->cache->coursesections5 = unserialize($this->coursesections5);
-        $this->cache->canviewhiddenactivities = true;
-        $this->cache->canviewhiddensections = true;
-        $this->cache->canviewhiddencourses = true;
-        $sub2->add('Test Course 5', new moodle_url('http://moodle.org'),navigation_node::TYPE_COURSE,null,'5');
-    }
-    public function test_load_generic_course_sections() {
-        $coursenode = $this->node->find('5', navigation_node::TYPE_COURSE);
-        $course = new stdClass;
-        $course->id = '5';
-        $course->numsections = 10;
-        $course->modinfo = $this->modinfo5;
-        $this->node->load_generic_course_sections($course, $coursenode, 'topic', 'topic');
-        $this->assertEqual($coursenode->children->count(),1);
     }
+
     public function test_format_display_course_content() {
         $this->assertTrue($this->node->exposed_format_display_course_content('topic'));
         $this->assertFalse($this->node->exposed_format_display_course_content('scorm'));