mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-01-19 06:18:28 +01:00
Code Issues Projects Releases Wiki Activity

MDL-73273 navigation: Add checks to enforce action param types

Browse Source 
* Added checks in navigationlib to enforce the data type of the passed in action
* Add nodes with non moodleurls to the end of the secondary nav
This commit is contained in:
Peter Dias 2022-01-27 11:44:53 +08:00
parent d24a4ab56f
commit bdf4f8a651
3 changed files with 141 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
lib/classes/navigation/views/secondary.php
View File

@ -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);
}
}
}

28
lib/navigationlib.php
View File

@ -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.
*

75
lib/tests/navigation/views/secondary_test.php
View File

@ -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
*