From: Coleman Watts <coleman@civicrm.org>
Date: Thu, 31 Mar 2022 13:42:42 +0000 (-0400)
Subject: SearchKit - Fix broken group-by caused by non-aggregated id column
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=4775ec6f46c14c550b7d6a9870c16bc9b0b6d599;p=civicrm-core.git

SearchKit - Fix broken group-by caused by non-aggregated id column

Before: The `id` column was being unconditionally added, breaking FULL_GROUP_BY sql
After: Let the Api4SelectQuery take care of it for DAO entities
---

diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
index 8a24c6b72f..8ae0fae974 100644
--- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
+++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
@@ -804,7 +804,10 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction {
     }, $apiParams['select']);
     $additions = [];
     // Add primary key field if actions are enabled
-    if (!empty($this->display['settings']['actions']) || !empty($this->display['settings']['draggable'])) {
+    // (only needed for non-dao entities, as Api4SelectQuery will auto-add the id)
+    if (!in_array('DAOEntity', CoreUtil::getInfoItem($this->savedSearch['api_entity'], 'type')) &&
+      (!empty($this->display['settings']['actions']) || !empty($this->display['settings']['draggable']))
+    ) {
       $additions = CoreUtil::getInfoItem($this->savedSearch['api_entity'], 'primary_key');
     }
     // Add draggable column (typically "weight")
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 6181ae547e..02fda6f709 100644
--- a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php
+++ b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php
@@ -2,6 +2,7 @@
 namespace api\v4\SearchDisplay;
 
 use Civi\API\Exception\UnauthorizedException;
+use Civi\Api4\Activity;
 use Civi\Api4\Contact;
 use Civi\Api4\ContactType;
 use Civi\Api4\Email;
@@ -666,6 +667,46 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter
     }
   }
 
+  public function testRunWithGroupBy() {
+    Activity::delete(FALSE)
+      ->addWhere('activity_type_id:name', 'IN', ['Meeting', 'Phone Call'])
+      ->execute();
+
+    $cid = Contact::create(FALSE)
+      ->execute()->first()['id'];
+    $sampleData = [
+      ['subject' => 'abc', 'activity_type_id:name' => 'Meeting', 'source_contact_id' => $cid],
+      ['subject' => 'def', 'activity_type_id:name' => 'Meeting', 'source_contact_id' => $cid],
+      ['subject' => 'xyz', 'activity_type_id:name' => 'Phone Call', 'source_contact_id' => $cid],
+    ];
+    $aids = Activity::save(FALSE)
+      ->setRecords($sampleData)
+      ->execute()->column('id');
+
+    $params = [
+      'checkPermissions' => FALSE,
+      'return' => 'page:1',
+      'savedSearch' => [
+        'api_entity' => 'Activity',
+        'api_params' => [
+          'version' => 4,
+          'select' => [
+            "activity_type_id:label",
+            "GROUP_CONCAT(DISTINCT subject) AS GROUP_CONCAT_subject",
+          ],
+          'groupBy' => ['activity_type_id'],
+          'orderBy' => ['activity_type_id:label'],
+          'where' => [],
+        ],
+      ],
+    ];
+
+    $result = civicrm_api4('SearchDisplay', 'run', $params);
+
+    $this->assertEquals(['abc', 'def'], $result[0]['data']['GROUP_CONCAT_subject']);
+    $this->assertEquals(['xyz'], $result[1]['data']['GROUP_CONCAT_subject']);
+  }
+
   /**
    * Test conditional styles
    */