MDL-44711 navigation: improved expandable course calculation

diff --git a/blocks/navigation/tests/behat/expand_courses_node.feature b/blocks/navigation/tests/behat/expand_courses_node.feature
new file mode 100644 (file)
index 0000000..2fd28ca
--- /dev/null
+++ b/blocks/navigation/tests/behat/expand_courses_node.feature
@@ -0,0 +1,190 @@
+@block @block_navigation
+Feature: Expand the courses nodes within the navigation block
+  In order to navigate the site
+  As an anonymous user, a guest, a student, and an admin
+  I need to expand the courses node in the navigation block and check the display of courses and categories.
+
+  Background:
+    Given the following "users" exist:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher | 1 | teacher1@local.host |
+      | student1 | Student | 1 | student1@local.host |
+    And the following "categories" exist:
+      | name   | category | idnumber | visible |
+      | cat1   | 0        | cat1     | 1       |
+      | cat2   | 0        | cat2     | 1       |
+      | cat21  | cat2     | cat21    | 1       |
+      | cat211 | cat21    | cat211   | 1       |
+      | cat3   | 0        | cat3     | 0       |
+    And the following "courses" exist:
+      | fullname  | shortname | category | visible |
+      | Course 1  | c1        | cat1     | 1       |
+      | Course 2  | c2        | cat2     | 1       |
+      | Course 3  | c3        | cat21    | 1       |
+      | Course 4  | c4        | cat211   | 1       |
+      | Course 5  | c5        | cat211   | 0       |
+      | Course 6  | c6        | cat211   | 0       |
+      | Course 7  | c7        | cat3     | 1       |
+      | Course 8  | c8        | cat3     | 0       |
+    And the following "course enrolments" exist:
+      | user     | course | role    |
+      | teacher1 | c1     | teacher |
+      | teacher1 | c3     | teacher |
+      | teacher1 | c5     | teacher |
+      | student1 | c1     | student |
+      | student1 | c2     | student |
+      | student1 | c4     | student |
+    And I log in as "admin"
+    And I follow "Course 2"
+    And I turn editing mode on
+    And I click on "Edit settings" "link" in the "Administration" "block"
+    And I set the following fields to these values:
+      | Allow guest access | Yes |
+    And I press "Save changes"
+    And I set the following administration settings values:
+      | Show all courses | 1 |
+    And I log out
+
+  @javascript
+  Scenario: As an anonymous user I expand the courses node to see courses.
+    When I should see "You are not logged in." in the ".logininfo" "css_element"
+    And I should see "Home" in the "Navigation" "block"
+    And I should see "Courses" in the "Navigation" "block"
+    And I expand "Courses" node
+    And I should see "cat1" in the "Navigation" "block"
+    And I should see "cat2" in the "Navigation" "block"
+    And I should not see "cat3" in the "Navigation" "block"
+    And I expand "cat1" node
+    And I expand "cat2" node
+    And I should see "cat21" in the "Navigation" "block"
+    And I expand "cat21" node
+    And I should see "cat211" in the "Navigation" "block"
+    And I expand "cat211" node
+    Then I should see "c1" in the "Navigation" "block"
+    And I should see "c2" in the "Navigation" "block"
+    And I should see "c3" in the "Navigation" "block"
+    And I should see "c4" in the "Navigation" "block"
+    And I should not see "c5" in the "Navigation" "block"
+    And I should not see "c6" in the "Navigation" "block"
+    And navigation node "c1" should not be expandable
+    And navigation node "c2" should not be expandable
+    And navigation node "c3" should not be expandable
+    And navigation node "c4" should not be expandable
+
+  @javascript
+  Scenario: As the admin user I expand the courses and category nodes to see courses.
+    When I log in as "admin"
+    And I should see "Home" in the "Navigation" "block"
+    And I should see "Courses" in the "Navigation" "block"
+    And I expand "Courses" node
+    And I should see "cat1" in the "Navigation" "block"
+    And I should see "cat2" in the "Navigation" "block"
+    And I should see "cat3" in the "Navigation" "block"
+    And I expand "cat1" node
+    And I expand "cat2" node
+    And I expand "cat3" node
+    And I should see "cat21" in the "Navigation" "block"
+    And I expand "cat21" node
+    And I should see "cat211" in the "Navigation" "block"
+    And I expand "cat211" node
+    Then I should see "c1" in the "Navigation" "block"
+    And I should see "c2" in the "Navigation" "block"
+    And I should see "c3" in the "Navigation" "block"
+    And I should see "c4" in the "Navigation" "block"
+    And I should see "c5" in the "Navigation" "block"
+    And I should see "c6" in the "Navigation" "block"
+    And I should see "c7" in the "Navigation" "block"
+    And I should see "c8" in the "Navigation" "block"
+    And navigation node "c1" should be expandable
+    And navigation node "c2" should be expandable
+    And navigation node "c3" should be expandable
+    And navigation node "c4" should be expandable
+    And navigation node "c5" should be expandable
+    And navigation node "c6" should be expandable
+    And navigation node "c7" should be expandable
+    And navigation node "c8" should be expandable
+
+  @javascript
+  Scenario: As teacher1 I expand the courses and category nodes to see courses.
+    When I log in as "teacher1"
+    And I should see "Home" in the "Navigation" "block"
+    And I should see "Courses" in the "Navigation" "block"
+    And I expand "Courses" node
+    And I should see "cat1" in the "Navigation" "block"
+    And I should see "cat2" in the "Navigation" "block"
+    And I should not see "cat3" in the "Navigation" "block"
+    And I expand "cat1" node
+    And I expand "cat2" node
+    And I should see "cat21" in the "Navigation" "block"
+    And I expand "cat21" node
+    And I should see "cat211" in the "Navigation" "block"
+    And I expand "cat211" node
+    Then I should see "c1" in the "Navigation" "block"
+    And I should see "c2" in the "Navigation" "block"
+    And I should see "c3" in the "Navigation" "block"
+    And I should see "c4" in the "Navigation" "block"
+    And I should see "c5" in the "Navigation" "block"
+    And I should not see "c6" in the "Navigation" "block"
+    And I should not see "c7" in the "Navigation" "block"
+    And I should not see "c8" in the "Navigation" "block"
+    And navigation node "c1" should be expandable
+    And navigation node "c2" should be expandable
+    And navigation node "c3" should be expandable
+    And navigation node "c4" should not be expandable
+    And navigation node "c5" should be expandable
+
+  @javascript
+  Scenario: As student1 I expand the courses and category nodes to see courses.
+    When I log in as "student1"
+    And I should see "Home" in the "Navigation" "block"
+    And I should see "Courses" in the "Navigation" "block"
+    And I expand "Courses" node
+    And I should see "cat1" in the "Navigation" "block"
+    And I should see "cat2" in the "Navigation" "block"
+    And I should not see "cat3" in the "Navigation" "block"
+    And I expand "cat1" node
+    And I expand "cat2" node
+    And I should see "cat21" in the "Navigation" "block"
+    And I expand "cat21" node
+    And I should see "cat211" in the "Navigation" "block"
+    And I expand "cat211" node
+    Then I should see "c1" in the "Navigation" "block"
+    And I should see "c2" in the "Navigation" "block"
+    And I should see "c3" in the "Navigation" "block"
+    And I should see "c4" in the "Navigation" "block"
+    And I should not see "c5" in the "Navigation" "block"
+    And I should not see "c6" in the "Navigation" "block"
+    And I should not see "c7" in the "Navigation" "block"
+    And I should not see "c8" in the "Navigation" "block"
+    And navigation node "c1" should be expandable
+    And navigation node "c2" should be expandable
+    And navigation node "c3" should not be expandable
+    And navigation node "c4" should be expandable
+
+  @javascript
+  Scenario: As guest I expand the courses and category nodes to see courses.
+    When I log in as "guest"
+    And I should see "Home" in the "Navigation" "block"
+    And I should see "Courses" in the "Navigation" "block"
+    And I expand "Courses" node
+    And I should see "cat1" in the "Navigation" "block"
+    And I should see "cat2" in the "Navigation" "block"
+    And I should not see "cat3" in the "Navigation" "block"
+    And I expand "cat1" node
+    And I expand "cat2" node
+    And I should see "cat21" in the "Navigation" "block"
+    And I expand "cat21" node
+    And I should see "cat211" in the "Navigation" "block"
+    And I expand "cat211" node
+    Then I should see "c1" in the "Navigation" "block"
+    And I should see "c2" in the "Navigation" "block"
+    And I should see "c3" in the "Navigation" "block"
+    And I should see "c4" in the "Navigation" "block"
+    And I should not see "c5" in the "Navigation" "block"
+    And I should not see "c6" in the "Navigation" "block"
+    And I should not see "c7" in the "Navigation" "block"
+    And I should not see "c8" in the "Navigation" "block"
+    And navigation node "c1" should not be expandable
+    And navigation node "c2" should be expandable
+    And navigation node "c3" should not be expandable
+    And navigation node "c4" should not be expandable
\ No newline at end of file

