From 3ffbd21c28afcf8f2adb54e25dc6550b9dac19f8 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Wed, 20 May 2020 14:55:20 -0400 Subject: [PATCH] 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 --- Civi/Api4/Generic/BasicGetAction.php | 2 +- Civi/Api4/Utils/FormattingUtil.php | 25 +++++--- .../api/v4/Action/BasicActionsTest.php | 59 +++++++++++++++++-- .../api/v4/Mock/Api4/MockBasicEntity.php | 23 ++++++++ 4 files changed, 95 insertions(+), 14 deletions(-) diff --git a/Civi/Api4/Generic/BasicGetAction.php b/Civi/Api4/Generic/BasicGetAction.php index 20cfe1821a49..f1d34d237f06 100644 --- a/Civi/Api4/Generic/BasicGetAction.php +++ b/Civi/Api4/Generic/BasicGetAction.php @@ -115,7 +115,7 @@ protected function formatRawValues(&$records) { 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 b3cd5ecd0528..77e50db8f537 100644 --- 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 @@ public static function formatWriteParams(&$params, $fields) { 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 @@ public static function formatWriteParams(&$params, $fields) { * @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 @@ public static function formatOutputValues(&$results, $fields, $entity, $action = */ 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 @@ public static function contactFieldsToRemove($contactType, $prefix) { \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 27ba63953ad5..de06f6882e9c 100644 --- a/tests/phpunit/api/v4/Action/BasicActionsTest.php +++ b/tests/phpunit/api/v4/Action/BasicActionsTest.php @@ -145,23 +145,43 @@ public function testBatchFrobnicate() { 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 @@ public function testWildcardSelect() { $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 f6fd57f9bf29..595a6d8d5d85 100644 --- a/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php +++ b/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php @@ -60,6 +60,29 @@ public static function getFields() { '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', + ], + ], + ], ]; }); }