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 988cab9a5f527e969886de1bf583749514652a1a..8d1095747eca8b887c024fdc162699e959c200d4 100644 (file)
--- 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 69c57aafc515814c1e92c3a7c9c54166f5203f7a..46908c6cfe5af0d16b388f7e4cd1bfc02e14263f 100644 (file)
--- 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('<a href="%s">%s</a>',
       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 bc905d21baa3f7d259071d8e8bcf72140d037dba..088a5d975e02b295a43b64ba893c3d29b5972caf 100644 (file)
--- 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 2ca112f31a01914c6fe4a2308b4ae0386987c4a0..47e72900f0db4c666f233fdc7d20b04496f87373 100644 (file)
--- 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 f163a167989c04c2a8fa05f40bb23779fec5849d..f398c3d990bc92a13558d3ff22630a2c494d4de1 100644 (file)
--- 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();