From 721c9da1a14e155e13d528106ca32690f171b515 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 3 Sep 2020 17:50:07 -0400 Subject: [PATCH] CustomField - Reformat data when modifying field serialize property. Adds or removes CRM_Core_DAO::VALUE_SEPARATOR from custom values when switching a field from single to muti-valued. --- CRM/Core/BAO/CustomField.php | 27 +++++ CRM/Core/DAO/AllCoreTables.php | 4 +- Civi/Api4/Generic/AbstractAction.php | 6 +- Civi/Api4/Generic/Traits/DAOActionTrait.php | 28 +++-- Civi/Api4/Query/Api4SelectQuery.php | 2 +- .../Schema/Joinable/CustomGroupJoinable.php | 11 +- .../Api4/Service/Schema/Joinable/Joinable.php | 18 +-- Civi/Api4/Utils/FormattingUtil.php | 18 ++- .../api/v4/Action/CustomFieldAlterTest.php | 114 ++++++++++++++++++ 9 files changed, 187 insertions(+), 41 deletions(-) create mode 100644 tests/phpunit/api/v4/Action/CustomFieldAlterTest.php diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php index 107242a515..c67b60f7a9 100644 --- a/CRM/Core/BAO/CustomField.php +++ b/CRM/Core/BAO/CustomField.php @@ -1696,6 +1696,25 @@ SELECT $columnName return CRM_Core_BAO_SchemaHandler::getFieldAlterSQL($params, $indexExist); } + /** + * Reformat existing values for a field when changing its serialize attribute + * + * @param CRM_Core_DAO_CustomField $field + * @throws CRM_Core_Exception + */ + private static function alterSerialize($field) { + $table = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $field->custom_group_id, 'table_name'); + $col = $field->column_name; + $sp = CRM_Core_DAO::VALUE_SEPARATOR; + if ($field->serialize) { + $sql = "UPDATE `$table` SET `$col` = CONCAT('$sp', `$col`, '$sp') WHERE `$col` IS NOT NULL AND `$col` NOT LIKE '$sp%' AND `$col` != ''"; + } + else { + $sql = "UPDATE `$table` SET `$col` = SUBSTRING_INDEX(SUBSTRING(`$col`, 2), '$sp', 1) WHERE `$col` LIKE '$sp%'"; + } + CRM_Core_DAO::executeQuery($sql); + } + /** * Determine whether it would be safe to move a field. * @@ -1989,6 +2008,9 @@ WHERE id IN ( %1, %2 ) $transaction = new CRM_Core_Transaction(); $params = self::prepareCreate($params); + $alterSerialize = isset($params['serialize']) && !empty($params['id']) + && ($params['serialize'] != CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomField', $params['id'], 'serialize')); + $customField = new CRM_Core_DAO_CustomField(); $customField->copyValues($params); $customField->save(); @@ -2008,6 +2030,11 @@ WHERE id IN ( %1, %2 ) // make sure all values are present in the object for further processing $customField->find(TRUE); + + if ($alterSerialize) { + self::alterSerialize($customField); + } + return $customField; } diff --git a/CRM/Core/DAO/AllCoreTables.php b/CRM/Core/DAO/AllCoreTables.php index 5b9ffe5ef6..9e4970c301 100644 --- a/CRM/Core/DAO/AllCoreTables.php +++ b/CRM/Core/DAO/AllCoreTables.php @@ -280,7 +280,7 @@ class CRM_Core_DAO_AllCoreTables { * Get the classname for the table. * * @param string $tableName - * @return string + * @return string|CRM_Core_DAO|NULL */ public static function getClassForTable($tableName) { //CRM-19677: on multilingual setup, trim locale from $tableName to fetch class name @@ -296,7 +296,7 @@ class CRM_Core_DAO_AllCoreTables { * * @param string $daoName * Ex: 'Contact'. - * @return CRM_Core_DAO|NULL + * @return string|CRM_Core_DAO|NULL * Ex: 'CRM_Contact_DAO_Contact'. */ public static function getFullName($daoName) { diff --git a/Civi/Api4/Generic/AbstractAction.php b/Civi/Api4/Generic/AbstractAction.php index 8d0111e6b3..ea17309053 100644 --- a/Civi/Api4/Generic/AbstractAction.php +++ b/Civi/Api4/Generic/AbstractAction.php @@ -494,7 +494,7 @@ abstract class AbstractAction implements \ArrayAccess { if ($field) { $optionFields[$fieldName] = [ 'val' => $record[$expr], - 'name' => empty($field['custom_field_id']) ? $field['name'] : 'custom_' . $field['custom_field_id'], + 'field' => $field, 'suffix' => substr($expr, $suffix + 1), 'depends' => $field['input_attrs']['controlField'] ?? NULL, ]; @@ -504,11 +504,11 @@ abstract class AbstractAction implements \ArrayAccess { } // Sort option lookups by dependency, so e.g. country_id is processed first, then state_province_id, then county_id uasort($optionFields, function ($a, $b) { - return $a['name'] === $b['depends'] ? -1 : 1; + return $a['field']['name'] === $b['depends'] ? -1 : 1; }); // Replace pseudoconstants. Note this is a reverse lookup as we are evaluating input not output. foreach ($optionFields as $fieldName => $info) { - $options = FormattingUtil::getPseudoconstantList($this->_entityName, $info['name'], $info['suffix'], $record, 'create'); + $options = FormattingUtil::getPseudoconstantList($info['field'], $info['suffix'], $record, 'create'); $record[$fieldName] = FormattingUtil::replacePseudoconstant($options, $info['val'], TRUE); } } diff --git a/Civi/Api4/Generic/Traits/DAOActionTrait.php b/Civi/Api4/Generic/Traits/DAOActionTrait.php index 6497168a3d..9404b11ecf 100644 --- a/Civi/Api4/Generic/Traits/DAOActionTrait.php +++ b/Civi/Api4/Generic/Traits/DAOActionTrait.php @@ -13,6 +13,7 @@ namespace Civi\Api4\Generic\Traits; use Civi\Api4\CustomField; +use Civi\Api4\Service\Schema\Joinable\CustomGroupJoinable; use Civi\Api4\Utils\FormattingUtil; /** @@ -178,7 +179,7 @@ trait DAOActionTrait { if (NULL !== $value) { if ($field['suffix']) { - $options = FormattingUtil::getPseudoconstantList($this->getEntityName(), 'custom_' . $field['id'], $field['suffix'], $params, $this->getActionName()); + $options = FormattingUtil::getPseudoconstantList($field, $field['suffix'], $params, $this->getActionName()); $value = FormattingUtil::replacePseudoconstant($options, $value, TRUE); } @@ -210,24 +211,33 @@ trait DAOActionTrait { /** * Gets field info needed to save custom data * - * @param string $name + * @param string $fieldExpr * Field identifier with possible suffix, e.g. MyCustomGroup.MyField1:label * @return array|NULL */ - protected function getCustomFieldInfo($name) { - if (strpos($name, '.') === FALSE) { + protected function getCustomFieldInfo(string $fieldExpr) { + if (strpos($fieldExpr, '.') === FALSE) { return NULL; } - list($groupName, $fieldName) = explode('.', $name); + list($groupName, $fieldName) = explode('.', $fieldExpr); list($fieldName, $suffix) = array_pad(explode(':', $fieldName), 2, NULL); - if (empty(\Civi::$statics['APIv4_Custom_Fields'][$groupName])) { - \Civi::$statics['APIv4_Custom_Fields'][$groupName] = (array) CustomField::get(FALSE) + $cacheKey = "APIv4_Custom_Fields-$groupName"; + $info = \Civi::cache('metadata')->get($cacheKey); + if (!isset($info[$fieldName])) { + $info = []; + $fields = CustomField::get(FALSE) ->addSelect('id', 'name', 'html_type', 'custom_group.extends') ->addWhere('custom_group.name', '=', $groupName) ->execute()->indexBy('name'); + foreach ($fields as $name => $field) { + $field['custom_field_id'] = $field['id']; + $field['name'] = $groupName . '.' . $name; + $field['entity'] = CustomGroupJoinable::getEntityFromExtends($field['custom_group.extends']); + $info[$name] = $field; + } + \Civi::cache('metadata')->set($cacheKey, $info); } - $info = \Civi::$statics['APIv4_Custom_Fields'][$groupName][$fieldName] ?? NULL; - return $info ? ['suffix' => $suffix] + $info : NULL; + return isset($info[$fieldName]) ? ['suffix' => $suffix] + $info[$fieldName] : NULL; } /** diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index e4f8ceb187..009095ebc9 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -275,7 +275,7 @@ class Api4SelectQuery { $suffix = strstr($item, ':'); if ($suffix && $expr->getType() === 'SqlField') { $field = $this->getField($item); - $options = FormattingUtil::getPseudoconstantList($field['entity'], $field['name'], substr($suffix, 1)); + $options = FormattingUtil::getPseudoconstantList($field, substr($suffix, 1)); if ($options) { asort($options); $column = "FIELD($column,'" . implode("','", array_keys($options)) . "')"; diff --git a/Civi/Api4/Service/Schema/Joinable/CustomGroupJoinable.php b/Civi/Api4/Service/Schema/Joinable/CustomGroupJoinable.php index a8ed4042b0..510ff78826 100644 --- a/Civi/Api4/Service/Schema/Joinable/CustomGroupJoinable.php +++ b/Civi/Api4/Service/Schema/Joinable/CustomGroupJoinable.php @@ -56,16 +56,19 @@ class CustomGroupJoinable extends Joinable { * @inheritDoc */ public function getEntityFields() { - if (!$this->entityFields) { + $cacheKey = 'APIv4_CustomGroupJoinable-' . $this->getTargetTable(); + $entityFields = (array) \Civi::cache('metadata')->get($cacheKey); + if (!$entityFields) { $fields = CustomField::get(FALSE) ->setSelect(['custom_group.name', 'custom_group.extends', 'custom_group.table_name', '*']) ->addWhere('custom_group.table_name', '=', $this->getTargetTable()) ->execute(); foreach ($fields as $field) { - $this->entityFields[] = \Civi\Api4\Service\Spec\SpecFormatter::arrayToField($field, $this->getEntityFromExtends($field['custom_group.extends'])); + $entityFields[] = \Civi\Api4\Service\Spec\SpecFormatter::arrayToField($field, self::getEntityFromExtends($field['custom_group.extends'])); } + \Civi::cache('metadata')->set($cacheKey, $entityFields); } - return $this->entityFields; + return $entityFields; } /** @@ -102,7 +105,7 @@ class CustomGroupJoinable extends Joinable { * @throws \API_Exception * @throws \Civi\API\Exception\UnauthorizedException */ - private function getEntityFromExtends($extends) { + public static function getEntityFromExtends($extends) { if (strpos($extends, 'Participant') === 0) { return 'Participant'; } diff --git a/Civi/Api4/Service/Schema/Joinable/Joinable.php b/Civi/Api4/Service/Schema/Joinable/Joinable.php index 0375fa0c2a..8e6d4e7116 100644 --- a/Civi/Api4/Service/Schema/Joinable/Joinable.php +++ b/Civi/Api4/Service/Schema/Joinable/Joinable.php @@ -78,11 +78,6 @@ class Joinable { */ protected $entity; - /** - * @var array - */ - protected $entityFields; - /** * @param $targetTable * @param $targetColumn @@ -268,15 +263,14 @@ class Joinable { * @return \Civi\Api4\Service\Spec\FieldSpec[] */ public function getEntityFields() { - if (!$this->entityFields) { - $bao = AllCoreTables::getClassForTable($this->getTargetTable()); - if ($bao) { - foreach ($bao::fields() as $field) { - $this->entityFields[] = \Civi\Api4\Service\Spec\SpecFormatter::arrayToField($field, $this->getEntity()); - } + $entityFields = []; + $bao = AllCoreTables::getClassForTable($this->getTargetTable()); + if ($bao) { + foreach ($bao::getSupportedFields() as $field) { + $entityFields[] = \Civi\Api4\Service\Spec\SpecFormatter::arrayToField($field, $this->getEntity()); } } - return $this->entityFields; + return $entityFields; } /** diff --git a/Civi/Api4/Utils/FormattingUtil.php b/Civi/Api4/Utils/FormattingUtil.php index 900df21ccf..f170969a7f 100644 --- a/Civi/Api4/Utils/FormattingUtil.php +++ b/Civi/Api4/Utils/FormattingUtil.php @@ -88,7 +88,7 @@ class FormattingUtil { // Evaluate pseudoconstant suffix $suffix = strpos($fieldName, ':'); if ($suffix) { - $options = self::getPseudoconstantList($fieldSpec['entity'], $fieldSpec['name'], substr($fieldName, $suffix + 1), $action); + $options = self::getPseudoconstantList($fieldSpec, substr($fieldName, $suffix + 1), $action); $value = self::replacePseudoconstant($options, $value, TRUE); return; } @@ -148,8 +148,7 @@ class FormattingUtil { // Evaluate pseudoconstant suffixes $suffix = strrpos($fieldExpr, ':'); if ($suffix) { - $fieldName = empty($field['custom_field_id']) ? $field['name'] : 'custom_' . $field['custom_field_id']; - $fieldOptions[$fieldExpr] = $fieldOptions[$fieldExpr] ?? self::getPseudoconstantList($field['entity'], $fieldName, substr($fieldExpr, $suffix + 1), $result, $action); + $fieldOptions[$fieldExpr] = $fieldOptions[$fieldExpr] ?? self::getPseudoconstantList($field, substr($fieldExpr, $suffix + 1), $result, $action); $dataType = NULL; } if (!empty($field['serialize'])) { @@ -178,9 +177,7 @@ class FormattingUtil { /** * Retrieves pseudoconstant option list for a field. * - * @param string $entity - * Name of api entity - * @param string $fieldName + * @param array $field * @param string $valueType * name|label|abbr from self::$pseudoConstantContexts * @param array $params @@ -189,27 +186,28 @@ class FormattingUtil { * @return array * @throws \API_Exception */ - public static function getPseudoconstantList($entity, $fieldName, $valueType, $params = [], $action = 'get') { + public static function getPseudoconstantList($field, $valueType, $params = [], $action = 'get') { $context = self::$pseudoConstantContexts[$valueType] ?? NULL; // For create actions, only unique identifiers can be used. // For get actions any valid suffix is ok. if (($action === 'create' && !$context) || !in_array($valueType, self::$pseudoConstantSuffixes, TRUE)) { throw new \API_Exception('Illegal expression'); } - $baoName = $context ? CoreUtil::getBAOFromApiName($entity) : NULL; + $baoName = $context ? CoreUtil::getBAOFromApiName($field['entity']) : NULL; // Use BAO::buildOptions if possible if ($baoName) { + $fieldName = empty($field['custom_field_id']) ? $field['name'] : 'custom_' . $field['custom_field_id']; $options = $baoName::buildOptions($fieldName, $context, $params); } // Fallback for option lists that exist in the api but not the BAO if (!isset($options) || $options === FALSE) { - $options = civicrm_api4($entity, 'getFields', ['action' => $action, 'loadOptions' => ['id', $valueType], 'where' => [['name', '=', $fieldName]]])[0]['options'] ?? NULL; + $options = civicrm_api4($field['entity'], 'getFields', ['action' => $action, 'loadOptions' => ['id', $valueType], 'where' => [['name', '=', $field['name']]]])[0]['options'] ?? NULL; $options = $options ? array_column($options, $valueType, 'id') : $options; } if (is_array($options)) { return $options; } - throw new \API_Exception("No option list found for '$fieldName'"); + throw new \API_Exception("No option list found for '{$field['name']}'"); } /** diff --git a/tests/phpunit/api/v4/Action/CustomFieldAlterTest.php b/tests/phpunit/api/v4/Action/CustomFieldAlterTest.php new file mode 100644 index 0000000000..3e44ca82e2 --- /dev/null +++ b/tests/phpunit/api/v4/Action/CustomFieldAlterTest.php @@ -0,0 +1,114 @@ +createEntity(['type' => 'Individual']); + + $customGroup = CustomGroup::create(FALSE) + ->addValue('title', 'MyFieldsToAlter') + ->addValue('extends', 'Activity') + ->addChain('field1', CustomField::create() + ->addValue('custom_group_id', '$id') + ->addValue('label', 'TestOptions') + ->addValue('html_type', 'Select') + ->addValue('option_values', [ + 1 => 'One', + 2 => 'Two', + 3 => 'Three', + 4 => 'Four', + ]), 0 + ) + ->addChain('field2', CustomField::create() + ->addValue('custom_group_id', '$id') + ->addValue('label', 'TestText') + ->addValue('html_type', 'Text'), 0 + ) + ->execute() + ->first(); + + Activity::save(FALSE) + ->setDefaults(['activity_type_id' => 1, 'source_contact_id' => $contact['id']]) + ->addRecord(['subject' => 'A1', 'MyFieldsToAlter.TestText' => 'A1', 'MyFieldsToAlter.TestOptions' => '1']) + ->addRecord(['subject' => 'A2', 'MyFieldsToAlter.TestText' => 'A2', 'MyFieldsToAlter.TestOptions' => '2']) + ->addRecord(['subject' => 'A3', 'MyFieldsToAlter.TestText' => 'A3', 'MyFieldsToAlter.TestOptions' => '']) + ->addRecord(['subject' => 'A4', 'MyFieldsToAlter.TestText' => 'A4']) + ->execute(); + + $result = Activity::get(FALSE) + ->addWhere('MyFieldsToAlter.TestText', 'IS NOT NULL') + ->addSelect('custom.*', 'subject', 'MyFieldsToAlter.TestOptions:label') + ->execute()->indexBy('subject'); + + $this->assertCount(4, $result); + $this->assertEquals(1, $result['A1']['MyFieldsToAlter.TestOptions']); + $this->assertEquals(2, $result['A2']['MyFieldsToAlter.TestOptions']); + $this->assertEquals('One', $result['A1']['MyFieldsToAlter.TestOptions:label']); + $this->assertEquals('Two', $result['A2']['MyFieldsToAlter.TestOptions:label']); + $this->assertTrue(empty($result['A3']['MyFieldsToAlter.TestOptions'])); + $this->assertTrue(empty($result['A4']['MyFieldsToAlter.TestOptions'])); + + // Change field to multiselect + CustomField::update(FALSE) + ->addWhere('id', '=', $customGroup['field1']['id']) + ->addValue('serialize', TRUE) + ->execute(); + + $result = Activity::get(FALSE) + ->addWhere('MyFieldsToAlter.TestText', 'IS NOT NULL') + ->addSelect('custom.*', 'subject', 'MyFieldsToAlter.TestOptions:label') + ->execute()->indexBy('subject'); + + $this->assertEquals([1], $result['A1']['MyFieldsToAlter.TestOptions']); + $this->assertEquals([2], $result['A2']['MyFieldsToAlter.TestOptions']); + $this->assertEquals(['One'], $result['A1']['MyFieldsToAlter.TestOptions:label']); + $this->assertTrue(empty($result['A3']['MyFieldsToAlter.TestOptions'])); + $this->assertTrue(empty($result['A4']['MyFieldsToAlter.TestOptions'])); + + // Change back to single-select + CustomField::update(FALSE) + ->addWhere('id', '=', $customGroup['field1']['id']) + ->addValue('serialize', FALSE) + ->execute(); + + $result = Activity::get(FALSE) + ->addWhere('MyFieldsToAlter.TestText', 'IS NOT NULL') + ->addSelect('custom.*', 'subject', 'MyFieldsToAlter.TestOptions:label') + ->execute()->indexBy('subject'); + + $this->assertCount(4, $result); + $this->assertEquals(1, $result['A1']['MyFieldsToAlter.TestOptions']); + $this->assertEquals(2, $result['A2']['MyFieldsToAlter.TestOptions']); + $this->assertEquals('One', $result['A1']['MyFieldsToAlter.TestOptions:label']); + $this->assertEquals('Two', $result['A2']['MyFieldsToAlter.TestOptions:label']); + $this->assertTrue(empty($result['A3']['MyFieldsToAlter.TestOptions'])); + $this->assertTrue(empty($result['A4']['MyFieldsToAlter.TestOptions'])); + } + +} -- 2.25.1