From cbd063f4c3133b330a0d27c9a0185343d3f3c740 Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Fri, 21 Mar 2014 13:08:35 +1300 Subject: [PATCH] MDL-44711 navigation: improved expandable course calculation --- .../tests/behat/expand_courses_node.feature | 190 ++++++++++++++++++ lang/en/cache.php | 1 + lib/db/caches.php | 10 + lib/navigationlib.php | 71 ++++++- 4 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 blocks/navigation/tests/behat/expand_courses_node.feature diff --git a/blocks/navigation/tests/behat/expand_courses_node.feature b/blocks/navigation/tests/behat/expand_courses_node.feature new file mode 100644 index 00000000000..2fd28caf442 --- /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 6d4eab72f76..603746947aa 100644 --- 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 c8820730764..c2f99cfd767 100644 --- 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 eac7afe0095..10fde52e255 100644 --- 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 * -- 2.36.1