From 259207d064db3dbd7a8cd85d1dda84192a66b5a5 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Tue, 15 Jun 2021 02:22:08 -0400
Subject: [PATCH] APIv4 - Preserve field data type when aggregating into an
 array using GROUP_CONCAT

Ensures that e.g. an array of integer fields will be returned as integers and not an array of strings
---
 Civi/Api4/Query/SqlFunctionCOUNT.php            | 5 ++++-
 Civi/Api4/Query/SqlFunctionGROUP_CONCAT.php     | 8 +++++++-
 Civi/Api4/Utils/FormattingUtil.php              | 5 ++---
 tests/phpunit/api/v4/Action/SqlFunctionTest.php | 4 ++++
 4 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/Civi/Api4/Query/SqlFunctionCOUNT.php b/Civi/Api4/Query/SqlFunctionCOUNT.php
index 2ab3d66163..19ddd4ceba 100644
--- a/Civi/Api4/Query/SqlFunctionCOUNT.php
+++ b/Civi/Api4/Query/SqlFunctionCOUNT.php
@@ -32,9 +32,12 @@ class SqlFunctionCOUNT extends SqlFunction {
    *
    * @see \Civi\Api4\Utils\FormattingUtil::formatOutputValues
    * @param string $value
+   * @param string $dataType
    * @return string|array
    */
-  public function formatOutputValue($value) {
+  public function formatOutputValue($value, &$dataType) {
+    // Count is always an integer
+    $dataType = 'Integer';
     return (int) $value;
   }
 
diff --git a/Civi/Api4/Query/SqlFunctionGROUP_CONCAT.php b/Civi/Api4/Query/SqlFunctionGROUP_CONCAT.php
index fb618e0ac7..1b52b941b5 100644
--- a/Civi/Api4/Query/SqlFunctionGROUP_CONCAT.php
+++ b/Civi/Api4/Query/SqlFunctionGROUP_CONCAT.php
@@ -51,13 +51,19 @@ class SqlFunctionGROUP_CONCAT extends SqlFunction {
    *
    * @see \Civi\Api4\Utils\FormattingUtil::formatOutputValues
    * @param string $value
+   * @param string $dataType
    * @return string|array
    */
-  public function formatOutputValue($value) {
+  public function formatOutputValue($value, &$dataType) {
     $exprArgs = $this->getArgs();
+    // By default, values are split into an array and formatted according to the field's dataType
     if (!$exprArgs[2]['prefix']) {
       $value = explode(\CRM_Core_DAO::VALUE_SEPARATOR, $value);
     }
+    // If using custom separator, unset $dataType to preserve raw string
+    else {
+      $dataType = NULL;
+    }
     return $value;
   }
 
diff --git a/Civi/Api4/Utils/FormattingUtil.php b/Civi/Api4/Utils/FormattingUtil.php
index 888aafb4b7..340870e77f 100644
--- a/Civi/Api4/Utils/FormattingUtil.php
+++ b/Civi/Api4/Utils/FormattingUtil.php
@@ -201,10 +201,9 @@ class FormattingUtil {
         $fieldName = \CRM_Utils_Array::first($fieldExpr->getFields());
         $field = $fieldName && isset($fields[$fieldName]) ? $fields[$fieldName] : NULL;
         $dataType = $field['data_type'] ?? ($fieldName == 'id' ? 'Integer' : NULL);
-        // If Sql Function e.g. GROUP_CONCAT or COUNT wants to do its own formatting, apply and skip dataType conversion
+        // If Sql Function e.g. GROUP_CONCAT or COUNT wants to do its own formatting, apply
         if (method_exists($fieldExpr, 'formatOutputValue') && is_string($value)) {
-          $result[$key] = $value = $fieldExpr->formatOutputValue($value);
-          $dataType = NULL;
+          $result[$key] = $value = $fieldExpr->formatOutputValue($value, $dataType);
         }
         if (!$field) {
           continue;
diff --git a/tests/phpunit/api/v4/Action/SqlFunctionTest.php b/tests/phpunit/api/v4/Action/SqlFunctionTest.php
index a4ecd8d11f..585a796a1f 100644
--- a/tests/phpunit/api/v4/Action/SqlFunctionTest.php
+++ b/tests/phpunit/api/v4/Action/SqlFunctionTest.php
@@ -71,12 +71,16 @@ class SqlFunctionTest extends UnitTestCase {
       ->addGroupBy('contact_id')
       ->addWhere('contact_id', '=', $cid)
       ->addSelect('GROUP_CONCAT(financial_type_id:name)')
+      ->addSelect('GROUP_CONCAT(financial_type_id)')
       ->addSelect('COUNT(*) AS count')
       ->execute()
       ->first();
 
     $this->assertTrue(4 === $agg['count']);
     $this->assertContains('Donation', $agg['GROUP_CONCAT:financial_type_id:name']);
+    foreach ($agg['GROUP_CONCAT:financial_type_id'] as $type) {
+      $this->assertTrue(is_int($type));
+    }
 
     // Test GROUP_CONCAT with a CONCAT as well
     $agg = Contribution::get(FALSE)
-- 
2.25.1