APIv4 - Cleanup getFields, add @internal flag for non-public field attributes
authorColeman Watts <coleman@civicrm.org>
Thu, 3 Jun 2021 01:31:54 +0000 (21:31 -0400)
committerColeman Watts <coleman@civicrm.org>
Thu, 3 Jun 2021 01:57:12 +0000 (21:57 -0400)
Civi/Api4/Generic/AbstractAction.php
Civi/Api4/Generic/BasicGetFieldsAction.php
Civi/Api4/Service/Spec/FieldSpec.php
Civi/Api4/Service/Spec/Provider/CustomValueSpecProvider.php
Civi/Api4/Service/Spec/SpecFormatter.php
tests/phpunit/api/v4/Action/GetFieldsTest.php

index 050fd5a6ca43757495e47d8d3e50a64ec1c85e3c..4f1f72a9b3f872a0a0a88f36cb61fb32a1859565 100644 (file)
@@ -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;
index 2a134d4fb73cc756de8b64079f22cae16b8e19ef..643ce11946a4174545db8a249a16448f2d5eca07 100644 (file)
@@ -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,
       ],
     ];
   }
index 54f7b948f868c01c66acd8ea8fd44dc904915b5e..54b582e25d4e6b88f3af97f2a509e1bf5babac0e 100644 (file)
@@ -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
    */
index 5e61b2a54f43dc4fb303762ed8a65f93cc132fbf..3e50a41faa300f8f22e3b61b369d3b22a8104a41 100644 (file)
@@ -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);
   }
 
index 4a4fc9838d82b17f8de230c3040f48aa3efe222a..250e8a186b6e4efa7a1360bb65e77ba4f0e2515d 100644 (file)
@@ -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);
index f7a735277fbb177953a8cbb0b7f363f00d28ea55..fbb1fa5cfbe3acd5f3d6205e536beab943094c68 100644 (file)
@@ -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);
+    }
+  }
+
 }