From e64435f1646d80e760e688f961c139f69ae2da14 Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 15 Aug 2020 11:04:31 +1200 Subject: [PATCH] Improve consistency of metadata type declarations --- CRM/Contact/BAO/Contact.php | 4 ++ CRM/Export/BAO/ExportProcessor.php | 65 +++++++++------------ tests/phpunit/CRM/Export/BAO/ExportTest.php | 36 ++++++------ 3 files changed, 50 insertions(+), 55 deletions(-) diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php index 29c82bd900..96e847a120 100644 --- a/CRM/Contact/BAO/Contact.php +++ b/CRM/Contact/BAO/Contact.php @@ -1543,14 +1543,17 @@ WHERE civicrm_contact.id = " . CRM_Utils_Type::escape($id, 'Integer'); [ 'name' => 'organization_name', 'title' => ts('Current Employer'), + 'type' => CRM_Utils_Type::T_STRING, ], ]); + // This probably would be added anyway by appendPseudoConstantsToFields. $locationType = [ 'location_type' => [ 'name' => 'location_type', 'where' => 'civicrm_location_type.name', 'title' => ts('Location Type'), + 'type' => CRM_Utils_Type::T_STRING, ], ]; @@ -1559,6 +1562,7 @@ WHERE civicrm_contact.id = " . CRM_Utils_Type::escape($id, 'Integer'); 'name' => 'im_provider', 'where' => 'civicrm_im.provider_id', 'title' => ts('IM Provider'), + 'type' => CRM_Utils_Type::T_STRING, ], ]; diff --git a/CRM/Export/BAO/ExportProcessor.php b/CRM/Export/BAO/ExportProcessor.php index c1231fecb7..7c2fbb8592 100644 --- a/CRM/Export/BAO/ExportProcessor.php +++ b/CRM/Export/BAO/ExportProcessor.php @@ -639,6 +639,9 @@ class CRM_Export_BAO_ExportProcessor { $queryFields['world_region']['context'] = 'country'; $queryFields['state_province']['context'] = 'province'; $queryFields['contact_id'] = ['title' => ts('Contact ID'), 'type' => CRM_Utils_Type::T_INT]; + $queryFields['tags']['type'] = CRM_Utils_Type::T_LONGTEXT; + $queryFields['groups']['type'] = CRM_Utils_Type::T_LONGTEXT; + $queryFields['notes']['type'] = CRM_Utils_Type::T_LONGTEXT; // Set the label to gender for gender_id as we it's ... magic (not in a good way). // In other places the query object offers e.g contribution_status & contribution_status_id $queryFields['gender_id']['title'] = ts('Gender'); @@ -803,7 +806,7 @@ class CRM_Export_BAO_ExportProcessor { // CRM-13982 - check if is deleted foreach ($params as $value) { - if ($value[0] == 'contact_is_deleted') { + if ($value[0] === 'contact_is_deleted') { unset($whereClauses['trash_clause']); } } @@ -825,10 +828,10 @@ class CRM_Export_BAO_ExportProcessor { } if (empty($where)) { - $where = "WHERE " . implode(' AND ', $whereClauses); + $where = 'WHERE ' . implode(' AND ', $whereClauses); } else { - $where .= " AND " . implode(' AND ', $whereClauses); + $where .= ' AND ' . implode(' AND ', $whereClauses); } $groupBy = $this->getGroupBy($query); @@ -1431,21 +1434,20 @@ class CRM_Export_BAO_ExportProcessor { public function getSqlColumnDefinition($fieldName, $columnName) { // early exit for master_id, CRM-12100 - // in the DB it is an ID, but in the export, we retrive the display_name of the master record - // also for current_employer, CRM-16939 - if ($columnName == 'master_id' || $columnName == 'current_employer') { + // in the DB it is an ID, but in the export, we retrieve the display_name of the master record + if ($columnName === 'master_id') { return "`$fieldName` varchar(128)"; } $queryFields = $this->getQueryFields(); - // @todo remove the enotice avoidance here, ensure all columns are declared. + // @todo remove the e-notice avoidance here, ensure all columns are declared. // tests will fail on the enotices until they all are & then all the 'else' // below can go. $fieldSpec = $queryFields[$columnName] ?? []; - + $type = $fieldSpec['type'] ?? ($fieldSpec['data_type'] ?? ''); // set the sql columns - if (isset($fieldSpec['type'])) { - switch ($fieldSpec['type']) { + if ($type) { + switch ($type) { case CRM_Utils_Type::T_INT: case CRM_Utils_Type::T_BOOLEAN: if (in_array(CRM_Utils_Array::value('data_type', $fieldSpec), ['Country', 'StateProvince', 'ContactReference'])) { @@ -1488,39 +1490,28 @@ class CRM_Export_BAO_ExportProcessor { return "`$fieldName` text"; } else { - $changeFields = [ - 'groups', - 'tags', - 'notes', - ]; + // set the sql columns for custom data + if (isset($queryFields[$columnName]['data_type'])) { - if (in_array($fieldName, $changeFields)) { - return "`$fieldName` text"; - } - else { - // set the sql columns for custom data - if (isset($queryFields[$columnName]['data_type'])) { + switch ($queryFields[$columnName]['data_type']) { + case 'String': + // May be option labels, which could be up to 512 characters + $length = max(512, CRM_Utils_Array::value('text_length', $queryFields[$columnName])); + return "`$fieldName` varchar($length)"; - switch ($queryFields[$columnName]['data_type']) { - case 'String': - // May be option labels, which could be up to 512 characters - $length = max(512, CRM_Utils_Array::value('text_length', $queryFields[$columnName])); - return "`$fieldName` varchar($length)"; + case 'Link': + return "`$fieldName` varchar(255)"; - case 'Link': - return "`$fieldName` varchar(255)"; + case 'Memo': + return "`$fieldName` text"; - case 'Memo': - return "`$fieldName` text"; - - default: - return "`$fieldName` varchar(255)"; - } - } - else { - return "`$fieldName` text"; + default: + return "`$fieldName` varchar(255)"; } } + else { + return "`$fieldName` text"; + } } } } diff --git a/tests/phpunit/CRM/Export/BAO/ExportTest.php b/tests/phpunit/CRM/Export/BAO/ExportTest.php index b2359ed4f0..deb43c526f 100644 --- a/tests/phpunit/CRM/Export/BAO/ExportTest.php +++ b/tests/phpunit/CRM/Export/BAO/ExportTest.php @@ -1807,7 +1807,7 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { // We need some data so that we can get to the end of the export // function. Hopefully one day that won't be required to get metadata info out. // eventually aspire to call $provider->getSQLColumns straight after it - // is intiated. + // is initiated. $this->setupBaseExportData($exportMode); $this->doExportTest(['selectAll' => TRUE, 'exportMode' => $exportMode, 'ids' => [1]]); $this->assertEquals($expected, $this->processor->getSQLColumns()); @@ -1913,7 +1913,7 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { 'participant_fee_level' => '`participant_fee_level` longtext', 'participant_is_pay_later' => '`participant_is_pay_later` varchar(16)', 'participant_id' => '`participant_id` varchar(16)', - 'participant_note' => '`participant_note` text', + 'participant_note' => '`participant_note` longtext', 'participant_role_id' => '`participant_role_id` varchar(128)', 'participant_role' => '`participant_role` varchar(255)', 'participant_source' => '`participant_source` varchar(128)', @@ -2508,8 +2508,8 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { 'addressee' => '`addressee` varchar(255)', 'email_greeting' => '`email_greeting` varchar(255)', 'postal_greeting' => '`postal_greeting` varchar(255)', - 'current_employer' => '`current_employer` varchar(128)', - 'location_type' => '`location_type` text', + 'current_employer' => '`current_employer` varchar(255)', + 'location_type' => '`location_type` varchar(255)', 'address_id' => '`address_id` varchar(16)', 'street_address' => '`street_address` varchar(96)', 'street_number' => '`street_number` varchar(16)', @@ -2538,14 +2538,14 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { 'is_bulkmail' => '`is_bulkmail` varchar(16)', 'signature_text' => '`signature_text` longtext', 'signature_html' => '`signature_html` longtext', - 'im_provider' => '`im_provider` text', + 'im_provider' => '`im_provider` varchar(255)', 'im' => '`im` varchar(64)', 'openid' => '`openid` varchar(255)', 'world_region' => '`world_region` varchar(128)', 'url' => '`url` varchar(128)', - 'groups' => '`groups` text', - 'tags' => '`tags` text', - 'notes' => '`notes` text', + 'groups' => '`groups` longtext', + 'tags' => '`tags` longtext', + 'notes' => '`notes` longtext', 'phone_type' => '`phone_type` varchar(255)', ]; if (!$isContactExport) { @@ -2628,7 +2628,7 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { 'participant_status_id' => '`participant_status_id` varchar(16)', 'participant_role' => '`participant_role` varchar(255)', 'participant_role_id' => '`participant_role_id` varchar(128)', - 'participant_note' => '`participant_note` text', + 'participant_note' => '`participant_note` longtext', 'participant_register_date' => '`participant_register_date` varchar(32)', 'participant_source' => '`participant_source` varchar(128)', 'participant_fee_level' => '`participant_fee_level` longtext', @@ -2696,8 +2696,8 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { 'addressee' => '`addressee` varchar(255)', 'email_greeting' => '`email_greeting` varchar(255)', 'postal_greeting' => '`postal_greeting` varchar(255)', - 'current_employer' => '`current_employer` varchar(128)', - 'location_type' => '`location_type` text', + 'current_employer' => '`current_employer` varchar(255)', + 'location_type' => '`location_type` varchar(255)', 'street_address' => '`street_address` varchar(96)', 'street_number' => '`street_number` varchar(16)', 'street_number_suffix' => '`street_number_suffix` varchar(8)', @@ -2723,7 +2723,7 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { 'is_bulkmail' => '`is_bulkmail` varchar(16)', 'signature_text' => '`signature_text` longtext', 'signature_html' => '`signature_html` longtext', - 'im_provider' => '`im_provider` text', + 'im_provider' => '`im_provider` varchar(255)', 'im' => '`im` varchar(64)', 'openid' => '`openid` varchar(255)', 'world_region' => '`world_region` varchar(128)', @@ -2753,15 +2753,15 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { 'contribution_status' => '`contribution_status` varchar(255)', 'contribution_recur_id' => '`contribution_recur_id` varchar(16)', 'amount_level' => '`amount_level` longtext', - 'contribution_note' => '`contribution_note` text', + 'contribution_note' => '`contribution_note` longtext', 'contribution_batch' => '`contribution_batch` text', 'contribution_campaign_title' => '`contribution_campaign_title` varchar(255)', 'contribution_campaign_id' => '`contribution_campaign_id` varchar(16)', 'contribution_soft_credit_name' => '`contribution_soft_credit_name` varchar(255)', - 'contribution_soft_credit_amount' => '`contribution_soft_credit_amount` varchar(255)', + 'contribution_soft_credit_amount' => '`contribution_soft_credit_amount` varchar(32)', 'contribution_soft_credit_type' => '`contribution_soft_credit_type` varchar(255)', - 'contribution_soft_credit_contact_id' => '`contribution_soft_credit_contact_id` varchar(255)', - 'contribution_soft_credit_contribution_id' => '`contribution_soft_credit_contribution_id` varchar(255)', + 'contribution_soft_credit_contact_id' => '`contribution_soft_credit_contact_id` varchar(16)', + 'contribution_soft_credit_contribution_id' => '`contribution_soft_credit_contribution_id` varchar(16)', ]; } @@ -2781,9 +2781,9 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { 'pledge_next_pay_amount' => '`pledge_next_pay_amount` text', 'pledge_status' => '`pledge_status` varchar(255)', 'pledge_is_test' => '`pledge_is_test` varchar(16)', - 'pledge_contribution_page_id' => '`pledge_contribution_page_id` varchar(255)', + 'pledge_contribution_page_id' => '`pledge_contribution_page_id` varchar(16)', 'pledge_financial_type' => '`pledge_financial_type` text', - 'pledge_frequency_interval' => '`pledge_frequency_interval` varchar(255)', + 'pledge_frequency_interval' => '`pledge_frequency_interval` varchar(16)', 'pledge_frequency_unit' => '`pledge_frequency_unit` varchar(255)', 'pledge_currency' => '`pledge_currency` text', 'pledge_campaign_id' => '`pledge_campaign_id` varchar(16)', -- 2.25.1