hook_civicrm_links - Add docblocks and event-checker

diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php
index 8739833f3c0e7f8fb143e0f3fc2066704f9f337e..744691c2e362b2f04ec5b64f70dc127bb8daf8f5 100644 (file)
--- a/CRM/Utils/Hook.php
+++ b/CRM/Utils/Hook.php
@@ -409,11 +409,27 @@ abstract class CRM_Utils_Hook {
    * @param string $op
    *   The type of operation being performed.
    * @param string $objectName
-   *   The name of the object.
+   *   The name of the object. This is generally a CamelCase entity (eg `Contact` or `Activity`).
+   *   Historical exceptions: 'CRM_Core_BAO_LocationType'
    * @param int $objectId
    *   The unique identifier for the object.
    * @param array $links
    *   (optional) the links array (introduced in v3.2).
+   *   Each of the links may have properties:
+   *   - 'name' (string): the link text
+   *   - 'url' (string): the link URL base path (like civicrm/contact/view, and fillable from $values)
+   *   - 'qs' (string|array): the link URL query parameters to be used by sprintf() with $values (like reset=1&cid=%%id%% when $values['id'] is the contact ID)
+   *   - 'title' (string) (optional): the text that appears when hovering over the link
+   *   - 'extra' (optional): additional attributes for the <a> tag (fillable from $values)
+   *   - 'bit' (optional): a binary number that will be filtered by $mask (sending nothing as $links['bit'] means the link will always display)
+   *   - 'ref' (optional, recommended): a CSS class to apply to the <a> tag.
+   *   - 'class' (string): Optional list of CSS classes
+   *   - 'weight' (int): Weight is used to order the links. If not set 0 will be used but e-notices could occur. This was introduced in CiviCRM 5.63 so it will not have any impact on earlier versions of CiviCRM.
+   *   - 'accessKey' (string) (optional): HTML access key. Single letter or empty string.
+   *   - 'icon' (string) (optional): FontAwesome class name
+   *
+   *   Depending on the specific screen, some fields (e.g. `icon`) may be ignored.
+   *   If you have any idea of a clearer rule, then please update the docs.
    * @param int|null $mask
    *   (optional) the bitmask to show/hide links.
    * @param array $values

diff --git a/tests/events/hook_civicrm_links.evch.php b/tests/events/hook_civicrm_links.evch.php
new file mode 100644 (file)
index 0000000..8db817e
--- /dev/null
+++ b/tests/events/hook_civicrm_links.evch.php
@@ -0,0 +1,95 @@
+<?php
+
+use Civi\Test\EventCheck;
+use Civi\Test\HookInterface;
+
+return new class() extends EventCheck implements HookInterface {
+
+  /**
+   * Ensure that the hook data is always well-formed.
+   *
+   * @see \CRM_Utils_Hook::links()
+   */
+  public function hook_civicrm_links($op, $objectName, &$objectId, &$links, &$mask = NULL, &$values = []): void {
+    // fprintf(STDERR, "CHECK hook_civicrm_links($op)\n");
+    $msg = sprintf('Non-conforming hook_civicrm_links(%s, %s)', json_encode($op), json_encode($objectName));
+
+    // These are $objectNames that deviate from the normal "CamelCase" convention.
+    $grandfatheredObjectNames = [
+      'CRM_Core_BAO_LocationType',
+    ];
+    // These are contexts where the "url" can be replaced with an onclick handler. It evidentally works on some screens, but it doesn't sound reliable.
+    $grandfatheredOnClickLinks = [
+      'case.tab.row::Activity',
+    ];
+    // These variants have majorly deviant data in $links. But they are protected by pre-existing unit-tests.
+    $grandfatheredInvalidLinks = [
+      'pcp.user.actions::Pcp',
+    ];
+
+    $this->assertTrue((bool) preg_match(';^\w+(\.\w+)+$;', $op), "$msg: Operation ($op) should be dotted expression");
+    $this->assertTrue((bool) preg_match(';^[A-Z][a-zA-Z0-9]+$;', $objectName) || in_array($objectName, $grandfatheredObjectNames),
+      "$msg: Object name ($objectName) should be a CamelCase name or a grandfathered name");
+
+    // $this->assertType('integer|null', $objectId, "$msg: Object ID ($objectId) should be int|null");
+    $this->assertTrue($objectId === NULL || is_numeric($objectId), "$msg: Object ID ($objectId) should be int|null");
+    // Sometimes it's a string-style int. Patch-welcome if someone wants to clean that up. But this is what it currently does.
+
+    $this->assertType('array', $links, "$msg: Links should be an array");
+    $this->assertType('integer|null', $mask, "$msg: Mask ($mask) should be int}null");
+    $this->assertType('array', $values, "$msg: Values should be an array");
+
+    if (in_array("$op::$objectName", $grandfatheredInvalidLinks)) {
+      return;
+    }
+    foreach ($links as $link) {
+      if (isset($link['name'])) {
+        $this->assertType('string', $link['name'], "$msg: name should be a string");
+      }
+      else {
+        $this->fail("$msg: name is missing");
+      }
+
+      if (isset($link['url'])) {
+        $this->assertType('string', $link['url'], "$msg: url should be a string");
+      }
+      elseif (in_array("$op::$objectName", $grandfatheredOnClickLinks)) {
+        $this->assertTrue((bool) preg_match(';onclick;', $link['extra']), "$msg: ");
+      }
+      else {
+        $this->fail("$msg: url is missing");
+      }
+
+      if (isset($link['qs'])) {
+        $this->assertType('string|array', $link['qs'], "$msg: qs should be a string");
+      }
+      if (isset($link['title'])) {
+        $this->assertType('string', $link['title'], "$msg: title should be a string");
+      }
+      if (isset($link['extra'])) {
+        $this->assertType('string', $link['extra'], "$msg: extra should be a string");
+      }
+      if (isset($link['bit'])) {
+        $this->assertType('integer', $link['bit'], "$msg: bit should be an int");
+      }
+      if (isset($link['ref'])) {
+        $this->assertType('string', $link['ref'], "$msg: ref should be an string");
+      }
+      if (isset($link['class'])) {
+        $this->assertType('string', $link['class'], "$msg: class should be a string");
+      }
+      $this->assertTrue(isset($link['weight']) && is_numeric($link['weight']), "$msg: weight should be numerical");
+      if (isset($link['accessKey'])) {
+        $this->assertTrue(is_string($link['accessKey']) && mb_strlen($link['accessKey']) <= 1, "$msg: accessKey should be a letter");
+      }
+      if (isset($link['icon'])) {
+        $this->assertTrue((bool) preg_match(';^fa-[-a-z0-9]+$;', $link['icon']), "$msg: Icon ({$link['icon']}) should be FontAwesome icon class");
+      }
+
+      $expectKeys = ['name', 'url', 'qs', 'title', 'extra', 'bit', 'ref', 'class', 'weight', 'accessKey', 'icon'];
+      $extraKeys = array_diff(array_keys($link), $expectKeys);
+      $this->assertEquals([], $extraKeys, "$msg: Link has unrecognized keys: " . json_encode($extraKeys));
+    }
+  }
+
+};