APIv4 - Improve pseudoconstant suffix support

Suffixes like :color now work for get actions (but not create because they are not unique idenfitiers).
Test added for rich option lists in getfields

diff --git a/Civi/Api4/Generic/BasicGetAction.php b/Civi/Api4/Generic/BasicGetAction.php
index 20cfe1821a4941f73848f15262dfad6ac3698481..f1d34d237f068426dcfee8fc8a86772861b19908 100644 (file)
--- a/Civi/Api4/Generic/BasicGetAction.php
+++ b/Civi/Api4/Generic/BasicGetAction.php
@@ -115,7 +115,7 @@ class BasicGetAction extends AbstractGetAction {
     foreach ($records as &$values) {
       foreach ($this->entityFields() as $field) {
         if (!empty($field['options'])) {
-          foreach (array_keys(FormattingUtil::$pseudoConstantContexts) as $suffix) {
+          foreach (FormattingUtil::$pseudoConstantSuffixes as $suffix) {
             $pseudofield = $field['name'] . ':' . $suffix;
             if (!isset($values[$pseudofield]) && isset($values[$field['name']]) && $this->_isFieldSelected($pseudofield)) {
               $values[$pseudofield] = $values[$field['name']];

diff --git a/Civi/Api4/Utils/FormattingUtil.php b/Civi/Api4/Utils/FormattingUtil.php
index b3cd5ecd0528b4d203dee505aae2fd2de4a42ab5..77e50db8f5370820fbe158cdf242eb29f9c42ad3 100644 (file)
--- a/Civi/Api4/Utils/FormattingUtil.php
+++ b/Civi/Api4/Utils/FormattingUtil.php
@@ -31,6 +31,8 @@ class FormattingUtil {
     'label' => 'get',
   ];
 
+  public static $pseudoConstantSuffixes = ['name', 'abbr', 'label', 'color', 'description', 'icon'];
+
   /**
    * Massage values into the format the BAO expects for a write operation
    *
@@ -46,7 +48,7 @@ class FormattingUtil {
         if ($value === 'null') {
           $value = 'Null';
         }
-        self::formatInputValue($value, $name, $field);
+        self::formatInputValue($value, $name, $field, 'create');
         // Ensure we have an array for serialized fields
         if (!empty($field['serialize'] && !is_array($value))) {
           $value = (array) $value;
@@ -80,19 +82,21 @@ class FormattingUtil {
    * @param $value
    * @param string $fieldName
    * @param array $fieldSpec
+   * @param string $action
    * @throws \API_Exception
+   * @throws \CRM_Core_Exception
    */
-  public static function formatInputValue(&$value, $fieldName, $fieldSpec) {
+  public static function formatInputValue(&$value, $fieldName, $fieldSpec, $action = 'get') {
     // Evaluate pseudoconstant suffix
     $suffix = strpos($fieldName, ':');
     if ($suffix) {
-      $options = self::getPseudoconstantList($fieldSpec['entity'], $fieldSpec['name'], substr($fieldName, $suffix + 1));
+      $options = self::getPseudoconstantList($fieldSpec['entity'], $fieldSpec['name'], substr($fieldName, $suffix + 1), $action);
       $value = self::replacePseudoconstant($options, $value, TRUE);
       return;
     }
     elseif (is_array($value)) {
       foreach ($value as &$val) {
-        self::formatInputValue($val, $fieldName, $fieldSpec);
+        self::formatInputValue($val, $fieldName, $fieldSpec, $action);
       }
       return;
     }
@@ -189,17 +193,20 @@ class FormattingUtil {
    */
   public static function getPseudoconstantList($entity, $fieldName, $valueType, $params = [], $action = 'get') {
     $context = self::$pseudoConstantContexts[$valueType] ?? NULL;
-    if (!$context) {
+    // For create actions, only unique identifiers can be used.
+    // For get actions any valid suffix is ok.
+    if (($action === 'create' && !$context) || !in_array($valueType, self::$pseudoConstantSuffixes, TRUE)) {
       throw new \API_Exception('Illegal expression');
     }
-    $baoName = CoreUtil::getBAOFromApiName($entity);
+    $baoName = $context ? CoreUtil::getBAOFromApiName($entity) : NULL;
     // Use BAO::buildOptions if possible
     if ($baoName) {
       $options = $baoName::buildOptions($fieldName, $context, $params);
     }
-    // Fallback for option lists that exist in the api but not the BAO - note: $valueType gets ignored here
+    // Fallback for option lists that exist in the api but not the BAO
     if (!isset($options) || $options === FALSE) {
-      $options = civicrm_api4($entity, 'getFields', ['action' => $action, 'loadOptions' => TRUE, 'where' => [['name', '=', $fieldName]]])[0]['options'] ?? NULL;
+      $options = civicrm_api4($entity, 'getFields', ['action' => $action, 'loadOptions' => ['id', $valueType], 'where' => [['name', '=', $fieldName]]])[0]['options'] ?? NULL;
+      $options = $options ? array_column($options, $valueType, 'id') : $options;
     }
     if (is_array($options)) {
       return $options;
@@ -278,7 +285,7 @@ class FormattingUtil {
           \Civi::$statics[__CLASS__][__FUNCTION__][$contactType][] = $field['name'];
           // Include suffixed variants like prefix_id:label
           if (!empty($field['pseudoconstant'])) {
-            foreach (array_keys(self::$pseudoConstantContexts) as $suffix) {
+            foreach (self::$pseudoConstantSuffixes as $suffix) {
               \Civi::$statics[__CLASS__][__FUNCTION__][$contactType][] = $field['name'] . ':' . $suffix;
             }
           }

diff --git a/tests/phpunit/api/v4/Action/BasicActionsTest.php b/tests/phpunit/api/v4/Action/BasicActionsTest.php
index 27ba63953ad514af65ab9ca2a821eaa4e1c34420..de06f6882e9c8b46c596b5ac4de52d88eff96e88 100644 (file)
--- a/tests/phpunit/api/v4/Action/BasicActionsTest.php
+++ b/tests/phpunit/api/v4/Action/BasicActionsTest.php
@@ -145,23 +145,43 @@ class BasicActionsTest extends UnitTestCase {
   public function testGetFields() {
     $getFields = MockBasicEntity::getFields()->execute()->indexBy('name');
 
-    $this->assertCount(6, $getFields);
+    $this->assertCount(7, $getFields);
     $this->assertEquals('Id', $getFields['id']['title']);
     // Ensure default data type is "String" when not specified
     $this->assertEquals('String', $getFields['color']['data_type']);
 
     // Getfields should default to loadOptions = false and reduce them to bool
     $this->assertTrue($getFields['group']['options']);
+    $this->assertTrue($getFields['fruit']['options']);
     $this->assertFalse($getFields['id']['options']);
 
-    // Now load options
+    // Load simple options
     $getFields = MockBasicEntity::getFields()
-      ->addWhere('name', '=', 'group')
+      ->addWhere('name', 'IN', ['group', 'fruit'])
       ->setLoadOptions(TRUE)
       ->execute()->indexBy('name');
 
-    $this->assertCount(1, $getFields);
+    $this->assertCount(2, $getFields);
     $this->assertArrayHasKey('one', $getFields['group']['options']);
+    // Complex options should be reduced to simple array
+    $this->assertArrayHasKey(1, $getFields['fruit']['options']);
+    $this->assertEquals('Banana', $getFields['fruit']['options'][3]);
+
+    // Load complex options
+    $getFields = MockBasicEntity::getFields()
+      ->addWhere('name', 'IN', ['group', 'fruit'])
+      ->setLoadOptions(['id', 'name', 'label', 'color'])
+      ->execute()->indexBy('name');
+
+    // Simple options should be expanded to non-assoc array
+    $this->assertCount(2, $getFields);
+    $this->assertEquals('one', $getFields['group']['options'][0]['id']);
+    $this->assertEquals('First', $getFields['group']['options'][0]['name']);
+    $this->assertEquals('First', $getFields['group']['options'][0]['label']);
+    $this->assertFalse(isset($getFields['group']['options'][0]['color']));
+    // Complex options should give all requested properties
+    $this->assertEquals('Banana', $getFields['fruit']['options'][2]['label']);
+    $this->assertEquals('yellow', $getFields['fruit']['options'][2]['color']);
   }
 
   public function testItemsToGet() {
@@ -235,4 +255,35 @@ class BasicActionsTest extends UnitTestCase {
     $this->assertEquals(['shape', 'size', 'weight'], array_keys($result));
   }
 
+  public function testPseudoconstantMatch() {
+    MockBasicEntity::delete()->addWhere('id', '>', 0)->execute();
+
+    $records = [
+      ['group:label' => 'First', 'shape' => 'round', 'fruit:name' => 'banana'],
+      ['group:name' => 'Second', 'shape' => 'square', 'fruit:label' => 'Pear'],
+    ];
+    MockBasicEntity::save()->setRecords($records)->execute();
+
+    $results = MockBasicEntity::get()
+      ->addSelect('*', 'group:label', 'group:name', 'fruit:name', 'fruit:color', 'fruit:label')
+      ->execute();
+
+    $this->assertEquals('round', $results[0]['shape']);
+    $this->assertEquals('one', $results[0]['group']);
+    $this->assertEquals('First', $results[0]['group:label']);
+    $this->assertEquals('First', $results[0]['group:name']);
+    $this->assertEquals(3, $results[0]['fruit']);
+    $this->assertEquals('Banana', $results[0]['fruit:label']);
+    $this->assertEquals('banana', $results[0]['fruit:name']);
+    $this->assertEquals('yellow', $results[0]['fruit:color']);
+
+    // Cannot match to a non-unique option property like :color on create
+    try {
+      MockBasicEntity::create()->addValue('fruit:color', 'yellow')->execute();
+    }
+    catch (\API_Exception $createError) {
+    }
+    $this->assertContains('Illegal expression', $createError->getMessage());
+  }
+
 }

diff --git a/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php b/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php
index f6fd57f9bf29446b5259820980afcda37562bc54..595a6d8d5d85d4fffcd0a1cb2c6299fc71100ac3 100644 (file)
--- a/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php
+++ b/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php
@@ -60,6 +60,29 @@ class MockBasicEntity extends Generic\AbstractEntity {
           'name' => 'weight',
           'data_type' => 'Integer',
         ],
+        [
+          'name' => 'fruit',
+          'options' => [
+            [
+              'id' => 1,
+              'name' => 'apple',
+              'label' => 'Apple',
+              'color' => 'red',
+            ],
+            [
+              'id' => 2,
+              'name' => 'pear',
+              'label' => 'Pear',
+              'color' => 'green',
+            ],
+            [
+              'id' => 3,
+              'name' => 'banana',
+              'label' => 'Banana',
+              'color' => 'yellow',
+            ],
+          ],
+        ],
       ];
     });
   }