From b50ce5b7de8a90ef50f931df5ae3e6c48cb71513 Mon Sep 17 00:00:00 2001
From: colemanw <coleman@civicrm.org>
Date: Tue, 12 Sep 2023 14:39:37 -0400
Subject: [PATCH] SearchKit - Fix link titles and icons

---
 .../Api4/Action/SearchDisplay/AbstractRunAction.php | 13 +++++++++----
 .../phpunit/api/v4/SearchDisplay/SearchRunTest.php  | 12 +++++++++++-
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
index 0c56f6be29..961d9f26cb 100644
--- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
+++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
@@ -517,13 +517,16 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction {
     $path = $this->replaceTokens($link['path'], $data, 'url', $index);
     if ($path) {
       $link['url'] = $this->getUrl($path);
-      $keys = ['url', 'text', 'title', 'target', 'style'];
+      $keys = ['url', 'text', 'title', 'target', 'icon', 'style'];
     }
     else {
-      $keys = ['task', 'text', 'title', 'style'];
+      $keys = ['task', 'text', 'title', 'icon', 'style'];
     }
     $link = array_intersect_key($link, array_flip($keys));
-    return array_filter($link);
+    return array_filter($link, function($value) {
+      // "0" is a valid title
+      return $value || (is_string($value) && strlen($value));
+    });
   }
 
   /**
@@ -688,7 +691,9 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction {
           }
         }
         elseif (!empty($task['apiBatch']) || !empty($task['uiDialog'])) {
-          $link['title'] = $link['title'] ?: $task['title'];
+          if (!strlen($link['title'])) {
+            $link['title'] = $task['title'];
+          }
           // Fill in the api action if known, for the sake of $this->checkLinkAccess
           $link['action'] = $task['apiBatch']['action'] ?? NULL;
         }
diff --git a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php
index ba60ed7c87..12593ce23a 100644
--- a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php
+++ b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php
@@ -278,8 +278,9 @@ class SearchRunTest extends Api4TestBase implements TransactionalInterface {
                 ],
                 [
                   'entity' => 'Contribution',
+                  'title' => 'Delete',
                   'action' => 'delete',
-                  'icon' => 'fa-pencil',
+                  'icon' => 'fa-trash',
                   'target' => 'crm-popup',
                 ],
               ],
@@ -297,12 +298,16 @@ class SearchRunTest extends Api4TestBase implements TransactionalInterface {
     // 1st link is to a quickform-based search task (CRM_Contribute_Task::PDF_RECEIPT)
     $this->assertArrayNotHasKey('task', $result[0]['columns'][1]['links'][0]);
     $this->assertStringContainsString('id=' . $contributions[0]['id'] . '&qfKey=', $result[0]['columns'][1]['links'][0]['url']);
+    $this->assertEquals('fa-external-link', $result[0]['columns'][1]['links'][0]['icon']);
     // 2nd link is to the native SK bulk-update task
     $this->assertArrayNotHasKey('url', $result[0]['columns'][1]['links'][1]);
     $this->assertEquals('update', $result[0]['columns'][1]['links'][1]['task']);
+    $this->assertEquals('fa-pencil', $result[0]['columns'][1]['links'][1]['icon']);
     // 3rd link is a popup link to the delete contribution quickform
     $this->assertStringContainsString('action=delete&id=' . $contributions[0]['id'], $result[0]['columns'][1]['links'][2]['url']);
     $this->assertEquals('crm-popup', $result[0]['columns'][1]['links'][2]['target']);
+    $this->assertEquals('fa-trash', $result[0]['columns'][1]['links'][2]['icon']);
+    $this->assertEquals('Delete', $result[0]['columns'][1]['links'][2]['title']);
   }
 
   public function testRelationshipCacheLinks():void {
@@ -345,6 +350,7 @@ class SearchRunTest extends Api4TestBase implements TransactionalInterface {
                   'icon' => 'fa-external-link',
                 ],
                 [
+                  'title' => '0',
                   'entity' => 'Relationship',
                   'task' => 'delete',
                   'icon' => 'fa-trash',
@@ -377,11 +383,15 @@ class SearchRunTest extends Api4TestBase implements TransactionalInterface {
     // 2nd link is to the native SK bulk-delete task
     $this->assertArrayNotHasKey('url', $result[0]['columns'][1]['links'][1]);
     $this->assertEquals('delete', $result[0]['columns'][1]['links'][1]['task']);
+    $this->assertEquals('fa-trash', $result[0]['columns'][1]['links'][1]['icon']);
+    // Ensure "empty" titles are still returned
+    $this->assertEquals('0', $result[0]['columns'][1]['links'][1]['title']);
     // 3rd link is the disable task for active relationships or the enable task for inactive ones
     $this->assertEquals('disable', $result[0]['columns'][1]['links'][2]['task']);
     $this->assertEquals('disable', $result[1]['columns'][1]['links'][2]['task']);
     $this->assertEquals('enable', $result[2]['columns'][1]['links'][2]['task']);
     $this->assertEquals('enable', $result[3]['columns'][1]['links'][2]['task']);
+    $this->assertStringContainsString('Enable', $result[3]['columns'][1]['links'][2]['title']);
   }
 
   /**
-- 
2.25.1