From 265192b2c2d5e77a4125af478c9c753e62fc7257 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Wed, 7 Jul 2021 15:54:57 -0400 Subject: [PATCH] APIv4 - Fix pseudoconstant matching function to pass params correctly across joins The getPseudoconstantList() function passes an array of params to core BAO::buildOptions, that list needs to be transformed to match the prefix on the field, in case that field belongs to a joined entity rather than the main entity. --- Civi/Api4/Generic/AbstractAction.php | 3 +- Civi/Api4/Generic/Traits/DAOActionTrait.php | 6 +- Civi/Api4/Query/Api4SelectQuery.php | 2 +- Civi/Api4/Utils/FormattingUtil.php | 74 +++++++++++++++---- .../phpunit/api/v4/Action/ContactGetTest.php | 65 ++++++++++++++++ 5 files changed, 131 insertions(+), 19 deletions(-) diff --git a/Civi/Api4/Generic/AbstractAction.php b/Civi/Api4/Generic/AbstractAction.php index a82852bf2a..8924c6c831 100644 --- a/Civi/Api4/Generic/AbstractAction.php +++ b/Civi/Api4/Generic/AbstractAction.php @@ -494,6 +494,7 @@ abstract class AbstractAction implements \ArrayAccess { if ($field) { $optionFields[$fieldName] = [ 'val' => $record[$expr], + 'expr' => $expr, 'field' => $field, 'suffix' => substr($expr, $suffix + 1), 'depends' => $field['input_attrs']['control_field'] ?? NULL, @@ -508,7 +509,7 @@ abstract class AbstractAction implements \ArrayAccess { }); // Replace pseudoconstants. Note this is a reverse lookup as we are evaluating input not output. foreach ($optionFields as $fieldName => $info) { - $options = FormattingUtil::getPseudoconstantList($info['field'], $info['suffix'], $record, 'create'); + $options = FormattingUtil::getPseudoconstantList($info['field'], $info['expr'], $record, 'create'); $record[$fieldName] = FormattingUtil::replacePseudoconstant($options, $info['val'], TRUE); } } diff --git a/Civi/Api4/Generic/Traits/DAOActionTrait.php b/Civi/Api4/Generic/Traits/DAOActionTrait.php index 0c5f5ebe8c..2c266c4028 100644 --- a/Civi/Api4/Generic/Traits/DAOActionTrait.php +++ b/Civi/Api4/Generic/Traits/DAOActionTrait.php @@ -215,7 +215,7 @@ trait DAOActionTrait { if (NULL !== $value) { if ($field['suffix']) { - $options = FormattingUtil::getPseudoconstantList($field, $field['suffix'], $params, $this->getActionName()); + $options = FormattingUtil::getPseudoconstantList($field, $name, $params, $this->getActionName()); $value = FormattingUtil::replacePseudoconstant($options, $value, TRUE); } @@ -266,8 +266,8 @@ trait DAOActionTrait { if (strpos($fieldExpr, '.') === FALSE) { return NULL; } - list($groupName, $fieldName) = explode('.', $fieldExpr); - list($fieldName, $suffix) = array_pad(explode(':', $fieldName), 2, NULL); + [$groupName, $fieldName] = explode('.', $fieldExpr); + [$fieldName, $suffix] = array_pad(explode(':', $fieldName), 2, NULL); $cacheKey = "APIv4_Custom_Fields-$groupName"; $info = \Civi::cache('metadata')->get($cacheKey); if (!isset($info[$fieldName])) { diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index 452a75bb1e..5a7e75d50f 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -319,7 +319,7 @@ class Api4SelectQuery { $suffix = strstr($item, ':'); if ($suffix && $expr->getType() === 'SqlField') { $field = $this->getField($item); - $options = FormattingUtil::getPseudoconstantList($field, substr($suffix, 1)); + $options = FormattingUtil::getPseudoconstantList($field, $item); if ($options) { asort($options); $column = "FIELD($column,'" . implode("','", array_keys($options)) . "')"; diff --git a/Civi/Api4/Utils/FormattingUtil.php b/Civi/Api4/Utils/FormattingUtil.php index beecbb5e3d..065ab8ea9d 100644 --- a/Civi/Api4/Utils/FormattingUtil.php +++ b/Civi/Api4/Utils/FormattingUtil.php @@ -90,7 +90,7 @@ class FormattingUtil { // Evaluate pseudoconstant suffix $suffix = strpos($fieldName, ':'); if ($suffix) { - $options = self::getPseudoconstantList($fieldSpec, substr($fieldName, $suffix + 1), [], $operator ? 'get' : 'create'); + $options = self::getPseudoconstantList($fieldSpec, $fieldName, [], $operator ? 'get' : 'create'); $value = self::replacePseudoconstant($options, $value, TRUE); return; } @@ -214,7 +214,7 @@ class FormattingUtil { // Evaluate pseudoconstant suffixes $suffix = strrpos($fieldName, ':'); if ($suffix) { - $fieldOptions[$fieldName] = $fieldOptions[$fieldName] ?? self::getPseudoconstantList($field, substr($fieldName, $suffix + 1), $result, $action); + $fieldOptions[$fieldName] = $fieldOptions[$fieldName] ?? self::getPseudoconstantList($field, $fieldName, $result, $action); $dataType = NULL; } if ($fieldExpr->supportsExpansion) { @@ -243,15 +243,16 @@ class FormattingUtil { * Retrieves pseudoconstant option list for a field. * * @param array $field - * @param string $valueType - * name|label|abbr from self::$pseudoConstantContexts + * @param string $fieldAlias + * Field path plus pseudoconstant suffix, e.g. 'contact.employer_id.contact_sub_type:label' * @param array $params * Other values for this object * @param string $action * @return array * @throws \API_Exception */ - public static function getPseudoconstantList($field, $valueType, $params = [], $action = 'get') { + public static function getPseudoconstantList(array $field, string $fieldAlias, $params = [], $action = 'get') { + [$fieldPath, $valueType] = explode(':', $fieldAlias); $context = self::$pseudoConstantContexts[$valueType] ?? NULL; // For create actions, only unique identifiers can be used. // For get actions any valid suffix is ok. @@ -262,7 +263,7 @@ class FormattingUtil { // Use BAO::buildOptions if possible if ($baoName) { $fieldName = empty($field['custom_field_id']) ? $field['name'] : 'custom_' . $field['custom_field_id']; - $options = $baoName::buildOptions($fieldName, $context, $params); + $options = $baoName::buildOptions($fieldName, $context, self::filterByPrefix($params, $fieldPath, $field['name'])); } // Fallback for option lists that exist in the api but not the BAO if (!isset($options) || $options === FALSE) { @@ -300,14 +301,17 @@ class FormattingUtil { return is_array($value) ? $matches : $matches[0] ?? NULL; } - private static function applyFormatters($result, $fieldName, $field, &$value) { - $row = []; - $prefix = substr($fieldName, 0, strpos($fieldName, $field['name'])); - foreach ($result as $key => $val) { - if (!$prefix || strpos($key, $prefix) === 0) { - $row[substr($key, strlen($prefix))] = $val; - } - } + /** + * Apply a field's output_formatters callback functions + * + * @param array $result + * @param string $fieldPath + * @param array $field + * @param mixed $value + */ + private static function applyFormatters(array $result, string $fieldPath, array $field, &$value) { + $row = self::filterByPrefix($result, $fieldPath, $field['name']); + foreach ($field['output_formatters'] as $formatter) { $formatter($value, $row, $field); } @@ -372,4 +376,46 @@ class FormattingUtil { }, \Civi::$statics[__CLASS__][__FUNCTION__][$contactType]); } + /** + * Given a field belonging to either the main entity or a joined entity, + * and a values array of [path => value], this returns all values which share the same root path. + * + * Works by filtering array keys to only include those with the same prefix as a given field, + * stripping them of that prefix. + * + * Ex: + * ``` + * $values = [ + * 'first_name' => 'a', + * 'middle_name' => 'b', + * 'related_contact.first_name' => 'c', + * 'related_contact.last_name' => 'd', + * 'activity.subject' => 'e', + * ] + * $fieldPath = 'related_contact.id' + * $fieldName = 'id' + * + * filterByPrefix($values, $fieldPath, $fieldName) + * returns [ + * 'first_name' => 'c', + * 'last_name' => 'd', + * ] + * ``` + * + * @param array $values + * @param string $fieldPath + * @param string $fieldName + * @return array + */ + public static function filterByPrefix(array $values, string $fieldPath, string $fieldName): array { + $filtered = []; + $prefix = substr($fieldPath, 0, strpos($fieldPath, $fieldName)); + foreach ($values as $key => $val) { + if (!$prefix || strpos($key, $prefix) === 0) { + $filtered[substr($key, strlen($prefix))] = $val; + } + } + return $filtered; + } + } diff --git a/tests/phpunit/api/v4/Action/ContactGetTest.php b/tests/phpunit/api/v4/Action/ContactGetTest.php index 422ae52156..c8cafd824d 100644 --- a/tests/phpunit/api/v4/Action/ContactGetTest.php +++ b/tests/phpunit/api/v4/Action/ContactGetTest.php @@ -20,6 +20,7 @@ namespace api\v4\Action; use Civi\Api4\Contact; +use Civi\Api4\Relationship; /** * @group headless @@ -194,4 +195,68 @@ class ContactGetTest extends \api\v4\UnitTestCase { $this->assertArrayHasKey($jan['id'], (array) $result); } + public function testGetRelatedWithSubType() { + $org = Contact::create(FALSE) + ->addValue('contact_type', 'Organization') + ->addValue('organization_name', 'Run Amok') + ->execute()->single()['id']; + + $ind = Contact::create(FALSE) + ->addValue('first_name', 'Guy') + ->addValue('last_name', 'Amok') + ->addValue('contact_sub_type', ['Student']) + ->addChain('relationship', Relationship::create() + ->addValue('contact_id_a', '$id') + ->addValue('contact_id_b', $org) + ->addValue("relationship_type_id:name", "Employee of") + ) + ->execute()->single()['id']; + + // We can retrieve contact sub-type directly + $result = Contact::get() + ->addSelect('contact_sub_type:label') + ->addWhere('id', '=', $ind) + ->execute()->single(); + $this->assertEquals(['Student'], $result['contact_sub_type:label']); + + // Ensure we can also retrieve it indirectly via join + $params = [ + 'select' => [ + 'id', + 'display_name', + 'contact_type', + 'Contact_RelationshipCache_Contact_01.id', + 'Contact_RelationshipCache_Contact_01.far_relation:label', + 'Contact_RelationshipCache_Contact_01.display_name', + 'Contact_RelationshipCache_Contact_01.contact_sub_type:label', + 'Contact_RelationshipCache_Contact_01.contact_type', + ], + 'where' => [ + ['contact_type:name', '=', 'Organization'], + ['Contact_RelationshipCache_Contact_01.contact_sub_type:name', 'CONTAINS', 'Student'], + ['id', '=', $org], + ], + 'join' => [ + [ + 'Contact AS Contact_RelationshipCache_Contact_01', + 'INNER', + 'RelationshipCache', + ['id', '=', 'Contact_RelationshipCache_Contact_01.far_contact_id'], + ['Contact_RelationshipCache_Contact_01.near_relation:name', 'IN', ['Employee of']], + ], + ], + 'checkPermissions' => TRUE, + 'limit' => 50, + 'offset' => 0, + 'debug' => TRUE, + ]; + + $results = civicrm_api4('Contact', 'get', $params); + $result = $results->single(); + $this->assertEquals('Run Amok', $result['display_name']); + $this->assertEquals('Guy Amok', $result['Contact_RelationshipCache_Contact_01.display_name']); + $this->assertEquals('Employer of', $result['Contact_RelationshipCache_Contact_01.far_relation:label']); + $this->assertEquals(['Student'], $result['Contact_RelationshipCache_Contact_01.contact_sub_type:label']); + } + } -- 2.25.1