APIv4 - Improve dataType handling within GROUP_CONCAT

Supports sqlEquations and function suffixes within the aggregated array

diff --git a/Civi/Api4/Query/SqlEquation.php b/Civi/Api4/Query/SqlEquation.php
index a6dd5f564f1806f3cf5353dea6252f7bbc6cc723..ac0fb8cf2271f6348f7350285edf11ff6f3f7fd4 100644 (file)
--- a/Civi/Api4/Query/SqlEquation.php
+++ b/Civi/Api4/Query/SqlEquation.php
@@ -133,11 +133,10 @@ class SqlEquation extends SqlExpression {
    * Change $dataType according to operator used in equation
    *
    * @see \Civi\Api4\Utils\FormattingUtil::formatOutputValues
-   * @param string $value
-   * @param string $dataType
-   * @return string
+   * @param string|null $dataType
+   * @param array $values
    */
-  public function formatOutputValue($value, &$dataType) {
+  public function formatOutputValue(?string &$dataType, array &$values) {
     foreach (self::$comparisonOperators as $op) {
       if (strpos($this->expr, " $op ")) {
         $dataType = 'Boolean';
@@ -148,7 +147,6 @@ class SqlEquation extends SqlExpression {
         $dataType = 'Float';
       }
     }
-    return $value;
   }
 
   public static function getTitle(): string {

diff --git a/Civi/Api4/Query/SqlFunction.php b/Civi/Api4/Query/SqlFunction.php
index a766e95bbdb2c50ccb301132cc87e3dab4a266b1..e2b8cb9118e0125c153086b3866905dab1c61f1b 100644 (file)
--- a/Civi/Api4/Query/SqlFunction.php
+++ b/Civi/Api4/Query/SqlFunction.php
@@ -109,34 +109,32 @@ abstract class SqlFunction extends SqlExpression {
   /**
    * Set $dataType and convert value by suffix
    *
+   * @param string|null $dataType
+   * @param array $values
+   * @param string $key
    * @see \Civi\Api4\Utils\FormattingUtil::formatOutputValues
-   * @param string $value
-   * @param string $dataType
-   * @return string
    */
-  public function formatOutputValue($value, &$dataType) {
+  public function formatOutputValue(?string &$dataType, array &$values, string $key): void {
     if (static::$dataType) {
       $dataType = static::$dataType;
     }
-    if (isset($value) && $this->suffix && $this->suffix !== 'id') {
+    if (isset($values[$key]) && $this->suffix && $this->suffix !== 'id') {
       $dataType = 'String';
+      $value =& $values[$key];
       $option = $this->getOptions()[$value] ?? NULL;
       // Option contains an array of suffix keys
       if (is_array($option)) {
-        return $option[$this->suffix] ?? NULL;
+        $value = $option[$this->suffix] ?? NULL;
       }
       // Flat arrays are name/value pairs
       elseif ($this->suffix === 'label') {
-        return $option;
-      }
-      elseif ($this->suffix === 'name') {
-        return $value;
+        $value = $option;
       }
-      else {
-        return NULL;
+      // Name needs no transformation, and any other suffix is invalid
+      elseif ($this->suffix !== 'name') {
+        $value = NULL;
       }
     }
-    return $value;
   }
 
   /**

diff --git a/Civi/Api4/Query/SqlFunctionGROUP_CONCAT.php b/Civi/Api4/Query/SqlFunctionGROUP_CONCAT.php
index e9f72907cc111a52650efaff7eb18e50feda87f5..dfb255a273d102d8715ff2ce9433c90c99ce85d1 100644 (file)
--- a/Civi/Api4/Query/SqlFunctionGROUP_CONCAT.php
+++ b/Civi/Api4/Query/SqlFunctionGROUP_CONCAT.php
@@ -25,7 +25,7 @@ class SqlFunctionGROUP_CONCAT extends SqlFunction {
       [
         'flag_before' => ['' => NULL, 'DISTINCT' => ts('Distinct')],
         'max_expr' => 1,
-        'must_be' => ['SqlField', 'SqlFunction'],
+        'must_be' => ['SqlField', 'SqlFunction', 'SqlEquation'],
         'optional' => FALSE,
       ],
       [
@@ -52,26 +52,27 @@ class SqlFunctionGROUP_CONCAT extends SqlFunction {
   /**
    * Reformat result as array if using default separator
    *
+   * @param string|null $dataType
+   * @param array $values
+   * @param string $key
    * @see \Civi\Api4\Utils\FormattingUtil::formatOutputValues
-   * @param string $value
-   * @param string $dataType
-   * @return string|array
    */
-  public function formatOutputValue($value, &$dataType) {
+  public function formatOutputValue(?string &$dataType, array &$values, string $key): void {
     $exprArgs = $this->getArgs();
     // By default, values are split into an array and formatted according to the field's dataType
     if (isset($exprArgs[2]['expr'][0]->expr) && $exprArgs[2]['expr'][0]->expr === \CRM_Core_DAO::VALUE_SEPARATOR) {
-      $value = explode(\CRM_Core_DAO::VALUE_SEPARATOR, $value);
-      // If the first expression is another sqlFunction, allow it to control the dataType
-      if ($exprArgs[0]['expr'][0] instanceof SqlFunction) {
-        $exprArgs[0]['expr'][0]->formatOutputValue(NULL, $dataType);
+      $values[$key] = explode(\CRM_Core_DAO::VALUE_SEPARATOR, $values[$key]);
+      // If the first expression is a SqlFunction/SqlEquation, allow it to control the dataType
+      if (method_exists($exprArgs[0]['expr'][0], 'formatOutputValue')) {
+        foreach (array_keys($values[$key]) as $index) {
+          $exprArgs[0]['expr'][0]->formatOutputValue($dataType, $values[$key], $index);
+        }
       }
     }
     // If using custom separator, preserve raw string
     else {
       $dataType = 'String';
     }
-    return $value;
   }
 
   /**

diff --git a/Civi/Api4/Utils/FormattingUtil.php b/Civi/Api4/Utils/FormattingUtil.php
index ca0a681260d4de90a0b429668414618e2cdfa336..252b71e841322bac4c60a74d6a35d94c74257842 100644 (file)
--- a/Civi/Api4/Utils/FormattingUtil.php
+++ b/Civi/Api4/Utils/FormattingUtil.php
@@ -237,9 +237,10 @@ class FormattingUtil {
       $baseName = $fieldName ? \CRM_Utils_Array::first(explode(':', $fieldName)) : NULL;
       $field = $fields[$fieldName] ?? $fields[$baseName] ?? NULL;
       $dataType = $field['data_type'] ?? ($fieldName == 'id' ? 'Integer' : NULL);
-      // Allow Sql Functions to do special formatting and/or alter the $dataType
+      // Allow Sql Functions to do alter the value and/or $dataType
       if (method_exists($fieldExpr, 'formatOutputValue') && is_string($value)) {
-        $result[$key] = $value = $fieldExpr->formatOutputValue($value, $dataType);
+        $fieldExpr->formatOutputValue($dataType, $result, $key);
+        $value = $result[$key];
       }
       if (!empty($field['output_formatters'])) {
         self::applyFormatters($result, $fieldName, $field, $value);

diff --git a/tests/phpunit/api/v4/Action/SqlFunctionTest.php b/tests/phpunit/api/v4/Action/SqlFunctionTest.php
index 9bcdb8c8bffb6e6747fb7adc121e1b2fcb623e9f..799371b0b525658c6a50018470dead124787c98a 100644 (file)
--- a/tests/phpunit/api/v4/Action/SqlFunctionTest.php
+++ b/tests/phpunit/api/v4/Action/SqlFunctionTest.php
@@ -49,9 +49,9 @@ class SqlFunctionTest extends Api4TestBase implements TransactionalInterface {
       ->setDefaults(['contact_id' => $cid, 'financial_type_id:name' => 'Donation'])
       ->setRecords([
         ['total_amount' => 100, 'receive_date' => '2020-01-01'],
-        ['total_amount' => 200, 'receive_date' => '2020-01-01'],
-        ['total_amount' => 300, 'receive_date' => '2020-01-01', 'financial_type_id:name' => 'Member Dues'],
-        ['total_amount' => 400, 'receive_date' => '2020-01-01', 'financial_type_id:name' => 'Event Fee'],
+        ['total_amount' => 200, 'receive_date' => '2020-02-01'],
+        ['total_amount' => 300, 'receive_date' => '2020-03-01', 'financial_type_id:name' => 'Member Dues'],
+        ['total_amount' => 400, 'receive_date' => '2020-04-01', 'financial_type_id:name' => 'Event Fee'],
       ])
       ->execute();
 
@@ -89,17 +89,21 @@ class SqlFunctionTest extends Api4TestBase implements TransactionalInterface {
       $this->assertTrue(is_int($type));
     }
 
-    // Test GROUP_CONCAT with a CONCAT as well
+    // Test GROUP_CONCAT with functions
     $agg = Contribution::get(FALSE)
       ->addGroupBy('contact_id')
       ->addWhere('contact_id', '=', $cid)
       ->addSelect("GROUP_CONCAT(CONCAT(financial_type_id, ', ', contact_id, ', ', total_amount))")
+      ->addSelect("GROUP_CONCAT((financial_type_id = 1)) AS is_donation")
+      ->addSelect("GROUP_CONCAT(MONTH(receive_date):label) AS months")
       ->addSelect('COUNT(*) AS count')
       ->execute()
       ->first();
 
     $this->assertTrue(4 === $agg['count']);
     $this->assertContains('1, ' . $cid . ', 100.00', $agg['GROUP_CONCAT:financial_type_id_contact_id_total_amount']);
+    $this->assertEquals([TRUE, TRUE, FALSE, FALSE], $agg['is_donation']);
+    $this->assertEquals(['January', 'February', 'March', 'April'], $agg['months']);
   }
 
   public function testGroupHaving(): void {