diff --git a/CRM/Admin/Form/LocationType.php b/CRM/Admin/Form/LocationType.php index e9a687b17c73..8ec8801282f1 100644 --- a/CRM/Admin/Form/LocationType.php +++ b/CRM/Admin/Form/LocationType.php @@ -65,8 +65,8 @@ public function buildQuickForm() { $this->add('text', 'description', ts('Description'), CRM_Core_DAO::getAttribute('CRM_Core_DAO_LocationType', 'description')); - $this->add('checkbox', 'is_active', ts('Enabled?')); - $this->add('checkbox', 'is_default', ts('Default?')); + $this->add('advcheckbox', 'is_active', ts('Enabled?')); + $this->add('advcheckbox', 'is_default', ts('Default?')); if ($this->_action & CRM_Core_Action::UPDATE) { if (CRM_Core_DAO::getFieldValue('CRM_Core_DAO_LocationType', $this->_id, 'is_reserved')) { @@ -82,38 +82,19 @@ public function buildQuickForm() { * Process the form submission. */ public function postProcess() { - CRM_Utils_System::flushCache(); - + // Delete action if ($this->_action & CRM_Core_Action::DELETE) { CRM_Core_BAO_LocationType::deleteRecord(['id' => $this->_id]); CRM_Core_Session::setStatus(ts('Selected Location type has been deleted.'), ts('Record Deleted'), 'success'); return; } - - // store the submitted values in an array + // Create or update actions $params = $this->exportValues(); - $params['is_active'] = CRM_Utils_Array::value('is_active', $params, FALSE); - $params['is_default'] = CRM_Utils_Array::value('is_default', $params, FALSE); - - // action is taken depending upon the mode - $locationType = new CRM_Core_DAO_LocationType(); - $locationType->name = $params['name']; - $locationType->display_name = $params['display_name']; - $locationType->vcard_name = $params['vcard_name']; - $locationType->description = $params['description']; - $locationType->is_active = $params['is_active']; - $locationType->is_default = $params['is_default']; - - if ($params['is_default']) { - $query = "UPDATE civicrm_location_type SET is_default = 0"; - CRM_Core_DAO::executeQuery($query); - } - if ($this->_action & CRM_Core_Action::UPDATE) { - $locationType->id = $this->_id; + $params['id'] = $this->_id; } - $locationType->save(); + $locationType = CRM_Core_BAO_LocationType::writeRecord($params); CRM_Core_Session::setStatus(ts("The location type '%1' has been saved.", [1 => $locationType->name] diff --git a/CRM/Admin/Page/LocationType.php b/CRM/Admin/Page/LocationType.php index 17896dae90ee..2c6093a553a9 100644 --- a/CRM/Admin/Page/LocationType.php +++ b/CRM/Admin/Page/LocationType.php @@ -76,7 +76,7 @@ protected function getRows($sort, $action, array $links): array { foreach ($rows as &$row) { // prevent smarty notices. foreach (['is_default', 'class', 'vcard_name'] as $expectedField) { - if (!isset($row['is_default'])) { + if (!isset($row[$expectedField])) { $row[$expectedField] = NULL; } } diff --git a/CRM/Core/BAO/LocationType.php b/CRM/Core/BAO/LocationType.php index 0de77f2a66d3..55290124f647 100644 --- a/CRM/Core/BAO/LocationType.php +++ b/CRM/Core/BAO/LocationType.php @@ -80,27 +80,13 @@ public static function getBilling() { * Add a Location Type. * * @param array $params - * Reference array contains the values submitted by the form. * - * - * @return object + * @deprecated + * @return CRM_Core_DAO_LocationType */ public static function create(&$params) { - if (empty($params['id'])) { - $params['is_active'] = CRM_Utils_Array::value('is_active', $params, FALSE); - $params['is_default'] = CRM_Utils_Array::value('is_default', $params, FALSE); - $params['is_reserved'] = CRM_Utils_Array::value('is_reserved', $params, FALSE); - } - - $locationType = new CRM_Core_DAO_LocationType(); - $locationType->copyValues($params); - if (!empty($params['is_default'])) { - $query = "UPDATE civicrm_location_type SET is_default = 0"; - CRM_Core_DAO::executeQuery($query); - } - - $locationType->save(); - return $locationType; + CRM_Core_Error::deprecatedFunctionWarning('writeRecord'); + return self::writeRecord($params); } /** @@ -129,6 +115,14 @@ public static function self_hook_civicrm_pre(\Civi\Core\Event\PreEvent $event) { ]); } } + elseif (in_array($event->action, ['create', 'edit'])) { + if (!empty($event->params['is_default'])) { + $query = "UPDATE civicrm_location_type SET is_default = 0"; + CRM_Core_DAO::executeQuery($query); + } + } + // Todo: This was moved from CRM_Admin_Form_LocationType::postProcess but is probably unnecessarily broad. + CRM_Utils_System::flushCache(); } } diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php index bfffbc2f595a..d39791f0c77f 100644 --- a/CRM/Core/DAO.php +++ b/CRM/Core/DAO.php @@ -3315,7 +3315,8 @@ private function makeNameFromLabel(): void { // No unique index on "name", do nothing return; } - $label = $this->label ?? $this->title ?? NULL; + $labelField = $this::$_labelField; + $label = $this->$labelField ?? NULL; if (!$label && $label !== '0') { // No label supplied, do nothing return; diff --git a/CRM/Core/DAO/LocationType.php b/CRM/Core/DAO/LocationType.php index 0c3db7375b77..feb3ef478217 100644 --- a/CRM/Core/DAO/LocationType.php +++ b/CRM/Core/DAO/LocationType.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/Core/LocationType.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:035ebc0bafa88ffbc21d969b983399ef) + * (GenCodeChecksum:64f83ce54ddb0f9cc115a807e773f9cf) */ /** @@ -23,6 +23,13 @@ class CRM_Core_DAO_LocationType extends CRM_Core_DAO { */ public static $_tableName = 'civicrm_location_type'; + /** + * Field to show when displaying a record. + * + * @var string + */ + public static $_labelField = 'display_name'; + /** * Should CiviCRM log any modifications to this table in the civicrm_log table. * @@ -53,7 +60,7 @@ class CRM_Core_DAO_LocationType extends CRM_Core_DAO { /** * Location Type Name. * - * @var string|null + * @var string * (SQL type: varchar(64)) * Note that values will be retrieved from the database as a string. */ @@ -62,7 +69,7 @@ class CRM_Core_DAO_LocationType extends CRM_Core_DAO { /** * Location Type Display Name. * - * @var string|null + * @var string * (SQL type: varchar(64)) * Note that values will be retrieved from the database as a string. */ @@ -89,7 +96,7 @@ class CRM_Core_DAO_LocationType extends CRM_Core_DAO { /** * Is this location type a predefined system location? * - * @var bool|string|null + * @var bool|string * (SQL type: tinyint) * Note that values will be retrieved from the database as a string. */ @@ -98,7 +105,7 @@ class CRM_Core_DAO_LocationType extends CRM_Core_DAO { /** * Is this property active? * - * @var bool|string|null + * @var bool|string * (SQL type: tinyint) * Note that values will be retrieved from the database as a string. */ @@ -107,7 +114,7 @@ class CRM_Core_DAO_LocationType extends CRM_Core_DAO { /** * Is this location type the default? * - * @var bool|string|null + * @var bool|string * (SQL type: tinyint) * Note that values will be retrieved from the database as a string. */ @@ -167,6 +174,7 @@ public static function &fields() { 'type' => CRM_Utils_Type::T_STRING, 'title' => ts('Location Type'), 'description' => ts('Location Type Name.'), + 'required' => TRUE, 'maxlength' => 64, 'size' => CRM_Utils_Type::BIG, 'usage' => [ @@ -180,6 +188,9 @@ public static function &fields() { 'entity' => 'LocationType', 'bao' => 'CRM_Core_BAO_LocationType', 'localizable' => 0, + 'html' => [ + 'type' => 'Text', + ], 'add' => '1.1', ], 'display_name' => [ @@ -187,6 +198,7 @@ public static function &fields() { 'type' => CRM_Utils_Type::T_STRING, 'title' => ts('Display Name'), 'description' => ts('Location Type Display Name.'), + 'required' => TRUE, 'maxlength' => 64, 'size' => CRM_Utils_Type::BIG, 'usage' => [ @@ -200,6 +212,9 @@ public static function &fields() { 'entity' => 'LocationType', 'bao' => 'CRM_Core_BAO_LocationType', 'localizable' => 1, + 'html' => [ + 'type' => 'Text', + ], 'add' => '4.1', ], 'vcard_name' => [ @@ -220,6 +235,9 @@ public static function &fields() { 'entity' => 'LocationType', 'bao' => 'CRM_Core_BAO_LocationType', 'localizable' => 0, + 'html' => [ + 'type' => 'Text', + ], 'add' => '1.1', ], 'description' => [ @@ -241,7 +259,7 @@ public static function &fields() { 'bao' => 'CRM_Core_BAO_LocationType', 'localizable' => 0, 'html' => [ - 'label' => ts("Description"), + 'type' => 'Text', ], 'add' => '1.1', ], @@ -250,6 +268,7 @@ public static function &fields() { 'type' => CRM_Utils_Type::T_BOOLEAN, 'title' => ts('Location Type is Reserved?'), 'description' => ts('Is this location type a predefined system location?'), + 'required' => TRUE, 'usage' => [ 'import' => FALSE, 'export' => FALSE, @@ -257,10 +276,15 @@ public static function &fields() { 'token' => FALSE, ], 'where' => 'civicrm_location_type.is_reserved', + 'default' => '0', 'table_name' => 'civicrm_location_type', 'entity' => 'LocationType', 'bao' => 'CRM_Core_BAO_LocationType', 'localizable' => 0, + 'html' => [ + 'type' => 'CheckBox', + 'label' => ts("Reserved"), + ], 'add' => '1.1', ], 'is_active' => [ @@ -268,6 +292,7 @@ public static function &fields() { 'type' => CRM_Utils_Type::T_BOOLEAN, 'title' => ts('Location Type is Active?'), 'description' => ts('Is this property active?'), + 'required' => TRUE, 'usage' => [ 'import' => FALSE, 'export' => FALSE, @@ -275,6 +300,7 @@ public static function &fields() { 'token' => FALSE, ], 'where' => 'civicrm_location_type.is_active', + 'default' => '1', 'table_name' => 'civicrm_location_type', 'entity' => 'LocationType', 'bao' => 'CRM_Core_BAO_LocationType', @@ -290,6 +316,7 @@ public static function &fields() { 'type' => CRM_Utils_Type::T_BOOLEAN, 'title' => ts('Default Location Type?'), 'description' => ts('Is this location type the default?'), + 'required' => TRUE, 'usage' => [ 'import' => FALSE, 'export' => FALSE, @@ -297,6 +324,7 @@ public static function &fields() { 'token' => FALSE, ], 'where' => 'civicrm_location_type.is_default', + 'default' => '0', 'table_name' => 'civicrm_location_type', 'entity' => 'LocationType', 'bao' => 'CRM_Core_BAO_LocationType', diff --git a/CRM/Core/I18n/SchemaStructure.php b/CRM/Core/I18n/SchemaStructure.php index d79f2493dd64..cd0b0939f5ea 100644 --- a/CRM/Core/I18n/SchemaStructure.php +++ b/CRM/Core/I18n/SchemaStructure.php @@ -30,7 +30,7 @@ public static function &columns() { if (!$result) { $result = [ 'civicrm_location_type' => [ - 'display_name' => "varchar(64) COMMENT 'Location Type Display Name.'", + 'display_name' => "varchar(64) NOT NULL COMMENT 'Location Type Display Name.'", ], 'civicrm_option_group' => [ 'title' => "varchar(255) COMMENT 'Option Group title.'", @@ -266,6 +266,7 @@ public static function &widgets() { 'civicrm_location_type' => [ 'display_name' => [ 'type' => "Text", + 'required' => "true", ], ], 'civicrm_option_group' => [ diff --git a/CRM/Upgrade/Incremental/php/FiveSixtyFive.php b/CRM/Upgrade/Incremental/php/FiveSixtyFive.php index 0abb94df6742..508c6dc52a7c 100644 --- a/CRM/Upgrade/Incremental/php/FiveSixtyFive.php +++ b/CRM/Upgrade/Incremental/php/FiveSixtyFive.php @@ -29,7 +29,12 @@ class CRM_Upgrade_Incremental_php_FiveSixtyFive extends CRM_Upgrade_Incremental_ */ public function upgrade_5_65_alpha1($rev): void { $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev); - // These 3 should run after the sql file. + // These should run after the sql file. + $this->addTask('Make LocationType.name required', 'alterColumn', 'civicrm_location_type', 'name', "varchar(64) NOT NULL COMMENT 'Location Type Name.'"); + $this->addTask('Make LocationType.display_name required', 'alterColumn', 'civicrm_location_type', 'display_name', "varchar(64) NOT NULL COMMENT 'Location Type Display Name.'", TRUE); + $this->addTask('Make LocationType.is_reserved required', 'alterColumn', 'civicrm_location_type', 'is_reserved', "tinyint NOT NULL DEFAULT 0 COMMENT 'Is this location type a predefined system location?'"); + $this->addTask('Make LocationType.is_active required', 'alterColumn', 'civicrm_location_type', 'is_active', "tinyint NOT NULL DEFAULT 1 COMMENT 'Is this property active?'"); + $this->addTask('Make LocationType.is_default required', 'alterColumn', 'civicrm_location_type', 'is_default', "tinyint NOT NULL DEFAULT 0 COMMENT 'Is this location type the default?'"); $this->addTask('Make Group.name required', 'alterColumn', 'civicrm_group', 'name', "varchar(255) NOT NULL COMMENT 'Internal name of Group.'"); $this->addTask('Make Group.title required', 'alterColumn', 'civicrm_group', 'title', "varchar(255) NOT NULL COMMENT 'Alternative public title for this Group.'", TRUE); $this->addTask('Make Group.frontend_title required', 'alterColumn', 'civicrm_group', 'frontend_title', "varchar(255) NOT NULL COMMENT 'Alternative public description of the group.'", TRUE); diff --git a/CRM/Upgrade/Incremental/sql/5.65.alpha1.mysql.tpl b/CRM/Upgrade/Incremental/sql/5.65.alpha1.mysql.tpl index b0bb6bd1f420..ec037c2896a8 100644 --- a/CRM/Upgrade/Incremental/sql/5.65.alpha1.mysql.tpl +++ b/CRM/Upgrade/Incremental/sql/5.65.alpha1.mysql.tpl @@ -36,3 +36,9 @@ body_text = REPLACE(body_text, '{welcome.group}', '{group.frontend_title}'), subject = REPLACE(subject, '{welcome.group}', '{group.frontend_title}') WHERE component_type = 'Welcome'; {/literal} + +UPDATE `civicrm_location_type` SET `is_reserved` = 0 WHERE `is_reserved` IS NULL; +UPDATE `civicrm_location_type` SET `is_active` = 0 WHERE `is_active` IS NULL; +UPDATE `civicrm_location_type` SET `is_default` = 0 WHERE `is_default` IS NULL; +UPDATE `civicrm_location_type` SET `name` = CONCAT('location_', id) WHERE `name` IS NULL; +UPDATE `civicrm_location_type` SET {localize field=display_name}`display_name` = COALESCE(`display_name`, `name`){/localize}; diff --git a/sql/civicrm_data/civicrm_location_type.sqldata.php b/sql/civicrm_data/civicrm_location_type.sqldata.php index 0d611080f08d..8c455483b7cb 100644 --- a/sql/civicrm_data/civicrm_location_type.sqldata.php +++ b/sql/civicrm_data/civicrm_location_type.sqldata.php @@ -2,35 +2,31 @@ return CRM_Core_CodeGen_SqlData::create('civicrm_location_type') ->addValues([ - // CRM-9120 for legacy reasons we are continuing to translate the 'name', but this - // field is used mainly as an ID, and display_name will be shown to the user, but - // we have not yet finished modifying all places where the 'name' is shown. [ - 'name' => ts('Home'), + 'name' => 'Home', 'display_name' => ts('Home'), 'vcard_name' => 'HOME', 'description' => ts('Place of residence'), 'is_default' => 1, ], [ - 'name' => ts('Work'), + 'name' => 'Work', 'display_name' => ts('Work'), 'vcard_name' => 'WORK', 'description' => ts('Work location'), ], [ - 'name' => ts('Main'), + 'name' => 'Main', 'display_name' => ts('Main'), 'vcard_name' => NULL, 'description' => ts('Main office location'), ], [ - 'name' => ts('Other'), + 'name' => 'Other', 'display_name' => ts('Other'), 'vcard_name' => NULL, 'description' => ts('Other location'), ], - // -- the following location must stay with the untranslated Billing name, CRM-2064 [ 'name' => 'Billing', 'display_name' => ts('Billing'), @@ -41,7 +37,6 @@ ]) ->addDefaults([ 'is_active' => 1, - 'is_default' => NULL, + 'is_default' => 0, 'is_reserved' => 0, - // FIXME: Doesn't 0 make more sense than NULL? ]); diff --git a/sql/civicrm_generated.mysql b/sql/civicrm_generated.mysql index 9448bab69684..cea2e0af981f 100644 --- a/sql/civicrm_generated.mysql +++ b/sql/civicrm_generated.mysql @@ -4274,10 +4274,10 @@ LOCK TABLES `civicrm_location_type` WRITE; /*!40000 ALTER TABLE `civicrm_location_type` DISABLE KEYS */; INSERT INTO `civicrm_location_type` (`id`, `name`, `display_name`, `vcard_name`, `description`, `is_reserved`, `is_active`, `is_default`) VALUES (1,'Home','Home','HOME','Place of residence',0,1,1), - (2,'Work','Work','WORK','Work location',0,1,NULL), - (3,'Main','Main',NULL,'Main office location',0,1,NULL), - (4,'Other','Other',NULL,'Other location',0,1,NULL), - (5,'Billing','Billing',NULL,'Billing Address location',1,1,NULL); + (2,'Work','Work','WORK','Work location',0,1,0), + (3,'Main','Main',NULL,'Main office location',0,1,0), + (4,'Other','Other',NULL,'Other location',0,1,0), + (5,'Billing','Billing',NULL,'Billing Address location',1,1,0); /*!40000 ALTER TABLE `civicrm_location_type` ENABLE KEYS */; UNLOCK TABLES; diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 2240371b0d42..66197d36aa83 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -1117,16 +1117,13 @@ public function locationAdd($contactID) { public function locationTypeCreate(array $params = []): int { $params = array_merge([ 'name' => 'New Location Type', + 'display_name' => 'New Location Type', 'vcard_name' => 'New Location Type', 'description' => 'Location Type for Delete', 'is_active' => 1, ], $params); - $locationType = new CRM_Core_DAO_LocationType(); - $locationType->copyValues($params); - $locationType->save(); - // clear getfields cache - CRM_Core_PseudoConstant::flush(); + $locationType = CRM_Core_BAO_LocationType::writeRecord($params); $this->callAPISuccess('Phone', 'getfields', ['version' => 3, 'cache_clear' => 1]); return $locationType->id; } diff --git a/xml/schema/Core/LocationType.xml b/xml/schema/Core/LocationType.xml index b6e4c9bf616c..1ee4d1705336 100644 --- a/xml/schema/Core/LocationType.xml +++ b/xml/schema/Core/LocationType.xml @@ -11,6 +11,7 @@ civicrm/admin/locationType/edit?action=update&id=[id]&reset=1 civicrm/admin/locationType/edit?action=delete&id=[id]&reset=1 + display_name id Location Type ID @@ -31,6 +32,10 @@ Location Type varchar 64 + + Text + + true Location Type Name. 1.1 @@ -39,6 +44,10 @@ Display Name varchar 64 + + Text + + true Location Type Display Name. true 4.1 @@ -47,6 +56,9 @@ vcard_name vCard Location Type varchar + + Text + 64 vCard Location Type Name. 1.1 @@ -55,6 +67,9 @@ description varchar 255 + + Text + Location Type Description. @@ -65,6 +80,12 @@ is_reserved Location Type is Reserved? boolean + 0 + true + + CheckBox + + Is this location type a predefined system location? 1.1 @@ -72,6 +93,8 @@ is_active Location Type is Active? boolean + 1 + true Is this property active? CheckBox @@ -83,6 +106,8 @@ is_default Default Location Type? boolean + 0 + true CheckBox