From 817d39b8aaade0e7df56ed007696bdcf07c9e22b Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Tue, 13 May 2014 11:18:47 +1200 Subject: [PATCH] MDL-44711 navigation: better handling of course expansion exception --- .../tests/behat/view_my_courses.feature | 34 +++++ lib/navigationlib.php | 6 + lib/tests/behat/behat_navigation.php | 137 ++++++++++++++---- 3 files changed, 145 insertions(+), 32 deletions(-) diff --git a/blocks/navigation/tests/behat/view_my_courses.feature b/blocks/navigation/tests/behat/view_my_courses.feature index 7c953c9eb22..ce073ccb406 100644 --- a/blocks/navigation/tests/behat/view_my_courses.feature +++ b/blocks/navigation/tests/behat/view_my_courses.feature @@ -66,3 +66,37 @@ Feature: View my courses in navigation block And I expand "cat33" node And I should see "c331" in the "Navigation" "block" And I should not see "c332" in the "Navigation" "block" + + @javascript + Scenario: I can expand categories and courses as guest + Given I set the following administration settings values: + | Show my course categories | 1 | + | Show all courses | 1 | + And I log out + 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 should not see "cat31" in the "Navigation" "block" + And I should not see "cat32" in the "Navigation" "block" + And I should not see "cat331" in the "Navigation" "block" + And I should not see "c1" in the "Navigation" "block" + And I should not see "c2" in the "Navigation" "block" + And I should not see "c31" in the "Navigation" "block" + And I should not see "c32" in the "Navigation" "block" + When I expand "cat3" node + And I expand "cat31" node + And I expand "cat1" node + And I should see "c1" in the "Navigation" "block" + And I expand "c1" node + Then I should not see " + 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 should see "cat31" in the "Navigation" "block" + And I should see "cat32" in the "Navigation" "block" + And I should not see "cat331" in the "Navigation" "block" + And I should see "c1" in the "Navigation" "block" + And I should not see "c2" in the "Navigation" "block" + And I should see "c31" in the "Navigation" "block" + And I should not see "c32" in the "Navigation" "block" \ No newline at end of file diff --git a/lib/navigationlib.php b/lib/navigationlib.php index 06081c06d2e..d5196ad7255 100644 --- a/lib/navigationlib.php +++ b/lib/navigationlib.php @@ -2769,6 +2769,12 @@ class global_navigation_for_ajax extends global_navigation { break; case self::TYPE_COURSE : $course = $DB->get_record('course', array('id' => $this->instanceid), '*', MUST_EXIST); + if (!can_access_course($course)) { + // Thats OK all courses are expandable by default. We don't need to actually expand it we can just + // add the course node and break. This leads to an empty node. + $this->add_course($course); + break; + } require_course_login($course, true, null, false, true); $this->page->set_context(context_course::instance($course->id)); $coursenode = $this->add_course($course); diff --git a/lib/tests/behat/behat_navigation.php b/lib/tests/behat/behat_navigation.php index 4d9271bdbb2..887743d2f82 100644 --- a/lib/tests/behat/behat_navigation.php +++ b/lib/tests/behat/behat_navigation.php @@ -40,12 +40,104 @@ use Behat\Behat\Context\Step\Given as Given, */ class behat_navigation extends behat_base { + /** + * Helper function to get a navigation nodes text element given its text from within the navigation block. + * + * This function finds the node with the given text from within the navigation block. + * It checks to make sure the node is visible, and then returns it. + * + * @param string $text + * @param bool $branch Set this true if you're only interested in the node if its a branch. + * @param null|bool $collapsed Set this to true or false if you want the node to either be collapsed or not. + * If its left as null then we don't worry about it. + * @param null|string|Exception|false $exception The exception to throw if the node is not found. + * @return \Behat\Mink\Element\NodeElement + */ + protected function get_node_text_node($text, $branch = false, $collapsed = null, $exception = null) { + if ($exception === null) { + $exception = new ExpectationException('The "' . $text . '" node could not be found', $this->getSession()); + } else if (is_string($exception)) { + $exception = new ExpectationException($exception, $this->getSession()); + } + + $nodetextliteral = $this->getSession()->getSelectorsHandler()->xpathLiteral($text); + $hasblocktree = "[contains(concat(' ', normalize-space(@class), ' '), ' block_tree ')]"; + $hasbranch = "[contains(concat(' ', normalize-space(@class), ' '), ' branch ')]"; + $hascollapsed = "li[contains(concat(' ', normalize-space(@class), ' '), ' collapsed ') or @data-exandable='1']"; + $notcollapsed = "[not(contains(concat(' ', normalize-space(@class), ' '), ' collapsed '))]"; + $match = "[normalize-space(.)={$nodetextliteral}]"; + + // Avoid problems with quotes. + $isbranch = ($branch) ? $hasbranch : ''; + if ($collapsed === true) { + $iscollapsed = $hascollapsed; + } else if ($collapsed === false) { + $iscollapsed = $notcollapsed; + } else { + $iscollapsed = 'li'; + } + + $xpath = "//ul{$hasblocktree}//li{$notcollapsed}/ul/{$iscollapsed}/p{$isbranch}/a{$match}|"; + $xpath .= "//ul{$hasblocktree}//li{$notcollapsed}/ul/{$iscollapsed}/p{$isbranch}/span{$match}"; + + $node = $this->find('xpath', $xpath, $exception); + $this->ensure_node_is_visible($node); + return $node; + } + + /** + * Returns true if the navigation node with the given text is expandable. + * + * @Given /^navigation node "([^"]*)" should be expandable$/ + * + * @throws ExpectationException + * @param string $nodetext + * @return bool + */ + public function navigation_node_should_be_expandable($nodetext) { + if (!$this->running_javascript()) { + // Nodes are only expandable when JavaScript is enabled. + return false; + } + + $node = $this->get_node_text_node($nodetext, true); + $node = $node->getParent(); + if ($node->hasAttribute('data-expandable') && $node->getAttribute('data-expandable')) { + return true; + } + throw new ExpectationException('The "' . $nodetext . '" node is not expandable', $this->getSession()); + } + + /** + * Returns true if the navigation node with the given text is not expandable. + * + * @Given /^navigation node "([^"]*)" should not be expandable$/ + * + * @throws ExpectationException + * @param string $nodetext + * @return bool + */ + public function navigation_node_should_not_be_expandable($nodetext) { + if (!$this->running_javascript()) { + // Nodes are only expandable when JavaScript is enabled. + return false; + } + + $node = $this->get_node_text_node($nodetext); + $node = $node->getParent(); + if ($node->hasAttribute('data-expandable') && $node->getAttribute('data-expandable')) { + throw new ExpectationException('The "' . $nodetext . '" node is expandable', $this->getSession()); + } + return true; + } + /** * Expands the selected node of the navigation tree that matches the text. * @Given /^I expand "(?P(?:[^"]|\\")*)" node$/ * * @throws ExpectationException * @param string $nodetext + * @return bool|void */ public function i_expand_node($nodetext) { @@ -60,23 +152,12 @@ class behat_navigation extends behat_base { return true; } - // Avoid problems with quotes. - $nodetextliteral = $this->getSession()->getSelectorsHandler()->xpathLiteral($nodetext); - - $xpath = "//ul[contains(concat(' ', normalize-space(@class), ' '), ' block_tree ')]" . - "/child::li[contains(concat(' ', normalize-space(@class), ' '), ' collapsed ')]" . - "/child::p[contains(concat(' ', normalize-space(@class), ' '), ' branch ')]" . - "/child::span[normalize-space(.)=$nodetextliteral]" . - "|" . - "//ul[contains(concat(' ', normalize-space(@class), ' '), ' block_tree ')]" . - "/descendant::li[not(contains(concat(' ', normalize-space(@class), ' '), ' collapsed '))]" . - "/descendant::li[contains(concat(' ', normalize-space(@class), ' '), ' collapsed ')]" . - "/child::p[contains(concat(' ', normalize-space(@class), ' '), ' branch ')]" . - "/child::span[normalize-space(.)=$nodetextliteral]"; - - $exception = new ExpectationException('The "' . $nodetext . '" node can not be expanded', $this->getSession()); - $node = $this->find('xpath', $xpath, $exception); - $this->ensure_node_is_visible($node); + $node = $this->get_node_text_node($nodetext, true, true, 'The "' . $nodetext . '" node can not be expanded'); + // Check if the node is a link AND a branch. + if (strtolower($node->getTagName()) === 'a') { + // We just want to expand the node, we don't want to follow it. + $node = $node->getParent(); + } $node->click(); } @@ -86,6 +167,7 @@ class behat_navigation extends behat_base { * @Given /^I collapse "(?P(?:[^"]|\\")*)" node$/ * @throws ExpectationException * @param string $nodetext + * @return bool|void */ public function i_collapse_node($nodetext) { @@ -94,21 +176,12 @@ class behat_navigation extends behat_base { return true; } - // Avoid problems with quotes. - $nodetextliteral = $this->getSession()->getSelectorsHandler()->xpathLiteral($nodetext); - - $xpath = "//ul[contains(concat(' ', normalize-space(@class), ' '), ' block_tree ')]" . - "/child::li[not(contains(concat(' ', normalize-space(@class), ' '), ' collapsed '))]" . - "/child::p[contains(concat(' ', normalize-space(@class), ' '), ' branch ')]" . - "/child::span[normalize-space(.)=$nodetextliteral]" . - "|" . - "//ul[contains(concat(' ', normalize-space(@class), ' '), ' block_tree ')]" . - "/descendant::li[not(contains(concat(' ', normalize-space(@class), ' '), ' collapsed '))]" . - "/child::p[contains(concat(' ', normalize-space(@class), ' '), ' branch ')]" . - "/child::span[normalize-space(.)=$nodetextliteral]"; - - $exception = new ExpectationException('The "' . $nodetext . '" node can not be collapsed', $this->getSession()); - $node = $this->find('xpath', $xpath, $exception); + $node = $this->get_node_text_node($nodetext, true, false, 'The "' . $nodetext . '" node can not be collapsed'); + // Check if the node is a link AND a branch. + if (strtolower($node->getTagName()) === 'a') { + // We just want to expand the node, we don't want to follow it. + $node = $node->getParent(); + } $node->click(); }