From b774b06593381aeba70fb2964fef4c860f11e153 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Tue, 13 Oct 2015 16:36:10 +0100 Subject: [PATCH] CRM-13803 - hook_navigationMenu - Infer navID/parentID This topic has previously been discussed a bit in https://issues.civicrm.org/jira/browse/CRM-13803 and https://github.com/civicrm/civihr/pull/190 . Long-story short: when an extension adds a new nav item, it must compute navID and parentID, and many extensions do this a buggy way. With this revision, an extension can simply omit the navID and parentID. The navID will be computed (as the next available ID), and the parentID will be inferred (based on the navID in the actual parent). --- CRM/Core/BAO/Navigation.php | 44 +++++++ tests/phpunit/CRM/Core/BAO/NavigationTest.php | 120 ++++++++++++++++++ 2 files changed, 164 insertions(+) diff --git a/CRM/Core/BAO/Navigation.php b/CRM/Core/BAO/Navigation.php index d56bf8ced7..c2047bf107 100644 --- a/CRM/Core/BAO/Navigation.php +++ b/CRM/Core/BAO/Navigation.php @@ -337,6 +337,7 @@ ORDER BY parent_id, weight"; // run the Navigation through a hook so users can modify it CRM_Utils_Hook::navigationMenu($navigations); + self::fixNavigationMenu($navigations); $i18n = CRM_Core_I18n::singleton(); @@ -463,6 +464,49 @@ ORDER BY parent_id, weight"; return $navigationString; } + /** + * Given a navigation menu, generate navIDs for any items which are + * missing them. + * + * @param array $nodes + * Each key is a numeral; each value is a node in + * the menu tree (with keys "child" and "attributes"). + */ + public static function fixNavigationMenu(&$nodes) { + $maxNavID = 1; + array_walk_recursive($nodes, function($item, $key) use (&$maxNavID) { + if ($key === 'navID') { + $maxNavID = max($maxNavID, $item); + } + }); + self::_fixNavigationMenu($nodes, $maxNavID, NULL); + } + + /** + * @param array $nodes + * Each key is a numeral; each value is a node in + * the menu tree (with keys "child" and "attributes"). + */ + private static function _fixNavigationMenu(&$nodes, &$maxNavID, $parentID) { + $origKeys = array_keys($nodes); + foreach ($origKeys as $origKey) { + if (!isset($nodes[$origKey]['attributes']['parentID']) && $parentID !== NULL) { + $nodes[$origKey]['attributes']['parentID'] = $parentID; + } + // If no navID, then assign navID and fix key. + if (!isset($nodes[$origKey]['attributes']['navID'])) { + $newKey = ++$maxNavID; + $nodes[$origKey]['attributes']['navID'] = $newKey; + $nodes[$newKey] = $nodes[$origKey]; + unset($nodes[$origKey]); + $origKey = $newKey; + } + if (isset($nodes[$origKey]['child']) && is_array($nodes[$origKey]['child'])) { + self::_fixNavigationMenu($nodes[$origKey]['child'], $maxNavID, $nodes[$origKey]['attributes']['navID']); + } + } + } + /** * Get Menu name. * diff --git a/tests/phpunit/CRM/Core/BAO/NavigationTest.php b/tests/phpunit/CRM/Core/BAO/NavigationTest.php index 71d744fb58..8533d75a9b 100644 --- a/tests/phpunit/CRM/Core/BAO/NavigationTest.php +++ b/tests/phpunit/CRM/Core/BAO/NavigationTest.php @@ -93,4 +93,124 @@ class CRM_Core_BAO_NavigationTest extends CiviUnitTestCase { "SELECT count(*) FROM civicrm_navigation WHERE url LIKE 'civicrm/report/instance/%'"); } + /** + * Run fixNavigationMenu() on a menu which already has navIDs + * everywhere. They should be unchanged. + */ + public function testFixNavigationMenu_preserveIDs() { + $input[10] = array( + 'attributes' => array( + 'label' => 'Custom Menu Entry', + 'parentID' => NULL, + 'navID' => 10, + 'active' => 1, + ), + 'child' => array( + '11' => array( + 'attributes' => array( + 'label' => 'Custom Child Menu', + 'parentID' => 10, + 'navID' => 11, + ), + 'child' => NULL, + ), + ), + ); + + $output = $input; + CRM_Core_BAO_Navigation::fixNavigationMenu($output); + + $this->assertEquals(NULL, $output[10]['attributes']['parentID']); + $this->assertEquals(10, $output[10]['attributes']['navID']); + $this->assertEquals(10, $output[10]['child'][11]['attributes']['parentID']); + $this->assertEquals(11, $output[10]['child'][11]['attributes']['navID']); + } + + /** + * Run fixNavigationMenu() on a menu which is missing some navIDs. They + * should be filled in, and others should be preserved. + */ + public function testFixNavigationMenu_inferIDs() { + $input[10] = array( + 'attributes' => array( + 'label' => 'Custom Menu Entry', + 'parentID' => NULL, + 'navID' => 10, + 'active' => 1, + ), + 'child' => array( + '0' => array( + 'attributes' => array( + 'label' => 'Custom Child Menu', + ), + 'child' => NULL, + ), + '100' => array( + 'attributes' => array( + 'label' => 'Custom Child Menu 2', + 'navID' => 100, + ), + 'child' => NULL, + ), + ), + ); + + $output = $input; + CRM_Core_BAO_Navigation::fixNavigationMenu($output); + + $this->assertEquals('Custom Menu Entry', $output[10]['attributes']['label']); + $this->assertEquals(NULL, $output[10]['attributes']['parentID']); + $this->assertEquals(10, $output[10]['attributes']['navID']); + + $this->assertEquals('Custom Child Menu', $output[10]['child'][101]['attributes']['label']); + $this->assertEquals(10, $output[10]['child'][101]['attributes']['parentID']); + $this->assertEquals(101, $output[10]['child'][101]['attributes']['navID']); + + $this->assertEquals('Custom Child Menu 2', $output[10]['child'][100]['attributes']['label']); + $this->assertEquals(10, $output[10]['child'][100]['attributes']['parentID']); + $this->assertEquals(100, $output[10]['child'][100]['attributes']['navID']); + } + + public function testFixNavigationMenu_inferIDs_deep() { + $input[10] = array( + 'attributes' => array( + 'label' => 'Custom Menu Entry', + 'parentID' => NULL, + 'navID' => 10, + 'active' => 1, + ), + 'child' => array( + '0' => array( + 'attributes' => array( + 'label' => 'Custom Child Menu', + ), + 'child' => array( + '100' => array( + 'attributes' => array( + 'label' => 'Custom Child Menu 2', + 'navID' => 100, + ), + 'child' => NULL, + ), + ), + ), + ), + ); + + $output = $input; + CRM_Core_BAO_Navigation::fixNavigationMenu($output); + + $this->assertEquals('Custom Menu Entry', $output[10]['attributes']['label']); + $this->assertEquals(NULL, $output[10]['attributes']['parentID']); + $this->assertEquals(10, $output[10]['attributes']['navID']); + + $this->assertEquals('Custom Child Menu', $output[10]['child'][101]['attributes']['label']); + $this->assertEquals(10, $output[10]['child'][101]['attributes']['parentID']); + $this->assertEquals(101, $output[10]['child'][101]['attributes']['navID']); + + $this->assertEquals('Custom Child Menu 2', $output[10]['child'][101]['child'][100]['attributes']['label']); + $this->assertEquals(101, $output[10]['child'][101]['child'][100]['attributes']['parentID']); + $this->assertEquals(100, $output[10]['child'][101]['child'][100]['attributes']['navID']); + } + } -- 2.25.1