From 5511ef1e62ad75398843004926e5aa389dfeefb5 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Fri, 25 Nov 2022 11:35:35 -0500
Subject: [PATCH] SearchKit - Fix Campaign, State, Country selectors (again)

This fixes the attempted fix from 7deb8c37 which broke more than it fixed.

The goal was to use ajax to load state, country & county lists because they are too long to prefetch.
But switching ALL FK fields to use ajax instead of option suffixes was too heavy-handed and would
have broken a lot of existing searches by no longer supporting those pseudoconstants in the UI.

This restores all previous option list fields, and targets only address country/county/state fields.
Ideally we'd turn the prefetch flag off on those fields but that would have major consequenses for
formBuilder and existing searches.
So this uses a subtler approach and tweaks their "suffixes" metadata to remove :name which didn't really
make sense for those fields anyway since they don't actually have machine names.
Then it teaches SearchKit to use ajax for selecting from fields with no :name, while still using :label
for their display value in the table.
---
 .../Service/Spec/Provider/AddressGetSpecProvider.php |  7 ++++++-
 ext/search_kit/ang/crmSearchAdmin.module.js          |  2 +-
 .../ang/crmSearchAdmin/crmSearchAdmin.component.js   |  4 ++--
 .../crmSearchInput/crmSearchInputVal.component.js    | 12 +++++-------
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/Civi/Api4/Service/Spec/Provider/AddressGetSpecProvider.php b/Civi/Api4/Service/Spec/Provider/AddressGetSpecProvider.php
index abf21de58477..82cdd50cf654 100644
--- a/Civi/Api4/Service/Spec/Provider/AddressGetSpecProvider.php
+++ b/Civi/Api4/Service/Spec/Provider/AddressGetSpecProvider.php
@@ -27,7 +27,12 @@ class AddressGetSpecProvider extends \Civi\Core\Service\AutoService implements G
    * @param \Civi\Api4\Service\Spec\RequestSpec $spec
    */
   public function modifySpec(RequestSpec $spec) {
-    // Groups field
+    // These fields do not have any sensible "name" pseudoconstant so should just use the ID
+    foreach (['state_province_id', 'country_id', 'county_id'] as $name) {
+      $spec->getFieldByName($name)->setSuffixes(['label', 'abbr']);
+    }
+
+    // Proximity search field
     $field = new FieldSpec('proximity', 'Address', 'Boolean');
     $field->setLabel(ts('Address Proximity'))
       ->setTitle(ts('Address Proximity'))
diff --git a/ext/search_kit/ang/crmSearchAdmin.module.js b/ext/search_kit/ang/crmSearchAdmin.module.js
index f4fffa91f055..88f3ec77c500 100644
--- a/ext/search_kit/ang/crmSearchAdmin.module.js
+++ b/ext/search_kit/ang/crmSearchAdmin.module.js
@@ -401,7 +401,7 @@
                 // For fields with both an FK and an option list, prefer the FK
                 // because it's more efficient to render an autocomplete than to
                 // pre-load potentially thousands of options into a select dropdown.
-                where: [['options', '!=', false], ['fk_entity', 'IS NULL']],
+                where: [['options', '!=', false], ['suffixes', 'CONTAINS', 'name']],
                 select: ['options']
               }, {name: 'options'}];
             }
diff --git a/ext/search_kit/ang/crmSearchAdmin/crmSearchAdmin.component.js b/ext/search_kit/ang/crmSearchAdmin/crmSearchAdmin.component.js
index a8e0893b9862..32fde96e7efe 100644
--- a/ext/search_kit/ang/crmSearchAdmin/crmSearchAdmin.component.js
+++ b/ext/search_kit/ang/crmSearchAdmin/crmSearchAdmin.component.js
@@ -532,8 +532,8 @@
           prefix = typeof prefix === 'undefined' ? '' : prefix;
           _.each(fields, function(field) {
             var item = {
-              // Use suffix only if the field is not an FK. EntityRef fields look-up by id.
-              id: prefix + field.name + (!field.fk_entity && _.includes(field.suffixes || [], suffix.replace(':', '')) ? suffix : ''),
+              // Use options suffix if available.
+              id: prefix + field.name + (field.options && _.includes(field.suffixes || [], suffix.replace(':', '')) ? suffix : ''),
               text: field.label,
               description: field.description
             };
diff --git a/ext/search_kit/ang/crmSearchTasks/crmSearchInput/crmSearchInputVal.component.js b/ext/search_kit/ang/crmSearchTasks/crmSearchInput/crmSearchInputVal.component.js
index b48165a03045..f55cce68d4bb 100644
--- a/ext/search_kit/ang/crmSearchTasks/crmSearchInput/crmSearchInputVal.component.js
+++ b/ext/search_kit/ang/crmSearchTasks/crmSearchInput/crmSearchInputVal.component.js
@@ -143,15 +143,13 @@
         }
 
         if (!_.includes(['>', '<', '>=', '<='], ctrl.op)) {
-          // For fields with both an FK and an option list, prefer the FK
-          // because it's more efficient to render an autocomplete than to
-          // pre-load potentially thousands of options into a select dropdown.
-          if ((field.fk_entity || field.name === 'id')) {
-            return '~/crmSearchTasks/crmSearchInput/entityRef.html';
-          }
-          if (field.options) {
+          // Only use option list if the field has a "name" suffix
+          if (field.options && field.suffixes && field.suffixes.includes('name')) {
             return '~/crmSearchTasks/crmSearchInput/select.html';
           }
+          if (field.fk_entity || field.name === 'id') {
+            return '~/crmSearchTasks/crmSearchInput/entityRef.html';
+          }
         }
 
         if (field.data_type === 'Integer') {