diff --git a/lang/en/cache.php b/lang/en/cache.php
index 6d4eab7..6037469 100644 (file)
--- a/lang/en/cache.php
+++ b/lang/en/cache.php
@@ -51,6 +51,7 @@ $string['cachedef_groupdata'] = 'Course group information';
 $string['cachedef_htmlpurifier'] = 'HTML Purifier - cleaned content';
 $string['cachedef_langmenu'] = 'List of available languages';
 $string['cachedef_locking'] = 'Locking';
+$string['cachedef_navigation_expandcourse'] = 'Navigation expandable courses';
 $string['cachedef_observers'] = 'Event observers';
 $string['cachedef_plugin_manager'] = 'Plugin info manager';
 $string['cachedef_questiondata'] = 'Question definitions';

diff --git a/lib/db/caches.php b/lib/db/caches.php
index c882073..c2f99cf 100644 (file)
--- a/lib/db/caches.php
+++ b/lib/db/caches.php
@@ -212,4 +212,14 @@ $definitions = array(
         'staticaccelerationsize' => 2, // Should not be required for more than one user at a time.
         'ttl' => 3600,
     ),
+
+    // A simple cache that stores whether a user can expand a course in the navigation.
+    // The key is the course ID and the value will either be 1 or 0 (cast to bool).
+    // The cache isn't always up to date, it should only ever be used to save a costly call to
+    // can_access_course on the first page request a user makes.
+    'navigation_expandcourse' => array(
+        'mode' => cache_store::MODE_SESSION,
+        'simplekeys' => true,
+        'simpledata' => true
+    )
 );

