From 0087242f14e30b05f7a261c8ff6a507d2b8d9d4c Mon Sep 17 00:00:00 2001
From: eileenmcnaugton <eileen@fuzion.co.nz>
Date: Tue, 29 Sep 2015 15:04:40 +1300
Subject: [PATCH] CRM-17176 further fix to navigation reset

The reset was stealing menu items from other menus as a result of the having the same name
---
 CRM/Core/BAO/Navigation.php                   | 39 +++++++++++--------
 tests/phpunit/CRM/Core/BAO/NavigationTest.php |  5 ++-
 2 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/CRM/Core/BAO/Navigation.php b/CRM/Core/BAO/Navigation.php
index 7e22a8635e..f87a855380 100644
--- a/CRM/Core/BAO/Navigation.php
+++ b/CRM/Core/BAO/Navigation.php
@@ -862,7 +862,8 @@ ORDER BY parent_id, weight";
       elseif ($component['name'] === 'CiviCampaign') {
         $permission = "access CiviReport";
       }
-      $component_nav = self::createOrUpdateReportNavItem($component_nav_name, 'civicrm/report/list', "compid={$component_id}&reset=1", $reports_nav->id, $permission, $domain_id);
+      $component_nav = self::createOrUpdateReportNavItem($component_nav_name, 'civicrm/report/list',
+        "compid={$component_id}&reset=1", $reports_nav->id, $permission, $domain_id, TRUE);
       foreach ($component['reports'] as $report_id => $report) {
         // Create or update the report instance links.
         $report_nav = self::createOrUpdateReportNavItem($report['title'], $report['url'], 'reset=1', $component_nav->id, $report['permission'], $domain_id);
@@ -910,17 +911,17 @@ ORDER BY parent_id, weight";
    * @param string $url
    * @param array $url_params
    *
-   * @return bool|\CRM_Core_DAO
+   * @param int|null $parent_id
+   *   Optionally restrict to one parent.
+   *
+   * @return bool|\CRM_Core_BAO_Navigation
    */
-  public static function getNavItemByUrl($url, $url_params) {
-    $query = "SELECT * FROM civicrm_navigation WHERE url = %1";
-    $params = array(
-      1 => array("{$url}?{$url_params}", 'String'),
-    );
-    $dao = CRM_Core_DAO::executeQuery($query, $params, TRUE, 'CRM_Core_DAO_Navigation');
-    $dao->fetch();
-    if (isset($dao->id)) {
-      return $dao;
+  public static function getNavItemByUrl($url, $url_params, $parent_id = NULL) {
+    $nav = new CRM_Core_BAO_Navigation();
+    $nav->url = "{$url}?{$url_params}";
+    $nav->parent_id = $parent_id;
+    if ($nav->find(TRUE)) {
+      return $nav;
     }
     return FALSE;
   }
@@ -979,14 +980,21 @@ ORDER BY parent_id, weight";
    * @param string $permission
    * @param int $domain_id
    *
+   * @param bool $onlyMatchParentID
+   *   If True then do not match with a url that has a different parent
+   *   (This is because for top level items there is a risk of 'stealing' rows that normally
+   *   live under 'Contact' and intentionally duplicate the report examples.)
+   *
    * @return \CRM_Core_DAO_Navigation
    */
-  protected static function createOrUpdateReportNavItem($name, $url, $url_params, $parent_id, $permission, $domain_id) {
+  protected static function createOrUpdateReportNavItem($name, $url, $url_params, $parent_id, $permission,
+                                                        $domain_id, $onlyMatchParentID = FALSE) {
     $id = NULL;
-    $existing_nav = CRM_Core_BAO_Navigation::getNavItemByUrl($url, $url_params);
+    $existing_nav = CRM_Core_BAO_Navigation::getNavItemByUrl($url, $url_params, ($onlyMatchParentID ? $parent_id : NULL));
     if ($existing_nav) {
       $id = $existing_nav->id;
     }
+
     $nav = self::createReportNavItem($name, $url, $url_params, $parent_id, $permission, $id, $domain_id);
     return $nav;
   }
@@ -1020,11 +1028,10 @@ ORDER BY parent_id, weight";
       ),
       'domain_id' => $domain_id,
     );
-    if ($id !== NULL) {
+    if ($id) {
       $params['id'] = $id;
     }
-    $nav = CRM_Core_BAO_Navigation::add($params);
-    return $nav;
+    return CRM_Core_BAO_Navigation::add($params);
   }
 
   /**
diff --git a/tests/phpunit/CRM/Core/BAO/NavigationTest.php b/tests/phpunit/CRM/Core/BAO/NavigationTest.php
index 900461857b..71d744fb58 100644
--- a/tests/phpunit/CRM/Core/BAO/NavigationTest.php
+++ b/tests/phpunit/CRM/Core/BAO/NavigationTest.php
@@ -34,7 +34,7 @@ class CRM_Core_BAO_NavigationTest extends CiviUnitTestCase {
   }
 
   /**
-   * Test that an existing report link is rebuilt under is't parent.
+   * Test that an existing report link is rebuilt under it's parent.
    *
    * Function tests CRM_Core_BAO_Navigation::rebuildReportsNavigation.
    */
@@ -48,7 +48,8 @@ class CRM_Core_BAO_NavigationTest extends CiviUnitTestCase {
     CRM_Core_BAO_Navigation::rebuildReportsNavigation(CRM_Core_Config::domainID());
     $parent_url = 'civicrm/report/list';
     $parent_url_params = 'compid=99&reset=1';
-    $parent_nav = CRM_Core_BAO_Navigation::getNavItemByUrl($parent_url, $parent_url_params);
+    $reportsMenu = CRM_Core_BAO_Navigation::createOrUpdateTopLevelReportsNavItem(CRM_Core_Config::domainID());
+    $parent_nav = CRM_Core_BAO_Navigation::getNavItemByUrl($parent_url, $parent_url_params, $reportsMenu->id);
     $this->assertNotEquals($parent_nav->id, 1);
     $changed_existing_nav = new CRM_Core_BAO_Navigation();
     $changed_existing_nav->id = $existing_nav->id;
-- 
2.25.1