APIv4 - Setting api misc fixes & tests
authorColeman Watts <coleman@civicrm.org>
Thu, 29 Apr 2021 15:32:39 +0000 (11:32 -0400)
committerColeman Watts <coleman@civicrm.org>
Tue, 4 May 2021 21:52:54 +0000 (17:52 -0400)
Fixes the setting api to correctly handle pseudoconstants & field suffixes

CRM/Core/BAO/Setting.php
CRM/Core/SelectValues.php
Civi/Api4/Action/Setting/AbstractSettingAction.php
Civi/Api4/Action/Setting/Get.php
Civi/Api4/Action/Setting/GetFields.php
Civi/Core/SettingsMetadata.php
api/api.php
tests/phpunit/api/v3/SettingTest.php
tests/phpunit/api/v4/Entity/SettingTest.php

index 02ec91a4ac5924d341b5fd820afa73ea3cde99a0..f075a24957346695449759a50d48e7cc9d22a072 100644 (file)
@@ -272,13 +272,15 @@ class CRM_Core_BAO_Setting extends CRM_Core_DAO_Setting {
    *   value of the setting to be set
    * @param array $fieldSpec
    *   Metadata for given field (drawn from the xml)
+   * @param bool $convertToSerializedString
+   *   Deprecated mode
    *
    * @return bool
    * @throws \API_Exception
    */
-  public static function validateSetting(&$value, array $fieldSpec) {
+  public static function validateSetting(&$value, array $fieldSpec, $convertToSerializedString = TRUE) {
     // Deprecated guesswork - should use $fieldSpec['serialize']
-    if ($fieldSpec['type'] == 'String' && is_array($value)) {
+    if ($convertToSerializedString && $fieldSpec['type'] == 'String' && is_array($value)) {
       $value = CRM_Core_DAO::VALUE_SEPARATOR . implode(CRM_Core_DAO::VALUE_SEPARATOR, $value) . CRM_Core_DAO::VALUE_SEPARATOR;
     }
     if (empty($fieldSpec['validate_callback'])) {
index 4ebec39596c691e7629c2d061174abdda077ef86..69866466fb7b1b9fc903ceb9331d01fadc8181e8 100644 (file)
@@ -437,7 +437,7 @@ class CRM_Core_SelectValues {
    *
    * @throws \CRM_Core_Exception
    */
-  public function taxDisplayOptions() {
+  public static function taxDisplayOptions() {
     return [
       'Do_not_show' => ts('Do not show breakdown, only show total - i.e %1', [
         1 => CRM_Utils_Money::format(120),
index abbd07ae9aaa7c54bb144e1d0f6d5908ea171217..3f362dc40462444b3f93c9348f598f941093a303 100644 (file)
@@ -58,14 +58,24 @@ abstract class AbstractSettingAction extends \Civi\Api4\Generic\AbstractAction {
    */
   protected function validateSettings($domain) {
     $meta = \Civi\Core\SettingsMetadata::getMetadata([], $domain);
-    $names = isset($this->values) ? array_keys($this->values) : $this->select;
+    $names = array_map(function($name) {
+      return explode(':', $name)[0];
+    }, isset($this->values) ? array_keys($this->values) : $this->select);
     $invalid = array_diff($names, array_keys($meta));
     if ($invalid) {
       throw new \API_Exception("Unknown settings for domain $domain: " . implode(', ', $invalid));
     }
     if (isset($this->values)) {
-      foreach ($this->values as $name => &$value) {
-        \CRM_Core_BAO_Setting::validateSetting($value, $meta[$name]);
+      foreach ($this->values as $name => $value) {
+        [$name, $suffix] = array_pad(explode(':', $name), 2, NULL);
+        // Replace pseudoconstants in values array
+        if ($suffix) {
+          $value = $this->matchPseudoconstant($name, $value, $suffix, 'id', $domain);
+          unset($this->values["$name:$suffix"]);
+          $this->values[$name] = $value;
+        }
+        \CRM_Core_BAO_Setting::validateSetting($this->values[$name], $meta[$name], FALSE);
+
       }
     }
     return $meta;
@@ -88,4 +98,34 @@ abstract class AbstractSettingAction extends \Civi\Api4\Generic\AbstractAction {
     }
   }
 
+  /**
+   * @param string $name
+   * @param mixed $value
+   * @param string $from
+   * @param string $to
+   * @param int $domain
+   * @return mixed
+   */
+  protected function matchPseudoconstant(string $name, $value, $from, $to, $domain) {
+    if ($value === NULL) {
+      return NULL;
+    }
+    if ($from === $to) {
+      return $value;
+    }
+    $meta = \Civi\Core\SettingsMetadata::getMetadata(['name' => [$name]], $domain, [$from, $to]);
+    $options = $meta[$name]['options'] ?? [];
+    $map = array_column($options, $to, $from);
+    $translated = [];
+    foreach ((array) $value as $key) {
+      if (isset($map[$key])) {
+        $translated[] = $map[$key];
+      }
+    }
+    if (!is_array($value)) {
+      return \CRM_Utils_Array::first($translated);
+    }
+    return $translated;
+  }
+
 }
index 6ce7b39b8e0b96659c4e47564cb75ed2e6136b42..e048c795614ee5575a52c09bf9532868342c5200 100644 (file)
@@ -39,15 +39,26 @@ class Get extends AbstractSettingAction {
   protected function processSettings(Result $result, $settingsBag, $meta, $domain) {
     if ($this->select) {
       foreach ($this->select as $name) {
+        [$name, $suffix] = array_pad(explode(':', $name), 2, NULL);
+        $value = $settingsBag->get($name);
+        if (isset($value) && !empty($meta[$name]['serialize'])) {
+          $value = \CRM_Core_DAO::unSerializeField($value, $meta[$name]['serialize']);
+        }
+        if ($suffix) {
+          $value = $this->matchPseudoconstant($name, $value, 'id', $suffix, $domain);
+        }
         $result[] = [
-          'name' => $name,
-          'value' => $settingsBag->get($name),
+          'name' => $suffix ? "$name:$suffix" : $name,
+          'value' => $value,
           'domain_id' => $domain,
         ];
       }
     }
     else {
       foreach ($settingsBag->all() as $name => $value) {
+        if (isset($value) && !empty($meta[$name]['serialize'])) {
+          $value = \CRM_Core_DAO::unSerializeField($value, $meta[$name]['serialize']);
+        }
         $result[] = [
           'name' => $name,
           'value' => $value,
@@ -55,11 +66,6 @@ class Get extends AbstractSettingAction {
         ];
       }
     }
-    foreach ($result as &$setting) {
-      if (isset($setting['value']) && !empty($meta[$setting['name']]['serialize'])) {
-        $setting['value'] = \CRM_Core_DAO::unSerializeField($setting['value'], $meta[$setting['name']]['serialize']);
-      }
-    }
   }
 
   /**
index 44711ef4c82680fdce5e1b20e5c53c78c7274310..bcd9a048212d3646cb5dde871ead445668472a0c 100644 (file)
@@ -28,11 +28,21 @@ class GetFields extends \Civi\Api4\Generic\BasicGetFieldsAction {
   protected $domainId;
 
   protected function getRecords() {
-    // TODO: Waiting for filter handling to get fixed in core
-    // $names = $this->_itemsToGet('name');
-    // $filter = $names ? ['name' => $names] : [];
-    $filter = [];
-    return \Civi\Core\SettingsMetadata::getMetadata($filter, $this->domainId, $this->loadOptions);
+    $names = $this->_itemsToGet('name');
+    $filter = $names ? ['name' => $names] : [];
+    $settings = \Civi\Core\SettingsMetadata::getMetadata($filter, $this->domainId, $this->loadOptions);
+    foreach ($settings as $index => $setting) {
+      // Unserialize default value
+      if (!empty($setting['serialize']) && !empty($setting['default']) && is_string($setting['default'])) {
+        $setting['default'] = \CRM_Core_DAO::unSerializeField($setting['default'], $setting['serialize']);
+      }
+      if (!isset($setting['options'])) {
+        $setting['options'] = !empty($setting['pseudoconstant']);
+      }
+      // Filter out deprecated properties
+      $settings[$index] = array_intersect_key($setting, array_column($this->fields(), NULL, 'name'));
+    }
+    return $settings;
   }
 
   public function fields() {
@@ -57,22 +67,10 @@ class GetFields extends \Civi\Api4\Generic\BasicGetFieldsAction {
         'name' => 'default',
         'data_type' => 'String',
       ],
-      [
-        'name' => 'pseudoconstant',
-        'data_type' => 'String',
-      ],
       [
         'name' => 'options',
         'data_type' => 'Array',
       ],
-      [
-        'name' => 'group_name',
-        'data_type' => 'String',
-      ],
-      [
-        'name' => 'group',
-        'data_type' => 'String',
-      ],
       [
         'name' => 'html_type',
         'data_type' => 'String',
index fb5c05aed8a906d36e130a8b4c04196e224f7047..c91e5b616b96b31e74b0fd065c096b22735bc2c7 100644 (file)
@@ -35,7 +35,7 @@ class SettingsMetadata {
    *
    * @param array $filters
    * @param int $domainID
-   * @param bool $loadOptions
+   * @param bool|array $loadOptions
    *
    * @return array
    *   the following information as appropriate for each setting
@@ -70,7 +70,7 @@ class SettingsMetadata {
 
     self::_filterSettingsSpecification($filters, $settingsMetadata);
     if ($loadOptions) {
-      self::loadOptions($settingsMetadata);
+      self::fillOptions($settingsMetadata, $loadOptions);
     }
 
     return $settingsMetadata;
@@ -98,7 +98,7 @@ class SettingsMetadata {
   /**
    * Load up settings metadata from files.
    *
-   * @param array $metaDataFolder
+   * @param string $metaDataFolder
    *
    * @return array
    */
@@ -141,22 +141,35 @@ class SettingsMetadata {
    * Retrieve options from settings metadata
    *
    * @param array $settingSpec
+   * @param bool|array $optionsFormat
+   *   TRUE for a flat array; otherwise an array of keys to return
    */
-  protected static function loadOptions(&$settingSpec) {
+  protected static function fillOptions(&$settingSpec, $optionsFormat) {
     foreach ($settingSpec as &$spec) {
       if (empty($spec['pseudoconstant'])) {
         continue;
       }
       $pseudoconstant = $spec['pseudoconstant'];
+      $spec['options'] = [];
       // It would be nice if we could leverage CRM_Core_PseudoConstant::get() somehow,
       // but it's tightly coupled to DAO/field. However, if you really need to support
       // more pseudoconstant types, then probably best to refactor it. For now, KISS.
-      if (!empty($pseudoconstant['callback'])) {
-        $spec['options'] = Resolver::singleton()->call($pseudoconstant['callback'], []);
-      }
-      elseif (!empty($pseudoconstant['optionGroupName'])) {
+      if (!empty($pseudoconstant['optionGroupName'])) {
         $keyColumn = \CRM_Utils_Array::value('keyColumn', $pseudoconstant, 'value');
-        $spec['options'] = \CRM_Core_OptionGroup::values($pseudoconstant['optionGroupName'], FALSE, FALSE, TRUE, NULL, 'label', TRUE, FALSE, $keyColumn);
+        if (is_array($optionsFormat)) {
+          $optionValues = \CRM_Core_OptionValue::getValues(['name' => $pseudoconstant['optionGroupName']]);
+          foreach ($optionValues as $option) {
+            $option['id'] = $option['value'];
+            $spec['options'][] = $option;
+          }
+        }
+        else {
+          $spec['options'] = \CRM_Core_OptionGroup::values($pseudoconstant['optionGroupName'], FALSE, FALSE, TRUE, NULL, 'label', TRUE, FALSE, $keyColumn);
+        }
+        continue;
+      }
+      if (!empty($pseudoconstant['callback'])) {
+        $options = Resolver::singleton()->call($pseudoconstant['callback'], []);
       }
       if (!empty($pseudoconstant['table'])) {
         $params = [
@@ -164,7 +177,19 @@ class SettingsMetadata {
           'keyColumn' => $pseudoconstant['keyColumn'] ?? NULL,
           'labelColumn' => $pseudoconstant['labelColumn'] ?? NULL,
         ];
-        $spec['options'] = \CRM_Core_PseudoConstant::renderOptionsFromTablePseudoconstant($pseudoconstant, $params, ($spec['localize_context'] ?? NULL), 'get');
+        $options = \CRM_Core_PseudoConstant::renderOptionsFromTablePseudoconstant($pseudoconstant, $params, ($spec['localize_context'] ?? NULL), 'get');
+      }
+      if (is_array($optionsFormat)) {
+        foreach ($options as $key => $value) {
+          $spec['options'][] = [
+            'id' => $key,
+            'name' => $value,
+            'label' => $value,
+          ];
+        }
+      }
+      else {
+        $spec['options'] = $options;
       }
     }
   }
index 7e45ab2ae62d6a35494e63e2225ffa823770074e..9ed9747dd0ca0b672fac76462c2f0f9ab911e6b2 100644 (file)
@@ -64,7 +64,7 @@ function civicrm_api4(string $entity, string $action, array $params = [], $index
   $removeIndexField = FALSE;
 
   // If index field is not part of the select query, we add it here and remove it below (except for oddball "Setting" api)
-  if ($indexField && !empty($params['select']) && is_array($params['select']) && $entity !== 'Setting' && !\Civi\Api4\Utils\SelectUtil::isFieldSelected($indexField, $params['select'])) {
+  if ($indexField && !empty($params['select']) && is_array($params['select']) && !($entity === 'Setting' && $action === 'get') && !\Civi\Api4\Utils\SelectUtil::isFieldSelected($indexField, $params['select'])) {
     $params['select'][] = $indexField;
     $removeIndexField = TRUE;
   }
index 1bc407ee8258fdf9c36e0f9c764d0619a91e4b31..36474d33d0215eaa62be68fff613a5ac754f8489 100644 (file)
@@ -111,11 +111,9 @@ class api_v3_SettingTest extends CiviUnitTestCase {
 
   /**
    * Test that getfields will filter on group.
-   * @param int $version
-   * @dataProvider versionThreeAndFour
    */
-  public function testGetFieldsGroupFilters($version) {
-    $this->_apiversion = $version;
+  public function testGetFieldsGroupFilters() {
+    $this->_apiversion = 3;
     $params = ['filters' => ['group' => 'multisite']];
     $result = $this->callAPISuccess('setting', 'getfields', $params);
     $this->assertArrayNotHasKey('customCSSURL', $result['values']);
@@ -363,11 +361,11 @@ class api_v3_SettingTest extends CiviUnitTestCase {
     $this->hookClass->setHook('civicrm_alterSettingsFolders', [$this, 'setExtensionMetadata']);
     $data = NULL;
     Civi::cache('settings')->flush();
-    $fields = $this->callAPISuccess('setting', 'getfields', ['filters' => ['group_name' => 'Test Settings']]);
+    $fields = $this->callAPISuccess('setting', 'getfields');
     $this->assertArrayHasKey('test_key', $fields['values']);
     $this->callAPISuccess('setting', 'create', ['test_key' => 'keyset']);
     $this->assertEquals('keyset', Civi::settings()->get('test_key'));
-    $result = $this->callAPISuccess('setting', 'getvalue', ['name' => 'test_key', 'group' => 'Test Settings']);
+    $result = $this->callAPISuccess('setting', 'getvalue', ['name' => 'test_key']);
     $this->assertEquals('keyset', $result);
   }
 
index a50c00605ce5b69702447bad08dc5a08a25a62be..9db05160a8169fd156ac5d4bb4c0a43b6ee9f84a 100644 (file)
@@ -51,10 +51,24 @@ class SettingTest extends UnitTestCase {
   }
 
   public function testSerailizedSetting() {
-    $settings = \Civi\Api4\Setting::get(FALSE)
-      ->addSelect('contact_edit_options')
+    $set = \Civi\Api4\Setting::set(FALSE)
+      ->addValue('contact_edit_options:name', [
+        'CommunicationPreferences',
+        'CustomData',
+      ])
       ->execute();
-    $this->assertTrue(is_array($settings[0]['value']));
+
+    $setting = \Civi\Api4\Setting::get(FALSE)
+      ->addSelect('contact_edit_options')
+      ->execute()->first();
+    $this->assertTrue(is_array($setting['value']));
+    $this->assertTrue(is_numeric($setting['value'][0]));
+
+    $setting = \Civi\Api4\Setting::get(FALSE)
+      ->addSelect('contact_edit_options:label')
+      ->execute()->first();
+    $this->assertEquals(['Communication Preferences', 'Custom Data'], $setting['value']);
+    $this->assertEquals('contact_edit_options:label', $setting['name']);
   }
 
   /**
@@ -78,4 +92,36 @@ class SettingTest extends UnitTestCase {
     $this->assertTrue($arrayValues);
   }
 
+  /**
+   * Make sure options load from getFields.
+   */
+  public function testSettingGetFieldsOptions() {
+    $setting = civicrm_api4('Setting', 'getFields', [
+      'select' => ['options'],
+      'loadOptions' => FALSE,
+    ], 'name');
+    $this->assertTrue($setting['contact_edit_options']['options']);
+
+    $setting = civicrm_api4('Setting', 'getFields', [
+      'select' => ['options'],
+      'where' => [['name', '=', 'contact_edit_options']],
+      'loadOptions' => TRUE,
+    ], 'name');
+    $this->assertContains('Custom Data', $setting['contact_edit_options']['options']);
+
+    $setting = civicrm_api4('Setting', 'getFields', [
+      'select' => ['options'],
+      'loadOptions' => ['id', 'name', 'label'],
+    ], 'name');
+    $this->assertTrue(is_array($setting['contact_edit_options']['options'][0]));
+  }
+
+  /**
+   * Ensure settings default values unserialize.
+   */
+  public function testSettingUnserializeDefaults() {
+    $setting = civicrm_api4('Setting', 'getFields', ['where' => [['name', '=', 'contact_view_options']]], 0);
+    $this->assertTrue(is_array($setting['default']));
+  }
+
 }