diff --git a/lib/navigationlib.php b/lib/navigationlib.php
index eac7afe..10fde52 100644 (file)
--- a/lib/navigationlib.php
+++ b/lib/navigationlib.php
@@ -1001,6 +1001,8 @@ class global_navigation extends navigation_node {
     protected $expansionlimit = 0;
     /** @var int userid to allow parent to see child's profile page navigation */
     protected $useridtouseforparentchecks = 0;
+    /** @var cache_session A cache that stores information on expanded courses */
+    protected $cacheexpandcourse = null;
 
     /** Used when loading categories to load all top level categories [parent = 0] **/
     const LOAD_ROOT_CATEGORIES = 0;
@@ -1166,6 +1168,14 @@ class global_navigation extends navigation_node {
 
                 // Not enrolled, can't view, and hasn't switched roles
                 if (!can_access_course($course)) {
+                    if ($coursenode->isexpandable === true) {
+                        // Obviously the situation has changed, update the cache and adjust the node.
+                        // This occurs if the user access to a course has been revoked (one way or another) after
+                        // initially logging in for this session.
+                        $this->get_expand_course_cache()->set($course->id, 1);
+                        $coursenode->isexpandable = true;
+                        $coursenode->nodetype = self::NODETYPE_BRANCH;
+                    }
                     // Very ugly hack - do not force "parents" to enrol into course their child is enrolled in,
                     // this hack has been propagated from user/view.php to display the navigation node. (MDL-25805)
                     if (!$this->current_user_is_parent_role()) {
@@ -1175,6 +1185,15 @@ class global_navigation extends navigation_node {
                     }
                 }
 
+                if ($coursenode->isexpandable === false) {
+                    // Obviously the situation has changed, update the cache and adjust the node.
+                    // This occurs if the user has been granted access to a course (one way or another) after initially
+                    // logging in for this session.
+                    $this->get_expand_course_cache()->set($course->id, 1);
+                    $coursenode->isexpandable = true;
+                    $coursenode->nodetype = self::NODETYPE_BRANCH;
+                }
+
                 // Add the essentials such as reports etc...
                 $this->add_course_essentials($coursenode, $course);
                 // Extend course navigation with it's sections/activities
@@ -2373,6 +2392,8 @@ class global_navigation extends navigation_node {
         // This is the name that will be shown for the course.
         $coursename = empty($CFG->navshowfullcoursenames) ? $shortname : $fullname;
 
+        // Can the user expand the course to see its content.
+        $canexpandcourse = true;
         if ($issite) {
             $parent = $this;
             $url = null;
@@ -2392,6 +2413,8 @@ class global_navigation extends navigation_node {
         } else {
             $parent = $this->rootnodes['courses'];
             $url = new moodle_url('/course/view.php', array('id'=>$course->id));
+            // They can only expand the course if they can access it.
+            $canexpandcourse = $this->can_expand_course($course);
             if (!empty($course->category) && $this->show_categories($coursetype == self::COURSE_MY)) {
                 if (!$this->is_category_fully_loaded($course->category)) {
                     // We need to load the category structure for this course
@@ -2408,11 +2431,18 @@ class global_navigation extends navigation_node {
         }
 
         $coursenode = $parent->add($coursename, $url, self::TYPE_COURSE, $shortname, $course->id);
-        $coursenode->nodetype = self::NODETYPE_BRANCH;
         $coursenode->hidden = (!$course->visible);
         // We need to decode &'s here as they will have been added by format_string above and attributes will be encoded again
         // later.
         $coursenode->title(str_replace('&', '&', $fullname));
+        if ($canexpandcourse) {
+            // This course can be expanded by the user, make it a branch to make the system aware that its expandable by ajax.
+            $coursenode->nodetype = self::NODETYPE_BRANCH;
+            $coursenode->isexpandable = true;
+        } else {
+            $coursenode->nodetype = self::NODETYPE_LEAF;
+            $coursenode->isexpandable = false;
+        }
         if (!$forcegeneric) {
             $this->addedcourses[$course->id] = $coursenode;
         }
@@ -2420,6 +2450,45 @@ class global_navigation extends navigation_node {
         return $coursenode;
     }
 
+    /**
+     * Returns a cache instance to use for the expand course cache.
+     * @return cache_session
+     */
+    protected function get_expand_course_cache() {
+        if ($this->cacheexpandcourse === null) {
+            $this->cacheexpandcourse = cache::make('core', 'navigation_expandcourse');
+        }
+        return $this->cacheexpandcourse;
+    }
+
+    /**
+     * Checks if a user can expand a course in the navigation.
+     *
+     * We use a cache here because in order to be accurate we need to call can_access_course which is a costly function.
+     * Because this functionality is basic + non-essential and because we lack good event triggering this cache
+     * permits stale data.
+     * In the situation the user is granted access to a course after we've initialised this session cache the cache
+     * will be stale.
+     * It is brought up to date in only one of two ways.
+     *   1. The user logs out and in again.
+     *   2. The user browses to the course they've just being given access to.
+     *
+     * Really all this controls is whether the node is shown as expandable or not. It is uber un-important.
+     *
+     * @param stdClass $course
+     * @return bool
+     */
+    protected function can_expand_course($course) {
+        $cache = $this->get_expand_course_cache();
+        $canexpand = $cache->get($course->id);
+        if ($canexpand === false) {
+            $canexpand = isloggedin() && can_access_course($course);
+            $canexpand = (int)$canexpand;
+            $cache->set($course->id, $canexpand);
+        }
+        return ($canexpand === 1);
+    }
+
     /**
      * Returns true if the category has already been loaded as have any child categories
      *