From 959990ca22fdf80b8bc1ca2a17939bdb5b73e432 Mon Sep 17 00:00:00 2001
From: colemanw <coleman@civicrm.org>
Date: Mon, 25 Sep 2023 10:02:21 -0400
Subject: [PATCH] Autocomplete - Fix search-by-id for entities without a
 numeric id

Fixes dev/core#4560 and the intermittent failure of AfformGetTest.testAfformAutocomplete
---
 Civi/Api4/EntitySet.php                       |  8 +++++++
 Civi/Api4/Generic/AutocompleteAction.php      | 24 ++++++++++++-------
 .../phpunit/Civi/Afform/AfformGetTest.php     |  4 +++-
 .../phpunit/api/v4/Entity/ConformanceTest.php |  1 -
 4 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/Civi/Api4/EntitySet.php b/Civi/Api4/EntitySet.php
index b77fdae15a..f25208137c 100644
--- a/Civi/Api4/EntitySet.php
+++ b/Civi/Api4/EntitySet.php
@@ -39,6 +39,7 @@ class EntitySet extends Generic\AbstractEntity {
   }
 
   /**
+   * This isn't a "real" entity and doesn't have any fields
    * @return \Civi\Api4\Generic\BasicGetFieldsAction
    */
   public static function getFields($checkPermissions = TRUE) {
@@ -61,4 +62,11 @@ class EntitySet extends Generic\AbstractEntity {
     return $plural ? ts('Entity Sets') : ts('Entity Set');
   }
 
+  public static function getInfo(): array {
+    $info = parent::getInfo();
+    // This isn't a "real" entity and doesn't have any fields, so no primary key
+    $info['primary_key'] = [];
+    return $info;
+  }
+
 }
diff --git a/Civi/Api4/Generic/AutocompleteAction.php b/Civi/Api4/Generic/AutocompleteAction.php
index 313f6a67ab..73ce6e9d4d 100644
--- a/Civi/Api4/Generic/AutocompleteAction.php
+++ b/Civi/Api4/Generic/AutocompleteAction.php
@@ -149,23 +149,29 @@ class AutocompleteAction extends AbstractAction {
     else {
       // Default search and sort field
       $labelField = $this->display['settings']['columns'][0]['key'];
-      $idField = CoreUtil::getIdFieldName($this->savedSearch['api_entity']);
+      $primaryKeys = CoreUtil::getInfoItem($this->savedSearch['api_entity'], 'primary_key');
       $this->display['settings'] += [
         'sort' => [$labelField, 'ASC'],
       ];
       // Always search on the first line of the display
       $searchFields = [$labelField];
-      // If input is an integer, search by id
-      $numericInput = $this->page == 1 && \CRM_Utils_Rule::positiveInteger($this->input);
-      if ($numericInput) {
-        $searchFields = [$idField];
+      // If input is an integer...
+      $searchById = \CRM_Utils_Rule::positiveInteger($this->input) &&
+        // ...and there is exactly one primary key (e.g. EntitySet has zero, others might have compound keys)
+        count($primaryKeys) === 1 &&
+        // ...and the primary key field is type Integer (e.g. Afform.name is a String)
+        ($this->getField($primaryKeys[0])['data_type'] ?? NULL) === 'Integer';
+      // ...then search by primary key on first page
+      $initialSearchById = $searchById && $this->page == 1;
+      if ($initialSearchById) {
+        $searchFields = $primaryKeys;
       }
-      // For subsequent pages when searching numeric input
-      elseif ($this->page > 1 && \CRM_Utils_Rule::positiveInteger($this->input)) {
+      // For subsequent pages when searching by id, subtract the "extra" first page
+      elseif ($searchById && $this->page > 1) {
         $this->page -= 1;
       }
       // If first line uses a rewrite, search on those fields too
-      if (!empty($this->display['settings']['columns'][0]['rewrite'])) {
+      if (!$initialSearchById && !empty($this->display['settings']['columns'][0]['rewrite'])) {
         $searchFields = array_merge($searchFields, $this->getTokens($this->display['settings']['columns'][0]['rewrite']));
       }
       $this->display['settings']['limit'] = $this->display['settings']['limit'] ?? \Civi::settings()->get('search_autocomplete_count');
@@ -198,7 +204,7 @@ class AutocompleteAction extends AbstractAction {
       $result[] = $item;
     }
     $result->setCountMatched($apiResult->count());
-    if (!empty($numericInput)) {
+    if (!empty($initialSearchById)) {
       // Trigger "more results" after searching by exact id
       $result->setCountMatched($apiResult->count() + 1);
     }
diff --git a/ext/afform/core/tests/phpunit/Civi/Afform/AfformGetTest.php b/ext/afform/core/tests/phpunit/Civi/Afform/AfformGetTest.php
index f93fea9bdf..fa5766e857 100644
--- a/ext/afform/core/tests/phpunit/Civi/Afform/AfformGetTest.php
+++ b/ext/afform/core/tests/phpunit/Civi/Afform/AfformGetTest.php
@@ -72,7 +72,9 @@ class AfformGetTest extends \PHPUnit\Framework\TestCase implements HeadlessInter
   }
 
   public function testAfformAutocomplete(): void {
-    $title = uniqid();
+    // Use a numeric title to test that the "search by id" feature
+    // doesn't kick in for Afforms (which don't have a numeric "id")
+    $title = (string) rand(1000, 999999);
     Afform::create()
       ->addValue('name', $this->formName)
       ->addValue('title', $title)
diff --git a/tests/phpunit/api/v4/Entity/ConformanceTest.php b/tests/phpunit/api/v4/Entity/ConformanceTest.php
index 75fa3b9ca3..da6725ba44 100644
--- a/tests/phpunit/api/v4/Entity/ConformanceTest.php
+++ b/tests/phpunit/api/v4/Entity/ConformanceTest.php
@@ -169,7 +169,6 @@ class ConformanceTest extends Api4TestBase implements HookInterface {
     $this->assertNotEmpty($info['type']);
     $this->assertNotEmpty($info['description']);
     $this->assertIsArray($info['primary_key']);
-    $this->assertNotEmpty($info['primary_key']);
     $this->assertMatchesRegularExpression(';^\d\.\d+$;', $info['since']);
     $this->assertContains($info['searchable'], ['primary', 'secondary', 'bridge', 'none']);
   }
-- 
2.25.1