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).

diff --git a/CRM/Core/BAO/Navigation.php b/CRM/Core/BAO/Navigation.php
index d56bf8ced714b1511b740fd0defcf9ae5c81a78f..c2047bf107e441522c7d0b3a235c96919111b6fc 100644 (file)
--- 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 71d744fb584006899fefbb5771fb52319a4d0310..8533d75a9b7c87be87da330caa9cb8fc1322784c 100644 (file)
--- 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']);
+  }
+
 }