From 51704c4d261138b156eceaacfc8ab162c7854cb0 Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 14 Oct 2015 15:52:08 +1300 Subject: [PATCH] CRM-17225 update report reset to cope with extra params on the url --- CRM/Core/BAO/Navigation.php | 13 +++-- tests/phpunit/CRM/Core/BAO/NavigationTest.php | 48 ++++++++++++++++++- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/CRM/Core/BAO/Navigation.php b/CRM/Core/BAO/Navigation.php index c2047bf107..ca069fc757 100644 --- 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 8533d75a9b..9183de7925 100644 --- 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. * -- 2.25.1