From bb6bfd6881dabf36c66b6e20cde08c635bfaecbe 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/Core/BAO/CustomField.php | 15 ++-- .../CustomFieldPreSaveSubscriber.php | 22 +++-- Civi/Api4/Generic/BasicGetFieldsAction.php | 68 ++++++++++++-- Civi/Api4/Service/Spec/FieldSpec.php | 90 +++++++++++++++++-- Civi/Api4/Service/Spec/SpecFormatter.php | 2 +- Civi/Api4/Utils/FormattingUtil.php | 11 +-- ang/api4Explorer/Explorer.js | 11 +++ .../api/v4/Action/PseudoconstantTest.php | 70 +++++++++++++-- .../Service/TestCreationParameterProvider.php | 2 +- .../phpunit/api/v4/Spec/SpecGathererTest.php | 8 +- 10 files changed, 250 insertions(+), 49 deletions(-) diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php index 4293acf293..128c2a8841 100644 --- a/CRM/Core/BAO/CustomField.php +++ b/CRM/Core/BAO/CustomField.php @@ -1810,15 +1810,15 @@ WHERE id IN ( %1, %2 ) * @param array $params * @param string $value * @param \CRM_Core_DAO_OptionGroup $optionGroup - * @param string $optionName + * @param string $index * @param string $dataType */ - protected static function createOptionValue(&$params, $value, CRM_Core_DAO_OptionGroup $optionGroup, $optionName, $dataType) { + protected static function createOptionValue(&$params, $value, CRM_Core_DAO_OptionGroup $optionGroup, $index, $dataType) { if (strlen(trim($value))) { $optionValue = new CRM_Core_DAO_OptionValue(); $optionValue->option_group_id = $optionGroup->id; - $optionValue->label = $params['option_label'][$optionName]; - $optionValue->name = CRM_Utils_String::titleToVar($params['option_label'][$optionName]); + $optionValue->label = $params['option_label'][$index]; + $optionValue->name = $params['option_name'][$index] ?? CRM_Utils_String::titleToVar($params['option_label'][$index]); switch ($dataType) { case 'Money': $optionValue->value = CRM_Utils_Rule::cleanMoney($value); @@ -1836,8 +1836,11 @@ WHERE id IN ( %1, %2 ) $optionValue->value = trim($value); } - $optionValue->weight = $params['option_weight'][$optionName]; - $optionValue->is_active = CRM_Utils_Array::value($optionName, $params['option_status'], FALSE); + $optionValue->weight = $params['option_weight'][$index]; + $optionValue->is_active = $params['option_status'][$index] ?? FALSE; + $optionValue->description = $params['option_description'][$index] ?? NULL; + $optionValue->color = $params['option_color'][$index] ?? NULL; + $optionValue->icon = $params['option_icon'][$index] ?? NULL; $optionValue->save(); } } diff --git a/Civi/Api4/Event/Subscriber/CustomFieldPreSaveSubscriber.php b/Civi/Api4/Event/Subscriber/CustomFieldPreSaveSubscriber.php index 9a4f2c4e84..d2344803d1 100644 --- a/Civi/Api4/Event/Subscriber/CustomFieldPreSaveSubscriber.php +++ b/Civi/Api4/Event/Subscriber/CustomFieldPreSaveSubscriber.php @@ -29,22 +29,26 @@ class CustomFieldPreSaveSubscriber extends Generic\PreSaveSubscriber { public function modify(&$field, AbstractAction $request) { if (!empty($field['option_values'])) { - $weight = 0; + $weight = $key = 0; + $field['option_label'] = $field['option_value'] = $field['option_status'] = $field['option_weight'] = []; + $field['option_name'] = $field['option_color'] = $field['option_description'] = $field['option_icon'] = []; foreach ($field['option_values'] as $key => $value) { // Translate simple key/value pairs into full-blown option values if (!is_array($value)) { $value = [ 'label' => $value, - 'value' => $key, - 'is_active' => 1, - 'weight' => $weight, + 'id' => $key, ]; - $key = $weight++; } - $field['option_label'][$key] = $value['label']; - $field['option_value'][$key] = $value['value']; - $field['option_status'][$key] = $value['is_active']; - $field['option_weight'][$key] = $value['weight']; + $weight++; + $field['option_label'][] = $value['label'] ?? $value['name']; + $field['option_name'][] = $value['name'] ?? NULL; + $field['option_value'][] = $value['id']; + $field['option_status'][] = $value['is_active'] ?? 1; + $field['option_weight'][] = $value['weight'] ?? $weight; + $field['option_color'][] = $value['color'] ?? NULL; + $field['option_description'][] = $value['description'] ?? NULL; + $field['option_icon'][] = $value['icon'] ?? NULL; } } $field['option_type'] = !empty($field['option_values']); diff --git a/Civi/Api4/Generic/BasicGetFieldsAction.php b/Civi/Api4/Generic/BasicGetFieldsAction.php index 9c2c577a15..a277bf5901 100644 --- a/Civi/Api4/Generic/BasicGetFieldsAction.php +++ b/Civi/Api4/Generic/BasicGetFieldsAction.php @@ -32,8 +32,8 @@ use Civi\API\Exception\NotImplementedException; * By default this will fetch the field list relevant to `get`, * but a different list may be returned if you specify another action. * - * @method $this setLoadOptions(bool $value) - * @method bool getLoadOptions() + * @method $this setLoadOptions(bool|array $value) + * @method bool|array getLoadOptions() * @method $this setAction(string $value) * @method $this setValues(array $values) * @method array getValues() @@ -43,7 +43,16 @@ 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: + * id, name, label, abbr, description, color, icon + * 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 +96,7 @@ class BasicGetFieldsAction extends BasicGetAction { else { $values = $this->getRecords(); } - $this->padResults($values); + $this->formatResults($values); $result->exchangeArray($this->queryArray($values)); } @@ -96,12 +105,14 @@ class BasicGetFieldsAction extends BasicGetAction { * * 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'); // Enforce field permissions if ($this->checkPermissions) { @@ -120,13 +131,54 @@ class BasicGetFieldsAction extends BasicGetAction { '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 31caa2abb7..fc8852dd10 100644 --- a/Civi/Api4/Service/Spec/FieldSpec.php +++ b/Civi/Api4/Service/Spec/FieldSpec.php @@ -382,9 +382,10 @@ class FieldSpec { /** * @param array $values + * @param array|bool $return * @return array */ - public function getOptions($values = []) { + public function getOptions($values = [], $return = TRUE) { if (!isset($this->options) || $this->options === TRUE) { $fieldName = $this->getName(); @@ -393,18 +394,95 @@ class FieldSpec { $fieldName = sprintf('custom_%d', $this->getCustomFieldId()); } + // BAO::buildOptions returns a single-dimensional list, we call that first because of the hook contract, + // @see CRM_Utils_Hook::fieldOptions + // We then supplement the data with additional properties if requested. $bao = CoreUtil::getBAOFromApiName($this->getEntity()); - $options = $bao::buildOptions($fieldName, NULL, $values); + $optionLabels = $bao::buildOptions($fieldName, NULL, $values); - if (!is_array($options) || !$options) { - $options = FALSE; + if (!is_array($optionLabels) || !$optionLabels) { + $this->options = FALSE; + } + else { + $this->options = \CRM_Utils_Array::makeNonAssociative($optionLabels, 'id', 'label'); + if (is_array($return)) { + self::addOptionProps($bao, $fieldName, $values, $return); + } } - - $this->setOptions($options); } return $this->options; } + /** + * Supplement the data from + * + * @param \CRM_Core_DAO $baoName + * @param string $fieldName + * @param array $values + * @param array $return + */ + private function addOptionProps($baoName, $fieldName, $values, $return) { + // FIXME: For now, call the buildOptions function again and then combine the arrays. Not an ideal approach. + // TODO: Teach CRM_Core_Pseudoconstant to always load multidimensional option lists so we can get more properties like 'color' and 'icon', + // however that might require a change to the hook_civicrm_fieldOptions signature so that's a bit tricky. + if (in_array('name', $return)) { + $props['name'] = $baoName::buildOptions($fieldName, 'validate', $values); + } + $return = array_diff($return, ['id', 'name', 'label']); + // CRM_Core_Pseudoconstant doesn't know how to fetch extra stuff like icon, description, color, etc., so we have to invent that wheel here... + if ($return) { + $optionIds = implode(',', array_column($this->options, 'id')); + $optionIndex = array_flip(array_column($this->options, 'id')); + if ($this instanceof CustomFieldSpec) { + $optionGroupId = \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomField', $this->getCustomFieldId(), 'option_group_id'); + } + else { + $dao = new $baoName(); + $fieldSpec = $dao->getFieldSpec($fieldName); + $pseudoconstant = $fieldSpec['pseudoconstant'] ?? NULL; + $optionGroupName = $pseudoconstant['optionGroupName'] ?? NULL; + $optionGroupId = $optionGroupName ? \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionGroup', $optionGroupName, 'id', 'name') : NULL; + } + if (!empty($optionGroupId)) { + $extraStuff = \CRM_Core_BAO_OptionValue::getOptionValuesArray($optionGroupId); + $keyColumn = $pseudoconstant['keyColumn'] ?? 'value'; + foreach ($extraStuff as $item) { + if (isset($optionIndex[$item[$keyColumn]])) { + foreach ($return as $ret) { + $this->options[$optionIndex[$item[$keyColumn]]][$ret] = $item[$ret] ?? NULL; + } + } + } + } + else { + // Fetch the abbr if requested using context: abbreviate + if (in_array('abbr', $return)) { + $props['abbr'] = $baoName::buildOptions($fieldName, 'abbreviate', $values); + $return = array_diff($return, ['abbr']); + } + // Fetch anything else (color, icon, description) + if ($return && !empty($pseudoconstant['table']) && \CRM_Utils_Rule::commaSeparatedIntegers($optionIds)) { + $sql = "SELECT * FROM {$pseudoconstant['table']} WHERE id IN (%1)"; + $query = \CRM_Core_DAO::executeQuery($sql, [1 => [$optionIds, 'CommaSeparatedIntegers']]); + while ($query->fetch()) { + foreach ($return as $ret) { + if (property_exists($query, $ret)) { + $this->options[$optionIndex[$query->id]][$ret] = $query->$ret; + } + } + } + } + } + } + if (isset($props)) { + foreach ($this->options as &$option) { + foreach ($props as $name => $prop) { + $option[$name] = $prop[$option['id']] ?? NULL; + } + } + } + } + /** * @param array|bool $options * diff --git a/Civi/Api4/Service/Spec/SpecFormatter.php b/Civi/Api4/Service/Spec/SpecFormatter.php index 2ef418d9ee..c6d884f1be 100644 --- a/Civi/Api4/Service/Spec/SpecFormatter.php +++ b/Civi/Api4/Service/Spec/SpecFormatter.php @@ -35,7 +35,7 @@ class SpecFormatter { foreach ($fields as $field) { if ($includeFieldOptions) { - $field->getOptions($values); + $field->getOptions($values, $includeFieldOptions); } $fieldArray[$field->getName()] = $field->toArray(); } diff --git a/Civi/Api4/Utils/FormattingUtil.php b/Civi/Api4/Utils/FormattingUtil.php index 818f591417..e8f27bb594 100644 --- a/Civi/Api4/Utils/FormattingUtil.php +++ b/Civi/Api4/Utils/FormattingUtil.php @@ -181,15 +181,16 @@ class FormattingUtil { * @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 @@ class FormattingUtil { 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 e8a896210e..0a27ebeb13 100644 --- a/ang/api4Explorer/Explorer.js +++ b/ang/api4Explorer/Explorer.js @@ -390,6 +390,17 @@ if (name === 'values') { defaultVal = defaultValues(defaultVal); } + if (name === 'loadOptions' && $scope.action === 'getFields') { + param.options = [ + false, + true, + ['id', 'name', 'label'], + ['id', 'name', 'label', 'abbr', 'description', 'color', 'icon'] + ]; + 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 e6cab2de67..220e534997 100644 --- a/tests/phpunit/api/v4/Action/PseudoconstantTest.php +++ b/tests/phpunit/api/v4/Action/PseudoconstantTest.php @@ -26,7 +26,9 @@ use Civi\Api4\Activity; use Civi\Api4\CustomField; use Civi\Api4\CustomGroup; use Civi\Api4\Email; +use Civi\Api4\EntityTag; use Civi\Api4\OptionValue; +use Civi\Api4\Tag; /** * @group headless @@ -38,24 +40,33 @@ class PseudoconstantTest extends BaseCustomValueTest { $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'])); } @@ -108,30 +119,54 @@ class PseudoconstantTest extends BaseCustomValueTest { } public function testCustomOptions() { + $technicolor = [ + ['id' => 'r', 'name' => 'red', 'label' => 'RED', 'color' => '#ff0000', 'description' => 'Red color', 'icon' => 'fa-red'], + ['id' => 'g', 'name' => 'green', 'label' => 'GREEN', 'color' => '#00ff00', 'description' => 'Green color', 'icon' => 'fa-green'], + ['id' => 'b', 'name' => 'blue', 'label' => 'BLUE', 'color' => '#0000ff', 'description' => 'Blue color', 'icon' => 'fa-blue'], + ]; + CustomGroup::create() ->setCheckPermissions(FALSE) ->addValue('name', 'myPseudoconstantTest') ->addValue('extends', 'Individual') - ->addChain('field', CustomField::create() + ->addChain('field1', 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') + )->addChain('field2', CustomField::create() + ->addValue('custom_group_id', '$id') + ->addValue('option_values', $technicolor) + ->addValue('label', 'Technicolor') + ->addValue('html_type', 'CheckBox') )->execute(); + $fields = Contact::getFields() + ->setLoadOptions(array_keys($technicolor[0])) + ->setIncludeCustom(TRUE) + ->execute() + ->indexBy('name'); + + foreach ($technicolor as $index => $option) { + foreach ($option as $prop => $val) { + $this->assertEquals($val, $fields['myPseudoconstantTest.Technicolor']['options'][$index][$prop]); + } + } + $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']); } @@ -174,4 +209,21 @@ class PseudoconstantTest extends BaseCustomValueTest { $this->assertNull($emails[$cid3]['contact.gender_id:label']); } + public function testTagOptions() { + $tag = uniqid('tag'); + Tag::create()->setCheckPermissions(FALSE) + ->addValue('name', $tag) + ->addValue('description', 'colorful') + ->addValue('color', '#aabbcc') + ->execute(); + $options = EntityTag::getFields() + ->setLoadOptions(['id', 'name', 'color', 'description', 'label']) + ->addWhere('name', '=', 'tag_id') + ->execute()->first()['options']; + $options = array_column($options, NULL, 'name'); + $this->assertEquals('colorful', $options[$tag]['description']); + $this->assertEquals('#aabbcc', $options[$tag]['color']); + $this->assertEquals($tag, $options[$tag]['label']); + } + } diff --git a/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php b/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php index 68ccbb15d0..5ed6c7dd94 100644 --- a/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php +++ b/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php @@ -102,7 +102,7 @@ class TestCreationParameterProvider { * @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 dc8d1bf549..cd1b490392 100644 --- a/tests/phpunit/api/v4/Spec/SpecGathererTest.php +++ b/tests/phpunit/api/v4/Spec/SpecGathererTest.php @@ -101,12 +101,12 @@ class SpecGathererTest extends UnitTestCase { $regularField = $spec->getFieldByName('contact_type'); $this->assertNotEmpty($regularField->getOptions()); - $this->assertContains('Individual', $regularField->getOptions()); + $this->assertContains('Individual', array_column($regularField->getOptions([], ['id', 'label']), 'label', '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([], ['id', 'name', 'label']), NULL, 'id'); + $this->assertEquals('Green', $options['g']['name']); + $this->assertEquals('Pink', $options['p']['label']); } } -- 2.25.1