From 9ed47542463496bb9d387f6866ad13b55de47027 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Fri, 24 Apr 2020 13:50:26 -0400 Subject: [PATCH] APIv4 - Allow field options to be returned in multiple formats --- CRM/Api4/Page/Api4Explorer.php | 1 + Civi/Api4/Generic/BasicGetFieldsAction.php | 63 +++++++++++++++++-- Civi/Api4/Service/Spec/FieldSpec.php | 10 ++- Civi/Api4/Utils/FormattingUtil.php | 11 ++-- ang/api4Explorer/Explorer.js | 6 ++ .../api/v4/Action/PseudoconstantTest.php | 26 +++++--- .../Service/TestCreationParameterProvider.php | 2 +- .../phpunit/api/v4/Spec/SpecGathererTest.php | 8 +-- 8 files changed, 102 insertions(+), 25 deletions(-) diff --git a/CRM/Api4/Page/Api4Explorer.php b/CRM/Api4/Page/Api4Explorer.php index 73545ea22d11..3fea8a2768f6 100644 --- a/CRM/Api4/Page/Api4Explorer.php +++ b/CRM/Api4/Page/Api4Explorer.php @@ -28,6 +28,7 @@ public function run() { 'docs' => \Civi\Api4\Utils\ReflectionUtils::parseDocBlock($apiDoc->getDocComment()), 'functions' => self::getSqlFunctions(), 'groupOptions' => array_column((array) $groupOptions, 'options', 'name'), + 'loadOptions' => [FALSE, TRUE, ['id', 'name', 'label']], ]; Civi::resources() ->addVars('api4', $vars) diff --git a/Civi/Api4/Generic/BasicGetFieldsAction.php b/Civi/Api4/Generic/BasicGetFieldsAction.php index 553d5bed2208..bfaa7e905de3 100644 --- a/Civi/Api4/Generic/BasicGetFieldsAction.php +++ b/Civi/Api4/Generic/BasicGetFieldsAction.php @@ -43,7 +43,15 @@ class BasicGetFieldsAction extends BasicGetAction { /** * Fetch option lists for fields? * - * @var bool + * This parameter can be either a boolean or an array of attributes to return from the option list: + * + * - If `FALSE`, each field's `options` property will be a boolean indicating whether the field has an option list + * - If `TRUE`, `options` will be returned as a flat array of the option list's `[id => label]` + * - If an array, `options` will be a non-associative array of requested properties, e.g. + * `loadOptions: ['id', 'name', 'label']` will return an array like `[[id: 1, name: 'Meeting', label: 'Meeting'], ...]` + * (note that names and labels are generally ONLY the same when the site's language is set to English). + * + * @var bool|array */ protected $loadOptions = FALSE; @@ -87,7 +95,7 @@ public function _run(Result $result) { else { $values = $this->getRecords(); } - $this->padResults($values); + $this->formatResults($values); $result->exchangeArray($this->queryArray($values)); } @@ -96,12 +104,14 @@ public function _run(Result $result) { * * Attempt to set some sensible defaults for some fields. * + * Format option lists. + * * In most cases it's not necessary to override this function, even if your entity is really weird. - * Instead just override $this->fields and thes function will respect that. + * Instead just override $this->fields and this function will respect that. * * @param array $values */ - protected function padResults(&$values) { + protected function formatResults(&$values) { $fields = array_column($this->fields(), 'name'); foreach ($values as &$field) { $defaults = array_intersect_key([ @@ -112,13 +122,54 @@ protected function padResults(&$values) { 'data_type' => \CRM_Utils_Array::value('type', $field, 'String'), ], array_flip($fields)); $field += $defaults; - if (!$this->loadOptions && isset($defaults['options'])) { - $field['options'] = (bool) $field['options']; + if (isset($defaults['options'])) { + $field['options'] = $this->formatOptionList($field['options']); } $field += array_fill_keys($fields, NULL); } } + /** + * Transforms option list into the format specified in $this->loadOptions + * + * @param $options + * @return array|bool + */ + private function formatOptionList($options) { + if (!$this->loadOptions || !is_array($options)) { + return (bool) $options; + } + if (!$options) { + return $options; + } + $formatted = []; + $first = reset($options); + // Flat array requested + if ($this->loadOptions === TRUE) { + // Convert non-associative to flat array + if (is_array($first) && isset($first['id'])) { + foreach ($options as $option) { + $formatted[$option['id']] = $option['label'] ?? $option['name'] ?? $option['id']; + } + return $formatted; + } + return $options; + } + // Non-associative array of multiple properties requested + foreach ($options as $id => $option) { + // Transform a flat list + if (!is_array($option)) { + $option = [ + 'id' => $id, + 'name' => $option, + 'label' => $option, + ]; + } + $formatted[] = array_intersect_key($option, array_flip($this->loadOptions)); + } + return $formatted; + } + /** * @return string */ diff --git a/Civi/Api4/Service/Spec/FieldSpec.php b/Civi/Api4/Service/Spec/FieldSpec.php index 31caa2abb79e..8ab47c991bf7 100644 --- a/Civi/Api4/Service/Spec/FieldSpec.php +++ b/Civi/Api4/Service/Spec/FieldSpec.php @@ -394,11 +394,19 @@ public function getOptions($values = []) { } $bao = CoreUtil::getBAOFromApiName($this->getEntity()); - $options = $bao::buildOptions($fieldName, NULL, $values); + $options = $bao::buildOptions($fieldName, 'get', $values); if (!is_array($options) || !$options) { $options = FALSE; } + else { + // FIXME: BuildOptions returns a single-dimensional list, but we want multiple properties of each option. + // FIXME: For now, call the buildOptions function twice and then combine the 2 arrays. Not an ideal approach. + // TODO: A better system of loading option lists so we can get more properties like 'color' and 'icon'. + $options = array_map(function($id, $label, $name) { + return ['id' => $id, 'label' => $label, 'name' => $name]; + }, array_keys($options), $options, $bao::buildOptions($fieldName, 'validate', $values)); + } $this->setOptions($options); } diff --git a/Civi/Api4/Utils/FormattingUtil.php b/Civi/Api4/Utils/FormattingUtil.php index 818f591417d4..e8f27bb5944c 100644 --- a/Civi/Api4/Utils/FormattingUtil.php +++ b/Civi/Api4/Utils/FormattingUtil.php @@ -181,15 +181,16 @@ public static function formatOutputValues(&$results, $fields, $entity, $action = * @param string $entity * Name of api entity * @param string $fieldName - * @param string $optionValue + * @param string $valueType + * name|label|abbr from self::$pseudoConstantContexts * @param array $params * Other values for this object * @param string $action * @return array * @throws \API_Exception */ - public static function getPseudoconstantList($entity, $fieldName, $optionValue, $params = [], $action = 'get') { - $context = self::$pseudoConstantContexts[$optionValue] ?? NULL; + public static function getPseudoconstantList($entity, $fieldName, $valueType, $params = [], $action = 'get') { + $context = self::$pseudoConstantContexts[$valueType] ?? NULL; if (!$context) { throw new \API_Exception('Illegal expression'); } @@ -198,8 +199,8 @@ public static function getPseudoconstantList($entity, $fieldName, $optionValue, if ($baoName) { $options = $baoName::buildOptions($fieldName, $context, $params); } - // Fallback for non-bao based entities - if (!isset($options)) { + // Fallback for option lists that exist in the api but not the BAO - note: $valueType gets ignored here + if (!isset($options) || $options === FALSE) { $options = civicrm_api4($entity, 'getFields', ['action' => $action, 'loadOptions' => TRUE, 'where' => [['name', '=', $fieldName]]])[0]['options'] ?? NULL; } if (is_array($options)) { diff --git a/ang/api4Explorer/Explorer.js b/ang/api4Explorer/Explorer.js index 369474fc0bf1..4d8c4aef4c6d 100644 --- a/ang/api4Explorer/Explorer.js +++ b/ang/api4Explorer/Explorer.js @@ -391,6 +391,12 @@ if (name === 'values') { defaultVal = defaultValues(defaultVal); } + if (name === 'loadOptions' && $scope.action === 'getFields') { + param.options = CRM.vars.api4.loadOptions; + format = 'json'; + defaultVal = false; + param.type = ['string']; + } $scope.$bindToRoute({ expr: 'params["' + name + '"]', param: name, diff --git a/tests/phpunit/api/v4/Action/PseudoconstantTest.php b/tests/phpunit/api/v4/Action/PseudoconstantTest.php index e6cab2de6788..1b7119c73b88 100644 --- a/tests/phpunit/api/v4/Action/PseudoconstantTest.php +++ b/tests/phpunit/api/v4/Action/PseudoconstantTest.php @@ -38,24 +38,33 @@ public function testOptionValue() { $subject = uniqid('subject'); OptionValue::create() ->addValue('option_group_id:name', 'activity_type') - ->addValue('label', 'Fake') + ->addValue('label', 'Fake Type') ->execute(); + $options = Activity::getFields() + ->addWhere('name', '=', 'activity_type_id') + ->setLoadOptions(['id', 'name', 'label']) + ->execute()->first()['options']; + $options = array_column($options, NULL, 'name'); + $this->assertEquals('Fake Type', $options['Fake_Type']['label']); + Activity::create() - ->addValue('activity_type_id:name', 'Fake') + ->addValue('activity_type_id:name', 'Fake_Type') ->addValue('source_contact_id', $cid) ->addValue('subject', $subject) ->execute(); $act = Activity::get() - ->addWhere('activity_type_id:name', '=', 'Fake') + ->addWhere('activity_type_id:label', '=', 'Fake Type') ->addWhere('subject', '=', $subject) + ->addSelect('activity_type_id:name') ->addSelect('activity_type_id:label') ->addSelect('activity_type_id') ->execute(); $this->assertCount(1, $act); - $this->assertEquals('Fake', $act[0]['activity_type_id:label']); + $this->assertEquals('Fake Type', $act[0]['activity_type_id:label']); + $this->assertEquals('Fake_Type', $act[0]['activity_type_id:name']); $this->assertTrue(is_numeric($act[0]['activity_type_id'])); } @@ -114,7 +123,7 @@ public function testCustomOptions() { ->addValue('extends', 'Individual') ->addChain('field', CustomField::create() ->addValue('custom_group_id', '$id') - ->addValue('option_values', ['r' => 'red', 'g' => 'green', 'b' => 'blue']) + ->addValue('option_values', ['r' => 'red', 'g' => 'green', 'b' => 'blü']) ->addValue('label', 'Color') ->addValue('html_type', 'Select') )->execute(); @@ -122,16 +131,17 @@ public function testCustomOptions() { $cid = Contact::create() ->setCheckPermissions(FALSE) ->addValue('first_name', 'col') - ->addValue('myPseudoconstantTest.Color:label', 'blue') + ->addValue('myPseudoconstantTest.Color:label', 'blü') ->execute()->first()['id']; $result = Contact::get() ->setCheckPermissions(FALSE) ->addWhere('id', '=', $cid) - ->addSelect('myPseudoconstantTest.Color:label', 'myPseudoconstantTest.Color') + ->addSelect('myPseudoconstantTest.Color:name', 'myPseudoconstantTest.Color:label', 'myPseudoconstantTest.Color') ->execute()->first(); - $this->assertEquals('blue', $result['myPseudoconstantTest.Color:label']); + $this->assertEquals('blü', $result['myPseudoconstantTest.Color:label']); + $this->assertEquals('bl_', $result['myPseudoconstantTest.Color:name']); $this->assertEquals('b', $result['myPseudoconstantTest.Color']); } diff --git a/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php b/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php index 68ccbb15d0bd..5ed6c7dd944f 100644 --- a/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php +++ b/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php @@ -102,7 +102,7 @@ private function getRequiredValue(FieldSpec $field) { * @return mixed */ private function getOption(FieldSpec $field) { - $options = $field->getOptions(); + $options = array_column($field->getOptions(), 'label', 'id'); return array_rand($options); } diff --git a/tests/phpunit/api/v4/Spec/SpecGathererTest.php b/tests/phpunit/api/v4/Spec/SpecGathererTest.php index dc8d1bf54965..6bcdc2ed84b9 100644 --- a/tests/phpunit/api/v4/Spec/SpecGathererTest.php +++ b/tests/phpunit/api/v4/Spec/SpecGathererTest.php @@ -101,12 +101,12 @@ public function testPseudoConstantOptionsWillBeAdded() { $regularField = $spec->getFieldByName('contact_type'); $this->assertNotEmpty($regularField->getOptions()); - $this->assertContains('Individual', $regularField->getOptions()); + $this->assertContains('Individual', array_column($regularField->getOptions(), 'name', 'id')); $customField = $spec->getFieldByName('FavoriteThings.FavColor'); - $this->assertNotEmpty($customField->getOptions()); - $this->assertContains('Green', $customField->getOptions()); - $this->assertEquals('Pink', $customField->getOptions()['p']); + $options = array_column($customField->getOptions(), NULL, 'id'); + $this->assertEquals('Green', $options['g']['name']); + $this->assertEquals('Pink', $options['p']['label']); } }