From 4b3b32e5e8614e93f71dacc176ad2e177187d6a1 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Wed, 2 Jun 2021 21:31:54 -0400 Subject: [PATCH] APIv4 - Cleanup getFields, add @internal flag for non-public field attributes --- Civi/Api4/Generic/AbstractAction.php | 3 +- Civi/Api4/Generic/BasicGetFieldsAction.php | 11 ++++- Civi/Api4/Service/Spec/FieldSpec.php | 45 +++---------------- .../Spec/Provider/CustomValueSpecProvider.php | 4 +- Civi/Api4/Service/Spec/SpecFormatter.php | 4 +- tests/phpunit/api/v4/Action/GetFieldsTest.php | 16 +++++++ 6 files changed, 37 insertions(+), 46 deletions(-) diff --git a/Civi/Api4/Generic/AbstractAction.php b/Civi/Api4/Generic/AbstractAction.php index 050fd5a6ca..4f1f72a9b3 100644 --- a/Civi/Api4/Generic/AbstractAction.php +++ b/Civi/Api4/Generic/AbstractAction.php @@ -440,7 +440,8 @@ abstract class AbstractAction implements \ArrayAccess { 'includeCustom' => FALSE, ]); $result = new Result(); - $getFields->_run($result); + // Pass TRUE for the private $isInternal param + $getFields->_run($result, TRUE); $this->_entityFields = (array) $result->indexBy('name'); } return $this->_entityFields; diff --git a/Civi/Api4/Generic/BasicGetFieldsAction.php b/Civi/Api4/Generic/BasicGetFieldsAction.php index 2a134d4fb7..643ce11946 100644 --- a/Civi/Api4/Generic/BasicGetFieldsAction.php +++ b/Civi/Api4/Generic/BasicGetFieldsAction.php @@ -94,7 +94,9 @@ class BasicGetFieldsAction extends BasicGetAction { else { $values = $this->getRecords(); } - $this->formatResults($values); + // $isInternal param is not part of function signature (to be compatible with parent class) + $isInternal = func_get_args()[1] ?? FALSE; + $this->formatResults($values, $isInternal); $this->queryArray($values, $result); } @@ -109,8 +111,9 @@ class BasicGetFieldsAction extends BasicGetAction { * Instead just override $this->fields and this function will respect that. * * @param array $values + * @param bool $isInternal */ - protected function formatResults(&$values) { + protected function formatResults(&$values, $isInternal) { $fieldDefaults = array_column($this->fields(), 'default_value', 'name') + array_fill_keys(array_column($this->fields(), 'name'), NULL); // Enforce field permissions @@ -121,6 +124,8 @@ class BasicGetFieldsAction extends BasicGetAction { } } } + // Unless this is an internal getFields call, filter out @internal properties + $internalProps = $isInternal ? [] : array_filter(array_column($this->fields(), '@internal', 'name')); foreach ($values as &$field) { $defaults = array_intersect_key([ 'title' => empty($field['name']) ? NULL : ucwords(str_replace('_', ' ', $field['name'])), @@ -134,6 +139,7 @@ class BasicGetFieldsAction extends BasicGetAction { if (isset($defaults['options'])) { $field['options'] = $this->formatOptionList($field['options']); } + $field = array_diff_key($field, $internalProps); } } @@ -325,6 +331,7 @@ class BasicGetFieldsAction extends BasicGetAction { [ 'name' => 'output_formatters', 'data_type' => 'Array', + '@internal' => TRUE, ], ]; } diff --git a/Civi/Api4/Service/Spec/FieldSpec.php b/Civi/Api4/Service/Spec/FieldSpec.php index 54f7b948f8..54b582e25d 100644 --- a/Civi/Api4/Service/Spec/FieldSpec.php +++ b/Civi/Api4/Service/Spec/FieldSpec.php @@ -128,7 +128,7 @@ class FieldSpec { /** * @var callable[] */ - public $outputFormatters = []; + public $outputFormatters; /** * Aliases for the valid data types @@ -390,13 +390,6 @@ class FieldSpec { return $this; } - /** - * @return callable[] - */ - public function getOutputFormatters() { - return $this->outputFormatters; - } - /** * @param callable[] $outputFormatters * @return $this @@ -412,35 +405,24 @@ class FieldSpec { * @return $this */ public function addOutputFormatter($outputFormatter) { + if (!$this->outputFormatters) { + $this->outputFormatters = []; + } $this->outputFormatters[] = $outputFormatter; return $this; } - /** - * @return bool - */ - public function getreadonly() { - return $this->readonly; - } - /** * @param bool $readonly * @return $this */ - public function setreadonly($readonly) { + public function setReadonly($readonly) { $this->readonly = (bool) $readonly; return $this; } - /** - * @return string|NULL - */ - public function getHelpPre() { - return $this->helpPre; - } - /** * @param string|NULL $helpPre */ @@ -448,13 +430,6 @@ class FieldSpec { $this->helpPre = is_string($helpPre) && strlen($helpPre) ? $helpPre : NULL; } - /** - * @return string|NULL - */ - public function getHelpPost() { - return $this->helpPost; - } - /** * @param string|NULL $helpPost */ @@ -464,8 +439,7 @@ class FieldSpec { /** * @param string $customFieldColumnName - * - * @return CustomFieldSpec + * @return $this */ public function setTableName($customFieldColumnName) { $this->tableName = $customFieldColumnName; @@ -530,13 +504,6 @@ class FieldSpec { return $this; } - /** - * @return callable - */ - public function getOptionsCallback() { - return $this->optionsCallback; - } - /** * @return string */ diff --git a/Civi/Api4/Service/Spec/Provider/CustomValueSpecProvider.php b/Civi/Api4/Service/Spec/Provider/CustomValueSpecProvider.php index 5e61b2a54f..3e50a41faa 100644 --- a/Civi/Api4/Service/Spec/Provider/CustomValueSpecProvider.php +++ b/Civi/Api4/Service/Spec/Provider/CustomValueSpecProvider.php @@ -32,14 +32,14 @@ class CustomValueSpecProvider implements Generic\SpecProviderInterface { if ($action !== 'create') { $idField = new FieldSpec('id', $spec->getEntity(), 'Integer'); $idField->setTitle(ts('Custom Value ID')); - $idField->setreadonly(TRUE); + $idField->setReadonly(TRUE); $spec->addFieldSpec($idField); } $entityField = new FieldSpec('entity_id', $spec->getEntity(), 'Integer'); $entityField->setTitle(ts('Entity ID')); $entityField->setRequired($action === 'create'); $entityField->setFkEntity('Contact'); - $entityField->setreadonly(TRUE); + $entityField->setReadonly(TRUE); $spec->addFieldSpec($entityField); } diff --git a/Civi/Api4/Service/Spec/SpecFormatter.php b/Civi/Api4/Service/Spec/SpecFormatter.php index 4a4fc9838d..250e8a186b 100644 --- a/Civi/Api4/Service/Spec/SpecFormatter.php +++ b/Civi/Api4/Service/Spec/SpecFormatter.php @@ -51,7 +51,7 @@ class SpecFormatter { if (self::customFieldHasOptions($data)) { $field->setOptionsCallback([__CLASS__, 'getOptions']); } - $field->setreadonly($data['is_view']); + $field->setReadonly($data['is_view']); } else { $name = $data['name'] ?? NULL; @@ -62,7 +62,7 @@ class SpecFormatter { if (!empty($data['pseudoconstant'])) { $field->setOptionsCallback([__CLASS__, 'getOptions']); } - $field->setreadonly(!empty($data['readonly'])); + $field->setReadonly(!empty($data['readonly'])); } $field->setSerialize($data['serialize'] ?? NULL); $field->setDefaultValue($data['default'] ?? NULL); diff --git a/tests/phpunit/api/v4/Action/GetFieldsTest.php b/tests/phpunit/api/v4/Action/GetFieldsTest.php index f7a735277f..fbb1fa5cfb 100644 --- a/tests/phpunit/api/v4/Action/GetFieldsTest.php +++ b/tests/phpunit/api/v4/Action/GetFieldsTest.php @@ -63,4 +63,20 @@ class GetFieldsTest extends UnitTestCase { $this->assertCount(1, $fields); } + public function testInternalPropsAreHidden() { + // Public getFields should not contain @internal props + $fields = Contact::getFields(FALSE) + ->execute() + ->getArrayCopy(); + foreach ($fields as $field) { + $this->assertArrayNotHasKey('output_formatters', $field); + } + // Internal entityFields should contain @internal props + $fields = Contact::get(FALSE) + ->entityFields(); + foreach ($fields as $field) { + $this->assertArrayHasKey('output_formatters', $field); + } + } + } -- 2.25.1