From b04fc44bda46ea3332e74422a9f706f39677ed38 Mon Sep 17 00:00:00 2001 From: colemanw Date: Thu, 17 Aug 2023 17:07:26 -0400 Subject: [PATCH] APIv4 - Improve dataType handling within GROUP_CONCAT Supports sqlEquations and function suffixes within the aggregated array --- Civi/Api4/Query/SqlEquation.php | 8 +++---- Civi/Api4/Query/SqlFunction.php | 24 +++++++++---------- Civi/Api4/Query/SqlFunctionGROUP_CONCAT.php | 21 ++++++++-------- Civi/Api4/Utils/FormattingUtil.php | 5 ++-- .../phpunit/api/v4/Action/SqlFunctionTest.php | 12 ++++++---- 5 files changed, 36 insertions(+), 34 deletions(-) diff --git a/Civi/Api4/Query/SqlEquation.php b/Civi/Api4/Query/SqlEquation.php index a6dd5f564f..ac0fb8cf22 100644 --- 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 a766e95bbd..e2b8cb9118 100644 --- 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 e9f72907cc..dfb255a273 100644 --- 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 ca0a681260..252b71e841 100644 --- 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 9bcdb8c8bf..799371b0b5 100644 --- 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 { -- 2.25.1