From 75c4fcec37be383d64c0c1d3b64329f2563a355a Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 18 Apr 2019 13:52:25 -0400 Subject: [PATCH 1/3] Fix creating Formatting fields in api and UI --- CRM/Core/BAO/UFField.php | 1 + api/v3/utils.php | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/CRM/Core/BAO/UFField.php b/CRM/Core/BAO/UFField.php index 9717f8ff3505..546476b4de95 100644 --- a/CRM/Core/BAO/UFField.php +++ b/CRM/Core/BAO/UFField.php @@ -1086,6 +1086,7 @@ public static function getAvailableFieldsFlat($force = FALSE) { */ public static function getAvailableFieldTitles() { $fields = self::getAvailableFieldsFlat(); + $fields['formatting'] = ['title' => ts('Formatting')]; return CRM_Utils_Array::collect('title', $fields); } diff --git a/api/v3/utils.php b/api/v3/utils.php index bac0826f5cd3..1e5d3836401b 100644 --- a/api/v3/utils.php +++ b/api/v3/utils.php @@ -2338,6 +2338,11 @@ function _civicrm_api3_api_match_pseudoconstant_value(&$value, $options, $fieldN return; } + // Hack for Profile formatting fields + if ($fieldName === 'field_name' && (strpos($value, 'formatting') === 0)) { + return; + } + // Translate value into key // Cast $value to string to avoid a bug in array_search $newValue = array_search((string) $value, $options); From aa9732b98759a2907bcbe1002bb2f24b94748be5 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 18 Apr 2019 14:00:51 -0400 Subject: [PATCH 2/3] (REF) Standardize UFField create function --- CRM/Core/BAO/UFField.php | 107 ++++++++++++++++++--------------------- CRM/UF/Form/Field.php | 9 +--- js/model/crm.designer.js | 4 +- 3 files changed, 51 insertions(+), 69 deletions(-) diff --git a/CRM/Core/BAO/UFField.php b/CRM/Core/BAO/UFField.php index 546476b4de95..4549d78d3b0a 100644 --- a/CRM/Core/BAO/UFField.php +++ b/CRM/Core/BAO/UFField.php @@ -52,85 +52,74 @@ class CRM_Core_BAO_UFField extends CRM_Core_DAO_UFField { * @return \CRM_Core_BAO_UFField * @throws \API_Exception */ - public static function create(&$params) { - // CRM-14756: kind of a hack-ish fix. If the user gives the id, uf_group_id is retrieved and then set. - if (isset($params['id'])) { - $groupId = civicrm_api3('UFField', 'getvalue', [ - 'return' => 'uf_group_id', - 'id' => $params['id'], - ]); - } - else { - $groupId = CRM_Utils_Array::value('uf_group_id', $params); - } + public static function create($params) { + $id = CRM_Utils_Array::value('id', $params); - $field_name = CRM_Utils_Array::value('field_name', $params); + // Merge in data from existing field + if (!empty($id)) { + $UFField = new CRM_Core_BAO_UFField(); + $UFField->id = $params['id']; + if ($UFField->find(TRUE)) { + $defaults = $UFField->toArray(); + // This will be calculated based on field name + unset($defaults['field_type']); + $params += $defaults; + } + else { + throw new API_Exception("UFFIeld id {$params['id']} not found."); + } + } - if (strpos($field_name, 'formatting') !== 0 && !CRM_Core_BAO_UFField::isValidFieldName($field_name)) { + // Validate field_name + if (strpos($params['field_name'], 'formatting') !== 0 && !CRM_Core_BAO_UFField::isValidFieldName($params['field_name'])) { throw new API_Exception('The field_name is not valid'); } - if (!(CRM_Utils_Array::value('group_id', $params))) { - $params['group_id'] = $groupId; + // Supply default label if not set + if (empty($id) && !isset($params['label'])) { + $params['label'] = self::getAvailableFieldTitles()[$params['field_name']]; } - $fieldId = CRM_Utils_Array::value('id', $params); - if (!empty($fieldId)) { - $UFField = new CRM_Core_BAO_UFField(); - $UFField->id = $fieldId; - if ($UFField->find(TRUE)) { - if (!(CRM_Utils_Array::value('group_id', $params))) { - // this copied here from previous api function - not sure if required - $params['group_id'] = $UFField->uf_group_id; - } - } - else { - throw new API_Exception("there is no field for this fieldId"); - } + // Supply field_type if not set + if (empty($params['field_type']) && strpos($params['field_name'], 'formatting') !== 0) { + $params['field_type'] = CRM_Utils_Array::pathGet(self::getAvailableFieldsFlat(), [$params['field_name'], 'field_type']); + } + elseif (empty($params['field_type'])) { + $params['field_type'] = 'Formatting'; + } + + // Generate unique name for formatting fields + if ($params['field_name'] === 'formatting') { + $params['field_name'] = 'formatting_' . substr(uniqid(), -4); } - $params['uf_group_id'] = $params['group_id']; - if (CRM_Core_BAO_UFField::duplicateField($params)) { + if (self::duplicateField($params)) { throw new API_Exception("The field was not added. It already exists in this profile."); } - // @todo fix BAO to be less weird. - $field_type = CRM_Utils_Array::value('field_type', $params); - $location_type_id = CRM_Utils_Array::value('location_type_id', $params, CRM_Utils_Array::value('website_type_id', $params)); - $phone_type = CRM_Utils_Array::value('phone_type_id', $params, CRM_Utils_Array::value('phone_type', $params)); - $params['field_name'] = [$field_type, $field_name, $location_type_id, $phone_type]; //@todo why is this even optional? Surely weight should just be 'managed' ?? if (CRM_Utils_Array::value('option.autoweight', $params, TRUE)) { $params['weight'] = CRM_Core_BAO_UFField::autoWeight($params); } - // set values for uf field properties and save + + // Set values for uf field properties and save $ufField = new CRM_Core_DAO_UFField(); $ufField->copyValues($params); - $ufField->field_type = $params['field_name'][0]; - $ufField->field_name = $params['field_name'][1]; - //should not set location type id for Primary - $locationTypeId = NULL; - if ($params['field_name'][1] == 'url') { - $ufField->website_type_id = CRM_Utils_Array::value(2, $params['field_name']); + if ($params['field_name'] == 'url') { + $ufField->location_type_id = 'null'; } else { - $locationTypeId = CRM_Utils_Array::value(2, $params['field_name']); - $ufField->website_type_id = NULL; - } - if ($locationTypeId) { - $ufField->location_type_id = $locationTypeId; + $ufField->website_type_id = 'null'; } - else { - $ufField->location_type_id = 'null'; + if (!strstr($params['field_name'], 'phone')) { + $ufField->phone_type_id = 'null'; } - $ufField->phone_type_id = CRM_Utils_Array::value(3, $params['field_name'], 'NULL'); - $ufField->save(); - $fieldsType = CRM_Core_BAO_UFGroup::calculateGroupType($groupId, TRUE); - CRM_Core_BAO_UFGroup::updateGroupTypes($groupId, $fieldsType); + $fieldsType = CRM_Core_BAO_UFGroup::calculateGroupType($ufField->uf_group_id, TRUE); + CRM_Core_BAO_UFGroup::updateGroupTypes($ufField->uf_group_id, $fieldsType); civicrm_api3('profile', 'getfields', ['cache_clear' => TRUE]); return $ufField; @@ -204,8 +193,8 @@ public static function del($id) { public static function duplicateField($params) { $ufField = new CRM_Core_DAO_UFField(); $ufField->uf_group_id = CRM_Utils_Array::value('uf_group_id', $params); - $ufField->field_type = $params['field_type']; - $ufField->field_name = $params['field_name']; + $ufField->field_type = CRM_Utils_Array::value('field_type', $params); + $ufField->field_name = CRM_Utils_Array::value('field_name', $params); $ufField->website_type_id = CRM_Utils_Array::value('website_type_id', $params); if (is_null(CRM_Utils_Array::value('location_type_id', $params, ''))) { // primary location type have NULL value in DB @@ -220,7 +209,7 @@ public static function duplicateField($params) { $ufField->whereAdd("id <> " . $params['id']); } - return ($ufField->find(TRUE) ? 1 : 0); + return (bool) $ufField->find(TRUE); } /** @@ -280,10 +269,10 @@ public static function autoWeight($params) { // fix for CRM-316 $oldWeight = NULL; - if (!empty($params['field_id'])) { - $oldWeight = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_UFField', $params['field_id'], 'weight', 'id'); + if (!empty($params['field_id']) || !empty($params['id'])) { + $oldWeight = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_UFField', !empty($params['id']) ? $params['id'] : $params['field_id'], 'weight', 'id'); } - $fieldValues = ['uf_group_id' => $params['group_id']]; + $fieldValues = ['uf_group_id' => !empty($params['uf_group_id']) ? $params['uf_group_id'] : $params['group_id']]; return CRM_Utils_Weight::updateOtherWeights('CRM_Core_DAO_UFField', $oldWeight, CRM_Utils_Array::value('weight', $params, 0), $fieldValues); } diff --git a/CRM/UF/Form/Field.php b/CRM/UF/Form/Field.php index 9086dfc0b17f..8640d593f07e 100644 --- a/CRM/UF/Form/Field.php +++ b/CRM/UF/Form/Field.php @@ -531,13 +531,8 @@ public function postProcess() { $name = $this->_selectFields[$params['field_name'][1]]; } - //Hack for Formatting Field Name - if ($params['field_name'][0] == 'Formatting') { - $fieldName = 'formatting_' . rand(1000, 9999); - } - else { - $fieldName = $params['field_name'][1]; - } + // If field_name is missing, it's formatting + $fieldName = CRM_Utils_Array::value(1, $params['field_name'], 'formatting'); //check for duplicate fields $apiFormattedParams = $params; diff --git a/js/model/crm.designer.js b/js/model/crm.designer.js index 752cf86415c8..8ca76604eb68 100644 --- a/js/model/crm.designer.js +++ b/js/model/crm.designer.js @@ -70,9 +70,7 @@ label: this.getLabel(), entity_name: this.get('entityName'), field_type: this.getFieldSchema().civiFieldType, - // For some reason the 'formatting' field gets a random number appended in core so we mimic that here. - // TODO: Why? - field_name: this.get('fieldName') == 'formatting' ? 'formatting_' + (Math.floor(Math.random() * 8999) + 1000) : this.get('fieldName') + field_name: this.get('fieldName') }); return model; } From 79da2aaccce219d5aa3b48945bf67b9d53f8e758 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Mon, 22 Apr 2019 08:22:13 -0400 Subject: [PATCH 3/3] Add UFField to syntaxConformanceTest --- .../phpunit/api/v3/SyntaxConformanceTest.php | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/phpunit/api/v3/SyntaxConformanceTest.php b/tests/phpunit/api/v3/SyntaxConformanceTest.php index 64dbdcda3f65..e22e8eea0c77 100644 --- a/tests/phpunit/api/v3/SyntaxConformanceTest.php +++ b/tests/phpunit/api/v3/SyntaxConformanceTest.php @@ -498,7 +498,6 @@ public static function toBeSkipped_updatesingle($sequential = FALSE) { 'Profile', 'CustomValue', 'UFJoin', - 'UFField', 'Relationship', 'RelationshipType', 'Note', @@ -736,6 +735,27 @@ public function getKnownUnworkablesUpdateSingle($entity, $key) { 'ignore_severity', ), ), + 'UFField' => array( + 'cant_update' => array( + // These fields get auto-adjusted by the BAO prior to saving + 'weight', + 'location_type_id', + 'phone_type_id', + 'website_type_id', + // Not a real field + 'option.autoweight', + ), + 'break_return' => array( + // These fields get auto-adjusted by the BAO prior to saving + 'weight', + 'field_type', + 'location_type_id', + 'phone_type_id', + 'website_type_id', + // Not a real field + 'option.autoweight', + ), + ), 'JobLog' => array( // For better or worse triggers override. 'break_return' => ['run_time'],