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.

diff --git a/Civi/Api4/Generic/AbstractAction.php b/Civi/Api4/Generic/AbstractAction.php
index a82852bf2a53c0df00f02896afc70b5804e8eedf..8924c6c831cbdaab4f6192aa15311efd9cf38a00 100644 (file)
--- 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 0c5f5ebe8c47bdd42836e0e009c8d5751829129f..2c266c4028ac3895dc6e625c1df46a48a9650890 100644 (file)
--- 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 452a75bb1e16006af6b19b4a54ae547723c3be34..5a7e75d50f7b93173903b5960a874b92413c4219 100644 (file)
--- 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 beecbb5e3dbdafc158ae13081fa606e1d227d2c4..065ab8ea9d39bb5e7cd106d544f996fe92ced768 100644 (file)
--- 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 422ae5215631d4f7e65711627d6e6e04020d7d00..c8cafd824d5e0b05356efaee9aa4a45ed0b8f3ab 100644 (file)
--- 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']);
+  }
+
 }