From 1ccb570e60f11c36e520b43b8a904630376e593c Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Fri, 23 Sep 2022 12:01:01 -0400 Subject: [PATCH] APIv4 - Fix Get operation to load options that depend on other options Fixes dev/core#3866 --- Civi/Api4/Query/Api4SelectQuery.php | 52 ++++++++++++++++--- Civi/Api4/Utils/FormattingUtil.php | 7 +-- .../phpunit/api/v4/Custom/CustomGroupTest.php | 23 ++++++++ 3 files changed, 73 insertions(+), 9 deletions(-) diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index 70dda45a15..284afe6bbd 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -96,6 +96,11 @@ class Api4SelectQuery { */ private $entityAccess = []; + /** + * @var array + */ + private $entityValues = []; + /** * @param \Civi\Api4\Generic\DAOGetAction $apiGet */ @@ -115,6 +120,8 @@ class Api4SelectQuery { $tableName = CoreUtil::getTableName($this->getEntity()); $this->query = \CRM_Utils_SQL_Select::from($tableName . ' ' . self::MAIN_TABLE_ALIAS); + $this->fillEntityValues(); + $this->entityAccess[$this->getEntity()] = TRUE; // Add ACLs first to avoid redundant subclauses @@ -379,6 +386,39 @@ class Api4SelectQuery { } } + /** + * This takes all the where clauses that use `=` to build an array of known values which every record must have. + * + * This gets passed to `FormattingUtil::getPseudoconstantList` to evaluate conditional pseudoconstants. + * + * @throws \CRM_Core_Exception + */ + private function fillEntityValues() { + foreach ($this->getWhere() as $clause) { + [$fieldName, $operator, $value] = array_pad($clause, 3, NULL); + if ( + // If the operator is `=` + $operator === '=' && + // And is using a literal value + empty($clause[3]) && + // And references a field not a function + !strpos($fieldName, ')') + ) { + $field = $this->getField($fieldName); + if ($field) { + // Resolve pseudoconstant suffix + FormattingUtil::formatInputValue($value, $fieldName, $field, $this->entityValues, $operator); + // If the operator is still `=` (so not a weird date range transformation) + if ($operator === '=') { + // Strip pseudoconstant suffix + [$fieldNameOnly] = explode(':', $fieldName); + $this->entityValues[$fieldNameOnly] = $value; + } + } + } + } + } + /** * Recursively validate and transform a branch or leaf clause array to SQL. * @@ -455,14 +495,14 @@ class Api4SelectQuery { if ($expr->getType() === 'SqlField') { $fieldName = count($expr->getFields()) === 1 ? $expr->getFields()[0] : NULL; $field = $this->getField($fieldName, TRUE); - FormattingUtil::formatInputValue($value, $fieldName, $field, $operator); + FormattingUtil::formatInputValue($value, $fieldName, $field, $this->entityValues, $operator); } elseif ($expr->getType() === 'SqlFunction') { $fauxField = [ 'name' => NULL, 'data_type' => $expr::getDataType(), ]; - FormattingUtil::formatInputValue($value, NULL, $fauxField, $operator); + FormattingUtil::formatInputValue($value, NULL, $fauxField, $this->entityValues, $operator); } $fieldAlias = $expr->render($this); } @@ -474,7 +514,7 @@ class Api4SelectQuery { // Attempt to format if this is a real field if (isset($this->apiFieldSpec[$expr])) { $field = $this->getField($expr); - FormattingUtil::formatInputValue($value, $expr, $field, $operator); + FormattingUtil::formatInputValue($value, $expr, $field, $this->entityValues, $operator); } } // Expr references a non-field expression like a function; convert to alias @@ -488,7 +528,7 @@ class Api4SelectQuery { [$selectField] = explode(':', $selectAlias); if ($selectAlias === $selectExpr && $fieldName === $selectField && isset($this->apiFieldSpec[$fieldName])) { $field = $this->getField($fieldName); - FormattingUtil::formatInputValue($value, $expr, $field, $operator); + FormattingUtil::formatInputValue($value, $expr, $field, $this->entityValues, $operator); $fieldAlias = $selectAlias; break; } @@ -512,7 +552,7 @@ class Api4SelectQuery { $valExpr = $this->getExpression($value); if ($expr->getType() === 'SqlField' && $valExpr->getType() === 'SqlString') { $value = $valExpr->getExpr(); - FormattingUtil::formatInputValue($value, $fieldName, $this->apiFieldSpec[$fieldName], $operator); + FormattingUtil::formatInputValue($value, $fieldName, $this->apiFieldSpec[$fieldName], $this->entityValues, $operator); return $this->createSQLClause($fieldAlias, $operator, $value, $this->apiFieldSpec[$fieldName], $depth); } else { @@ -522,7 +562,7 @@ class Api4SelectQuery { } elseif ($expr->getType() === 'SqlField') { $field = $this->getField($fieldName); - FormattingUtil::formatInputValue($value, $fieldName, $field, $operator); + FormattingUtil::formatInputValue($value, $fieldName, $field, $this->entityValues, $operator); } } diff --git a/Civi/Api4/Utils/FormattingUtil.php b/Civi/Api4/Utils/FormattingUtil.php index 2fe49d3453..c47468f0a3 100644 --- a/Civi/Api4/Utils/FormattingUtil.php +++ b/Civi/Api4/Utils/FormattingUtil.php @@ -82,22 +82,23 @@ class FormattingUtil { * @param $value * @param string|null $fieldName * @param array $fieldSpec + * @param array $params * @param string|null $operator (only for 'get' actions) * @param null $index (for recursive loops) * @throws \CRM_Core_Exception */ - public static function formatInputValue(&$value, ?string $fieldName, array $fieldSpec, &$operator = NULL, $index = NULL) { + public static function formatInputValue(&$value, ?string $fieldName, array $fieldSpec, array $params = [], &$operator = NULL, $index = NULL) { // Evaluate pseudoconstant suffix $suffix = strpos(($fieldName ?? ''), ':'); if ($suffix) { - $options = self::getPseudoconstantList($fieldSpec, $fieldName, [], $operator ? 'get' : 'create'); + $options = self::getPseudoconstantList($fieldSpec, $fieldName, $params, $operator ? 'get' : 'create'); $value = self::replacePseudoconstant($options, $value, TRUE); return; } elseif (is_array($value)) { $i = 0; foreach ($value as &$val) { - self::formatInputValue($val, $fieldName, $fieldSpec, $operator, $i++); + self::formatInputValue($val, $fieldName, $fieldSpec, $params, $operator, $i++); } return; } diff --git a/tests/phpunit/api/v4/Custom/CustomGroupTest.php b/tests/phpunit/api/v4/Custom/CustomGroupTest.php index 7e85a43c2c..a5cf4a77f9 100644 --- a/tests/phpunit/api/v4/Custom/CustomGroupTest.php +++ b/tests/phpunit/api/v4/Custom/CustomGroupTest.php @@ -51,4 +51,27 @@ class CustomGroupTest extends CustomTestBase { $this->assertEquals('Contribution', $groups[1]['extends']); } + public function testGetExtendsEntityColumnValuePseudoconstant() { + $activityTypeName = uniqid(); + $activityType = $this->createTestRecord('OptionValue', [ + 'option_group_id:name' => 'activity_type', + 'name' => $activityTypeName, + ]); + $customGroup1 = $this->createTestRecord('CustomGroup', [ + 'extends' => 'Activity', + 'extends_entity_column_value:name' => [$activityTypeName], + ]); + $customGroup2 = $this->createTestRecord('CustomGroup', [ + 'extends' => 'Activity', + ]); + $this->assertEquals([$activityType['value']], $customGroup1['extends_entity_column_value']); + + $result = CustomGroup::get(FALSE) + ->addWhere('extends_entity_column_value:name', 'CONTAINS', $activityTypeName) + ->addWhere('extends:name', '=', 'Activities') + ->execute()->single(); + + $this->assertEquals([$activityType['value']], $result['extends_entity_column_value']); + } + } -- 2.25.1