From: Coleman Watts <coleman@civicrm.org>
Date: Tue, 19 Apr 2022 14:07:13 +0000 (-0400)
Subject: SearchKit - Prevent errors trying to orderBy nonaggregated columns when using groupBy
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=aaf0f2504a0e795e417406093266e4bea818a1c4;p=civicrm-core.git

SearchKit - Prevent errors trying to orderBy nonaggregated columns when using groupBy
---

diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
index 6d63a56d8f..63a59d9da4 100644
--- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
+++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
@@ -854,13 +854,13 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction {
     }
 
     $defaultSort = $this->display['settings']['sort'] ?? [];
-    $currentSort = $this->sort;
+    $currentSort = [];
 
-    // Verify requested sort corresponds to sortable columns
+    // Add requested sort after verifying it corresponds to sortable columns
     foreach ($this->sort as $item) {
       $column = array_column($this->display['settings']['columns'], NULL, 'key')[$item[0]] ?? NULL;
-      if (!$column || (isset($column['sortable']) && !$column['sortable'])) {
-        $currentSort = NULL;
+      if ($column && !(isset($column['sortable']) && !$column['sortable'])) {
+        $currentSort[] = $item;
       }
     }
 
@@ -870,6 +870,10 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction {
       if ($item[0] === 'RAND()' && isset($this->seed)) {
         $item[0] = 'RAND(' . $this->seed . ')';
       }
+      // Prevent errors trying to orderBy nonaggregated columns when using groupBy
+      if ($this->canAggregate($item[0])) {
+        continue;
+      }
       $orderBy[$item[0]] = $item[1];
     }
     return $orderBy;
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 0f7cc73637..2b8a2c02c6 100644
--- a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php
+++ b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php
@@ -1040,4 +1040,43 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter
     $this->assertEquals('Contact', $result[0]['columns'][0]['val']);
   }
 
+  public function testGroupByContactType(): void {
+    $source = uniqid(__FUNCTION__);
+    $sampleData = [
+      ['contact_type' => 'Individual'],
+      ['contact_type' => 'Individual'],
+      ['contact_type' => 'Individual'],
+      ['contact_type' => 'Organization'],
+      ['contact_type' => 'Organization'],
+      ['contact_type' => 'Household'],
+    ];
+    Contact::save(FALSE)
+      ->addDefault('source', $source)
+      ->setRecords($sampleData)
+      ->execute();
+
+    $params = [
+      'checkPermissions' => FALSE,
+      'return' => 'page:1',
+      'savedSearch' => [
+        'api_entity' => 'Contact',
+        'api_params' => [
+          'version' => 4,
+          'select' => ['contact_type:label', 'COUNT(id) AS COUNT_id'],
+          'where' => [['source', '=', $source]],
+          'groupBy' => ['contact_type'],
+        ],
+      ],
+      'display' => NULL,
+      'afform' => NULL,
+    ];
+
+    $result = civicrm_api4('SearchDisplay', 'run', $params);
+    $this->assertCount(3, $result);
+    $data = array_column(array_column((array) $result, 'data'), 'COUNT_id', 'contact_type:label');
+    $this->assertEquals(3, $data['Individual']);
+    $this->assertEquals(2, $data['Organization']);
+    $this->assertEquals(1, $data['Household']);
+  }
+
 }