Skip to content

Commit

Permalink
MDL-44711 navigation: improved expandable course calculation
Browse files Browse the repository at this point in the history
  • Loading branch information
Sam Hemelryk committed Jun 8, 2014
1 parent e7ea6b9 commit cbd063f
Show file tree
Hide file tree
Showing 4 changed files with 271 additions and 1 deletion.
190 changes: 190 additions & 0 deletions blocks/navigation/tests/behat/expand_courses_node.feature
View file
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions lang/en/cache.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
$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';
Expand Down
10 changes: 10 additions & 0 deletions lib/db/caches.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,4 +212,14 @@
'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
)
);
71 changes: 70 additions & 1 deletion lib/navigationlib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1166,6 +1168,14 @@ public function initialise() {

// 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()) {
Expand All @@ -1175,6 +1185,15 @@ public function initialise() {
}
}

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
Expand Down Expand Up @@ -2373,6 +2392,8 @@ public function add_course(stdClass $course, $forcegeneric = false, $coursetype
// 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;
Expand All @@ -2392,6 +2413,8 @@ public function add_course(stdClass $course, $forcegeneric = false, $coursetype
} 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
Expand All @@ -2408,18 +2431,64 @@ public function add_course(stdClass $course, $forcegeneric = false, $coursetype
}

$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;
}

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
*
Expand Down

0 comments on commit cbd063f

Please sign in to comment.