From: Coleman Watts Date: Tue, 3 Nov 2020 17:11:56 +0000 (-0500) Subject: Remove unused hook_civicrm_crudLink and switch to using metadata for generating crudLinks X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=002f5eaf8bee947f1eea6ea157d8f2f846d41bd7;p=civicrm-core.git Remove unused hook_civicrm_crudLink and switch to using metadata for generating crudLinks The hook civicrm_crudLink is unused according to a grep of 'civicrm_universe', and the function CRM_Utils_System::createDefaultCrudLink had poor entity coverage (it could only generate links for the Contact entity). Switching that function to use the new 'paths' metadata vastly improves coverage. --- diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php index 988cab9a5f..8d1095747e 100644 --- a/CRM/Contact/BAO/Contact.php +++ b/CRM/Contact/BAO/Contact.php @@ -714,47 +714,6 @@ WHERE civicrm_contact.id = " . CRM_Utils_Type::escape($id, 'Integer'); return TRUE; } - /** - * Create last viewed link to recently updated contact. - * - * @param array $crudLinkSpec - * - action: int, CRM_Core_Action::UPDATE or CRM_Core_Action::VIEW [default: VIEW] - * - entity_table: string, eg "civicrm_contact" - * - entity_id: int - * - * @return array|NULL - * NULL if unavailable, or - * [path: string, query: string, title: string] - * @see CRM_Utils_System::createDefaultCrudLink - */ - public function createDefaultCrudLink($crudLinkSpec) { - switch ($crudLinkSpec['action']) { - case CRM_Core_Action::VIEW: - $result = [ - 'title' => $this->display_name, - 'path' => 'civicrm/contact/view', - 'query' => [ - 'reset' => 1, - 'cid' => $this->id, - ], - ]; - return $result; - - case CRM_Core_Action::UPDATE: - $result = [ - 'title' => $this->display_name, - 'path' => 'civicrm/contact/add', - 'query' => [ - 'reset' => 1, - 'action' => 'update', - 'cid' => $this->id, - ], - ]; - return $result; - } - return NULL; - } - /** * Get the values for pseudoconstants for name->value and reverse. * diff --git a/CRM/Core/Smarty/plugins/function.crmCrudLink.php b/CRM/Core/Smarty/plugins/function.crmCrudLink.php index 69c57aafc5..46908c6cfe 100644 --- a/CRM/Core/Smarty/plugins/function.crmCrudLink.php +++ b/CRM/Core/Smarty/plugins/function.crmCrudLink.php @@ -20,29 +20,26 @@ * * @param array $params * Array with keys: - * - table: string + * - entity|table: string * - id: int - * - action: string, 'VIEW' or 'UPDATE' [default: VIEW] + * - action: string, 'view', 'update', 'delete', etc [default: view] * - title: string [optionally override default title] * @param CRM_Core_Smarty $smarty * * @return string */ function smarty_function_crmCrudLink($params, &$smarty) { - if (empty($params['action'])) { - $params['action'] = 'VIEW'; - } - $link = CRM_Utils_System::createDefaultCrudLink([ - 'action' => constant('CRM_Core_Action::' . $params['action']), - 'entity_table' => $params['table'], - 'entity_id' => $params['id'], + 'action' => $params['action'] ?? 'view', + 'entity' => $params['entity'] ?? NULL, + 'entity_table' => $params['table'] ?? NULL, + 'id' => $params['id'], ]); if ($link) { return sprintf('%s', htmlspecialchars($link['url']), - htmlspecialchars(CRM_Utils_Array::value('title', $params, $link['title'])) + htmlspecialchars($params['title'] ?? $link['title']) ); } else { diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php index bc905d21ba..088a5d975e 100644 --- a/CRM/Utils/Hook.php +++ b/CRM/Utils/Hook.php @@ -2409,32 +2409,6 @@ abstract class CRM_Utils_Hook { \Civi::dispatcher()->dispatch('hook_civicrm_caseChange', $event); } - /** - * Generate a default CRUD URL for an entity. - * - * @param array $spec - * With keys:. - * - action: int, eg CRM_Core_Action::VIEW or CRM_Core_Action::UPDATE - * - entity_table: string - * - entity_id: int - * @param CRM_Core_DAO $bao - * @param array $link - * To define the link, add these keys to $link:. - * - title: string - * - path: string - * - query: array - * - url: string (used in lieu of "path"/"query") - * Note: if making "url" CRM_Utils_System::url(), set $htmlize=false - * @return mixed - * @deprecated - */ - public static function crudLink($spec, $bao, &$link) { - return self::singleton()->invoke(['spec', 'bao', 'link'], $spec, $bao, $link, - self::$_nullObject, self::$_nullObject, self::$_nullObject, - 'civicrm_crudLink' - ); - } - /** * Modify the CiviCRM container - add new services, parameters, extensions, etc. * diff --git a/CRM/Utils/System.php b/CRM/Utils/System.php index 2ca112f31a..47e72900f0 100644 --- a/CRM/Utils/System.php +++ b/CRM/Utils/System.php @@ -1859,57 +1859,69 @@ class CRM_Utils_System { } /** - * Determine the standard URL for viewing or editing the specified link. - * - * This function delegates the decision-making to (a) the hook system and - * (b) the BAO system. + * Determine the standard URL for view/update/delete of a given entity. * * @param array $crudLinkSpec * With keys:. - * - action: int, CRM_Core_Action::UPDATE or CRM_Core_Action::VIEW [default: VIEW] - * - entity_table: string, eg "civicrm_contact" - * - entity_id: int + * - action: sting|int, e.g. 'update' or CRM_Core_Action::UPDATE or 'view' or CRM_Core_Action::VIEW [default: 'view'] + * - entity|entity_table: string, eg "Contact" or "civicrm_contact" + * - id|entity_id: int + * + * @param bool $absolute whether the generated link should have an absolute (external) URL beginning with http * * @return array|NULL * NULL if unavailable, or an array. array has keys: - * - path: string - * - query: array * - title: string * - url: string - * @deprecated */ - public static function createDefaultCrudLink($crudLinkSpec) { - $crudLinkSpec['action'] = CRM_Utils_Array::value('action', $crudLinkSpec, CRM_Core_Action::VIEW); - $daoClass = CRM_Core_DAO_AllCoreTables::getClassForTable($crudLinkSpec['entity_table']); - if (!$daoClass) { - return NULL; + public static function createDefaultCrudLink($crudLinkSpec, $absolute = FALSE) { + $action = $crudLinkSpec['action'] ?? 'view'; + if (is_numeric($action)) { + $action = CRM_Core_Action::description($action); } - - $baoClass = str_replace('_DAO_', '_BAO_', $daoClass); - if (!class_exists($baoClass)) { - return NULL; + else { + $action = strtolower($action); } - $bao = new $baoClass(); - $bao->id = $crudLinkSpec['entity_id']; - if (!$bao->find(TRUE)) { + $daoClass = isset($crudLinkSpec['entity']) ? CRM_Core_DAO_AllCoreTables::getFullName($crudLinkSpec['entity']) : CRM_Core_DAO_AllCoreTables::getClassForTable($crudLinkSpec['entity_table']); + $paths = $daoClass ? $daoClass::getEntityPaths() : []; + $path = $paths[$action] ?? NULL; + if (!$path) { return NULL; } - $link = []; - CRM_Utils_Hook::crudLink($crudLinkSpec, $bao, $link); - if (empty($link) && is_callable([$bao, 'createDefaultCrudLink'])) { - $link = $bao->createDefaultCrudLink($crudLinkSpec); + if (empty($crudLinkSpec['id']) && !empty($crudLinkSpec['entity_id'])) { + $crudLinkSpec['id'] = $crudLinkSpec['entity_id']; + } + foreach ($crudLinkSpec as $key => $value) { + $path = str_replace('[' . $key . ']', $value, $path); } - if (!empty($link)) { - if (!isset($link['url'])) { - $link['url'] = self::url($link['path'], $link['query'], TRUE, NULL, FALSE); - } - return $link; + switch ($action) { + case 'add': + $title = ts('New %1', [1 => $daoClass::getEntityTitle()]); + break; + + case 'view': + $title = ts('View %1', [1 => $daoClass::getEntityTitle()]); + break; + + case 'update': + $title = ts('Edit %1', [1 => $daoClass::getEntityTitle()]); + break; + + case 'delete': + $title = ts('Delete %1', [1 => $daoClass::getEntityTitle()]); + break; + + default: + $title = ts(ucfirst($action)) . ' ' . $daoClass::getEntityTitle(); } - return NULL; + return [ + 'title' => $title, + 'url' => self::url($path, NULL, $absolute, NULL, FALSE), + ]; } /** diff --git a/tests/phpunit/CRM/Utils/SystemTest.php b/tests/phpunit/CRM/Utils/SystemTest.php index f163a16798..f398c3d990 100644 --- a/tests/phpunit/CRM/Utils/SystemTest.php +++ b/tests/phpunit/CRM/Utils/SystemTest.php @@ -250,6 +250,35 @@ class CRM_Utils_SystemTest extends CiviUnitTestCase { $this->assertTrue(CRM_Utils_System::isNull($arr)); } + public function crudLinkExamples() { + return [ + 'Contact:update' => [ + ['entity' => 'Contact', 'action' => 'update', 'id' => 123], + ['title' => 'Edit Contact', 'url' => '/index.php?q=civicrm/contact/add&reset=1&action=update&cid=123'], + ], + 'civicrm_contact:UPDATE' => [ + ['entity_table' => 'civicrm_contact', 'action' => CRM_Core_Action::UPDATE, 'entity_id' => 123], + ['title' => 'Edit Contact', 'url' => '/index.php?q=civicrm/contact/add&reset=1&action=update&cid=123'], + ], + 'civicrm_activity:ADD' => [ + ['entity_table' => 'civicrm_activity', 'action' => CRM_Core_Action::ADD], + ['title' => 'New Activity', 'url' => '/index.php?q=civicrm/activity&reset=1&action=add&context=standalone'], + ], + 'Contribution:delete' => [ + ['entity' => 'Contribution', 'action' => 'DELETE', 'id' => 456], + ['title' => 'Delete Contribution', 'url' => '/index.php?q=civicrm/contact/view/contribution&reset=1&action=delete&id=456'], + ], + ]; + } + + /** + * @dataProvider crudLinkExamples + */ + public function testCrudLink($params, $expectedResult) { + $result = CRM_Utils_System::createDefaultCrudLink($params); + $this->assertEquals($expectedResult, $result); + } + /** * Test that flushing cache clears the asset cache. */ @@ -258,7 +287,7 @@ class CRM_Utils_SystemTest extends CiviUnitTestCase { // method to get it, so create a file in the folder using public methods, // then get the path from that, then flush the cache, then check if the // folder is empty. - \Civi::dispatcher()->addListener('hook_civicrm_buildAsset', array($this, 'flushCacheClearsAssetCache_buildAsset')); + \Civi::dispatcher()->addListener('hook_civicrm_buildAsset', [$this, 'flushCacheClearsAssetCache_buildAsset']); $fakeFile = \Civi::service("asset_builder")->getPath('fakeFile.json'); CRM_Utils_System::flushCache();