CRM-17225 update report reset to cope with extra params on the url

diff --git a/CRM/Core/BAO/Navigation.php b/CRM/Core/BAO/Navigation.php
index c2047bf107e441522c7d0b3a235c96919111b6fc..ca069fc7571cb89094dab6d593f79612db5316ff 100644 (file)
--- a/CRM/Core/BAO/Navigation.php
+++ b/CRM/Core/BAO/Navigation.php
@@ -914,7 +914,7 @@ ORDER BY parent_id, weight";
         "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);
+        $report_nav = self::createOrUpdateReportNavItem($report['title'], $report['url'], 'reset=1', $component_nav->id, $report['permission'], $domain_id, FALSE, TRUE);
         // Update the report instance to include the navigation id.
         $query = "UPDATE civicrm_report_instance SET navigation_id = %1 WHERE id = %2";
         $params = array(
@@ -956,6 +956,9 @@ ORDER BY parent_id, weight";
   /**
    * Retrieve a navigation item using it's url.
    *
+   * Note that we use LIKE to permit a wildcard as the calling code likely doesn't
+   * care about output params appended.
+   *
    * @param string $url
    * @param array $url_params
    *
@@ -966,8 +969,9 @@ ORDER BY parent_id, weight";
    */
   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;
+    $nav->whereAdd("url LIKE '{$url}?{$url_params}'");
+
     if ($nav->find(TRUE)) {
       return $nav;
     }
@@ -1036,9 +1040,10 @@ ORDER BY parent_id, weight";
    * @return \CRM_Core_DAO_Navigation
    */
   protected static function createOrUpdateReportNavItem($name, $url, $url_params, $parent_id, $permission,
-                                                        $domain_id, $onlyMatchParentID = FALSE) {
+                                                        $domain_id, $onlyMatchParentID = FALSE, $useWildcard = TRUE) {
     $id = NULL;
-    $existing_nav = CRM_Core_BAO_Navigation::getNavItemByUrl($url, $url_params, ($onlyMatchParentID ? $parent_id : NULL));
+    $existing_url_params = $useWildcard ? $url_params . '%' : $url_params;
+    $existing_nav = CRM_Core_BAO_Navigation::getNavItemByUrl($url, $existing_url_params, ($onlyMatchParentID ? $parent_id : NULL));
     if ($existing_nav) {
       $id = $existing_nav->id;
     }

diff --git a/tests/phpunit/CRM/Core/BAO/NavigationTest.php b/tests/phpunit/CRM/Core/BAO/NavigationTest.php
index 8533d75a9b7c87be87da330caa9cb8fc1322784c..9183de792556c6f5b8f1c38ea2717169f36836ab 100644 (file)
--- a/tests/phpunit/CRM/Core/BAO/NavigationTest.php
+++ b/tests/phpunit/CRM/Core/BAO/NavigationTest.php
@@ -21,7 +21,7 @@ class CRM_Core_BAO_NavigationTest extends CiviUnitTestCase {
    */
   public function testCreateMissingReportMenuItemLink() {
     $reportCount = $this->getCountReportInstances();
-    CRM_Core_DAO::executeQuery("DELETE FROM civicrm_navigation WHERE url = 'civicrm/report/instance/1?reset=1'");
+    CRM_Core_DAO::executeQuery("DELETE FROM civicrm_navigation WHERE url LIKE 'civicrm/report/instance/1?reset=1%'");
     $this->assertEquals($reportCount - 1, $this->getCountReportInstances());
     CRM_Core_BAO_Navigation::rebuildReportsNavigation(CRM_Core_Config::domainID());
 
@@ -33,6 +33,22 @@ class CRM_Core_BAO_NavigationTest extends CiviUnitTestCase {
     $this->assertNotNull($new_nav->id);
   }
 
+  /**
+   * Test that a link with output=criteria at the end is not duplicated.
+   */
+  public function testNoDuplicateReportMenuItemLink() {
+    CRM_Core_BAO_Navigation::rebuildReportsNavigation(CRM_Core_Config::domainID());
+    $reportCount = $this->getCountReportInstances();
+    CRM_Core_DAO::executeQuery("
+      UPDATE civicrm_navigation
+      SET url = CONCAT(url, '&output=critieria')
+      WHERE url LIKE 'civicrm/report/instance/%?reset=1'");
+    $this->assertEquals($reportCount, $this->getCountReportInstances());
+    CRM_Core_BAO_Navigation::rebuildReportsNavigation(CRM_Core_Config::domainID());
+
+    $this->assertEquals($reportCount, $this->getCountReportInstances());
+  }
+
   /**
    * Test that an existing report link is rebuilt under it's parent.
    *
@@ -42,6 +58,7 @@ class CRM_Core_BAO_NavigationTest extends CiviUnitTestCase {
     $url = 'civicrm/report/instance/1';
     $url_params = 'reset=1';
     $existing_nav = CRM_Core_BAO_Navigation::getNavItemByUrl($url, $url_params);
+
     $this->assertNotEquals(FALSE, $existing_nav);
     $existing_nav->parent_id = 1;
     $existing_nav->save();
@@ -57,7 +74,6 @@ class CRM_Core_BAO_NavigationTest extends CiviUnitTestCase {
     $this->assertEquals($changed_existing_nav->parent_id, $parent_nav->id);
   }
 
-
   /**
    * Test that a navigation item can be retrieved by it's url.
    */
@@ -83,6 +99,34 @@ class CRM_Core_BAO_NavigationTest extends CiviUnitTestCase {
     $new_nav->delete();
   }
 
+  /**
+   * Test that a navigation item can be retrieved by it's url with a wildcard.
+   *
+   * We want to be able to get a report url with OR without the output=criteria since
+   * that is part of the navigation but not the instance.
+   */
+  public function testGetNavItemByUrlWildcard() {
+    $random_string = substr(sha1(rand()), 0, 7);
+    $name = "Test Menu Link {$random_string}";
+    $url = "civicrm/test/{$random_string}";
+    $url_params = "reset=1&output=criteria";
+    $params = array(
+      'name' => $name,
+      'label' => ts($name),
+      'url' => "{$url}?{$url_params}",
+      'parent_id' => NULL,
+      'is_active' => TRUE,
+      'permission' => array(
+        'access CiviCRM',
+      ),
+    );
+    CRM_Core_BAO_Navigation::add($params);
+    $new_nav = CRM_Core_BAO_Navigation::getNavItemByUrl($url, 'reset=1%');
+    $this->assertObjectHasAttribute('id', $new_nav);
+    $this->assertNotNull($new_nav->id);
+    $new_nav->delete();
+  }
+
   /**
    * Get a count of report instances.
    *