diff --git a/lib/classes/navigation/views/secondary.php b/lib/classes/navigation/views/secondary.php index 42b3b7aa846..fe47bc7e1c1 100644 --- a/lib/classes/navigation/views/secondary.php +++ b/lib/classes/navigation/views/secondary.php @@ -288,6 +288,39 @@ class secondary extends view { return $newnode; } + /** + * Recursive call to add all custom navigation nodes to secondary + * + * @param navigation_node $node The node which should be added to secondary + * @param navigation_node $basenode The original parent node + * @param bool $forceadd Whether or not to bypass the external action check and force add all nodes + */ + protected function add_external_nodes_to_secondary(navigation_node $node, navigation_node $basenode, bool $forceadd = false) { + // Add the first node. + if ($node->has_action() && !$this->get($node->key)) { + $this->add_node(clone $node); + } + + // If the node has an external action add all children to the secondary navigation. + if (!$node->has_internal_action() || $forceadd) { + if ($node->has_children()) { + foreach ($node->children as $child) { + if ($child->has_children()) { + $this->add_external_nodes_to_secondary($child, $basenode, true); + } else if ($child->has_action() && !$this->get($child->key)) { + // Check whether the basenode matches a child's url. + // This would have happened in get_first_action_for_node. + // In these cases, we prefer the specific child content. + if ($basenode->has_action() && $basenode->action()->compare($child->action())) { + $this->children->remove($basenode->key, $basenode->type); + } + $this->add_node(clone $child); + } + } + } + } + } + /** * Returns a list of all expected nodes in the course administration. * @@ -339,12 +372,10 @@ class secondary extends view { foreach ($value->children as $other) { if (array_search($other->key, $expectedcourseadmin) === false) { $othernode = $this->get_first_action_for_node($other); + $recursivenode = $othernode && !$this->get($othernode->key) ? $othernode : $other; // Get the first node and check whether it's been added already. - if ($othernode && !$this->get($othernode->key)) { - $this->add_node($othernode); - } else { - $this->add_node($other); - } + // Also check if the first node is an external link. If it is, add all children. + $this->add_external_nodes_to_secondary($recursivenode, $recursivenode); } } } @@ -677,9 +708,9 @@ class secondary extends view { $leftovernode = $this->get_first_action_for_node($leftovernode); } - // Confirm we have a valid object to add. + // We have found the first node with an action. if ($leftovernode) { - $this->add_node(clone $leftovernode); + $this->add_external_nodes_to_secondary($leftovernode, $leftovernode); } } } diff --git a/lib/navigationlib.php b/lib/navigationlib.php index 1bc6ee6c8ff..b12c7642613 100644 --- a/lib/navigationlib.php +++ b/lib/navigationlib.php @@ -328,6 +328,11 @@ class navigation_node implements renderable { */ public static function create($text, $action=null, $type=self::TYPE_CUSTOM, $shorttext=null, $key=null, pix_icon $icon=null) { + if ($action && !($action instanceof moodle_url || $action instanceof action_link)) { + debugging( + "It is required that the action provided be either an action_url|moodle_url." . + " Please update your definition.", E_NOTICE); + } // Properties array used when creating the new navigation node $itemarray = array( 'text' => $text, @@ -363,6 +368,9 @@ class navigation_node implements renderable { * @return navigation_node */ public function add($text, $action=null, $type=self::TYPE_CUSTOM, $shorttext=null, $key=null, pix_icon $icon=null) { + if ($action && is_string($action)) { + $action = new moodle_url($action); + } // Create child node $childnode = self::create($text, $action, $type, $shorttext, $key, $icon); @@ -698,6 +706,26 @@ class navigation_node implements renderable { return !empty($this->action); } + /** + * Used to easily determine if the action is an internal link. + * + * @return bool + */ + public function has_internal_action(): bool { + global $CFG; + if ($this->has_action()) { + $url = $this->action(); + if ($this->action() instanceof \action_link) { + $url = $this->action()->url; + } + + if (($url->out() === $CFG->wwwroot) || (strpos($url->out(), $CFG->wwwroot.'/') === 0)) { + return true; + } + } + return false; + } + /** * Used to easily determine if this link in the breadcrumbs is hidden. * diff --git a/lib/tests/navigation/views/secondary_test.php b/lib/tests/navigation/views/secondary_test.php index 0dbd80add1f..128983e28bd 100644 --- a/lib/tests/navigation/views/secondary_test.php +++ b/lib/tests/navigation/views/secondary_test.php @@ -651,6 +651,81 @@ class secondary_test extends \advanced_testcase { ]; } + /** + * Test the add_external_nodes_to_secondary function. + * + * @param array $structure The structure of the navigation node tree to setup with. + * @param array $expectednodes The expected nodes added to the secondary navigation + * @dataProvider add_external_nodes_to_secondary_provider + */ + public function test_add_external_nodes_to_secondary(array $structure, array $expectednodes) { + global $PAGE; + + $this->resetAfterTest(); + $course = $this->getDataGenerator()->create_course(); + $context = \context_course::instance($course->id); + $PAGE->set_context($context); + $PAGE->set_url('/'); + + $node = $this->generate_node_tree_construct($structure, 'parentnode'); + $secondary = new secondary($PAGE); + $secondary->add_node($node); + $firstnode = $node->get('parentnode1'); + + $method = new ReflectionMethod('core\navigation\views\secondary', 'add_external_nodes_to_secondary'); + $method->setAccessible(true); + $method->invoke($secondary, $firstnode, $firstnode); + + $test = $secondary->get_children_key_list(); + $this->assertEquals($expectednodes, $test); + } + + /** + * Provider for the add_external_nodes_to_secondary function. + * + * @return array + */ + public function add_external_nodes_to_secondary_provider() { + return [ + "Container node with internal action and external children" => [ + [ + 'parentnode1' => [ + 'action' => '/test.php', + 'children' => [ + 'child2.1' => 'https://example.org', + 'child2.2' => 'https://example.net', + ] + ] + ], + ['parentnode', 'parentnode1'] + ], + "Container node with external action and external children" => [ + [ + 'parentnode1' => [ + 'action' => '/test.php', + 'children' => [ + 'child2.1' => 'https://example.org', + 'child2.2' => 'https://example.net', + ] + ] + ], + ['parentnode', 'parentnode1'] + ], + "Container node with external action and internal children" => [ + [ + 'parentnode1' => [ + 'action' => 'https://example.org', + 'children' => [ + 'child2.1' => '/view/course.php', + 'child2.2' => '/view/admin.php', + ] + ] + ], + ['parentnode', 'parentnode1', 'child2.1', 'child2.2'] + ], + ]; + } + /** * Test the get_overflow_menu_data function *