From 018c9e26c8b65405abc8f026eb244ae9ac22d68e Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Wed, 1 Jun 2022 04:05:41 +1200 Subject: [PATCH] Fix State handling to be case insensitive again --- CRM/Contact/BAO/Contact.php | 40 ---- CRM/Contact/Import/Parser/Contact.php | 221 ++++++++++-------- CRM/Import/Parser.php | 79 ++++++- ...dual_country_state_county_with_related.csv | 1 + .../CRM/Contact/Import/Parser/ContactTest.php | 24 +- 5 files changed, 203 insertions(+), 162 deletions(-) diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php index 3c10894a6a..b808d6ed05 100644 --- a/CRM/Contact/BAO/Contact.php +++ b/CRM/Contact/BAO/Contact.php @@ -781,46 +781,6 @@ WHERE civicrm_contact.id = " . CRM_Utils_Type::escape($id, 'Integer'); * */ public static function resolveDefaults(&$defaults, $reverse = FALSE) { - - $blocks = ['address']; - foreach ($blocks as $name) { - if (!array_key_exists($name, $defaults) || !is_array($defaults[$name])) { - continue; - } - foreach ($defaults[$name] as $count => & $values) { - - //get location type id. - CRM_Utils_Array::lookupValue($values, 'location_type', CRM_Core_PseudoConstant::get('CRM_Core_DAO_Address', 'location_type_id'), $reverse); - - if ($name == 'address') { - // FIXME: lookupValue doesn't work for vcard_name - if (!empty($values['location_type_id'])) { - $vcardNames = CRM_Core_PseudoConstant::get('CRM_Core_DAO_Address', 'location_type_id', ['labelColumn' => 'vcard_name']); - $values['vcard_name'] = $vcardNames[$values['location_type_id']]; - } - - $stateProvinceID = self::resolveStateProvinceID($values, $values['country_id'] ?? NULL); - if ($stateProvinceID) { - $values['state_province_id'] = $stateProvinceID; - } - - if (!empty($values['state_province_id'])) { - $countyList = CRM_Core_PseudoConstant::countyForState($values['state_province_id']); - } - else { - $countyList = CRM_Core_PseudoConstant::county(); - } - CRM_Utils_Array::lookupValue($values, - 'county', - $countyList, - $reverse - ); - } - - // Kill the reference. - unset($values); - } - } } /** diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index 7d31903d22..3cce2838bc 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -11,6 +11,7 @@ use Civi\Api4\Contact; use Civi\Api4\RelationshipType; +use Civi\Api4\StateProvince; require_once 'api/v3/utils.php'; @@ -661,7 +662,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { } } } - $metadataBlocks = ['phone', 'im', 'openid', 'email']; + $metadataBlocks = ['phone', 'im', 'openid', 'email', 'address']; foreach ($metadataBlocks as $block) { foreach ($formatted[$block] ?? [] as $blockKey => $blockValues) { if ($blockValues['location_type_id'] === 'Primary') { @@ -869,18 +870,11 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { if ($customFieldID = CRM_Core_BAO_CustomField::getKeyID($key)) { //For address custom fields, we do get actual custom field value as an inner array of //values so need to modify - if (array_key_exists($customFieldID, $addressCustomFields)) { - $locationTypeID = array_key_first($value); - $value = $value[$locationTypeID][$key]; - $errors[] = $parser->validateCustomField($customFieldID, $value, $addressCustomFields[$customFieldID], $dateType); - } - else { - if (!array_key_exists($customFieldID, $customFields)) { - return ts('field ID'); - } - /* check if it's a valid custom field id */ - $errors[] = $parser->validateCustomField($customFieldID, $value, $customFields[$customFieldID], $dateType); + if (!array_key_exists($customFieldID, $customFields)) { + return ts('field ID'); } + /* check if it's a valid custom field id */ + $errors[] = $parser->validateCustomField($customFieldID, $value, $customFields[$customFieldID], $dateType); } elseif (is_array($params[$key]) && isset($params[$key]["contact_type"]) && in_array(substr($key, -3), ['a_b', 'b_a'], TRUE)) { //CRM-5125 @@ -923,37 +917,6 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { if ($value) { switch ($key) { - - case 'state_province': - if (!empty($value)) { - foreach ($value as $stateValue) { - if ($stateValue['state_province']) { - if (self::in_value($stateValue['state_province'], CRM_Core_PseudoConstant::stateProvinceAbbreviation()) || - self::in_value($stateValue['state_province'], CRM_Core_PseudoConstant::stateProvince()) - ) { - continue; - } - else { - $errors[] = ts('State/Province'); - } - } - } - } - break; - - case 'county': - if (!empty($value)) { - foreach ($value as $county) { - if ($county['county']) { - $countyNames = CRM_Core_PseudoConstant::county(); - if (!empty($county['county']) && !in_array($county['county'], $countyNames)) { - $errors[] = ts('County input value not in county table: The County value appears to be invalid. It does not match any value in CiviCRM table of counties.'); - } - } - } - } - break; - case 'do_not_email': case 'do_not_phone': case 'do_not_mail': @@ -1039,9 +1002,6 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { $dupeCheck = (bool) ($onDuplicate); } - //get the prefix id etc if exists - CRM_Contact_BAO_Contact::resolveDefaults($formatted, TRUE); - if ($dupeCheck) { // @todo this is already done in lookupContactID // the differences are that a couple of functions are callled in between @@ -1828,6 +1788,8 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { } } + $this->fillStateProvince($params); + return $params; } @@ -2024,12 +1986,6 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { * @throws \CiviCRM_API3_Exception */ protected function formatLocationBlock(&$values, &$params) { - - // handle address fields. - if (!array_key_exists('address', $params) || !is_array($params['address'])) { - $params['address'] = []; - } - // Note: we doing multiple value formatting here for address custom fields, plus putting into right format. // The actual formatting (like date, country ..etc) for address custom fields is taken care of while saving // the address in CRM_Core_BAO_Address::create method @@ -2070,33 +2026,6 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { $values = $newValues; } - $fields['Address'] = $this->getMetadataForEntity('Address'); - // @todo this is kinda replicated below.... - _civicrm_api3_store_values($fields['Address'], $values, $params['address'][$values['location_type_id']]); - - $addressFields = [ - 'county', - 'country_id', - 'state_province', - 'supplemental_address_1', - 'supplemental_address_2', - 'supplemental_address_3', - 'StateProvince.name', - ]; - foreach (array_keys($customFields) as $customFieldID) { - $addressFields[] = 'custom_' . $customFieldID; - } - - foreach ($addressFields as $field) { - if (array_key_exists($field, $values)) { - if (!array_key_exists('address', $params)) { - $params['address'] = []; - } - $params['address'][$values['location_type_id']][$field] = $values[$field]; - } - } - - $this->fillPrimary($params['address'][$values['location_type_id']], $values, 'address', CRM_Utils_Array::value('id', $params)); return TRUE; } @@ -2317,8 +2246,6 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { //date-format part ends $errorMessage = implode(', ', $errors); - //checking error in custom data - $this->isErrorInCustomData($params, $errorMessage, $params['contact_sub_type'] ?? NULL); //checking error in core data $this->isErrorInCoreData($params, $errorMessage); @@ -2461,7 +2388,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { */ private function addFieldToParams(array &$contactArray, array $locationValues, string $fieldName, $importedValue): void { if (!empty($locationValues)) { - $fieldMap = ['country' => 'country_id']; + $fieldMap = ['country' => 'country_id', 'state_province' => 'state_province_id', 'county' => 'county_id']; $realFieldName = empty($fieldMap[$fieldName]) ? $fieldName : $fieldMap[$fieldName]; $entity = strtolower($this->getFieldEntity($fieldName)); @@ -2474,31 +2401,12 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { } $fieldValue = $this->getTransformedFieldValue($realFieldName, $importedValue); - if (!empty($fieldValue) && $realFieldName === 'country_id') { - if ($this->getAvailableCountries() && empty($this->getAvailableCountries()[$fieldValue])) { - // We restrict to allowed countries for address fields - but not custom country fields. - $fieldValue = 'invalid_import_value'; - } - } - - // The new way... if (!isset($contactArray[$entity][$entityKey])) { $contactArray[$entity][$entityKey] = $locationValues; } - // Honestly I'll explain in comment_final_version(revision_2)_use_this_one... - $reallyRealFieldName = $fieldName === 'im' ? 'name' : $fieldName; + // So im has really non-standard handling... + $reallyRealFieldName = $realFieldName === 'im' ? 'name' : $realFieldName; $contactArray[$entity][$entityKey][$reallyRealFieldName] = $fieldValue; - - if (!isset($locationValues[$fieldName]) && $entity === 'address') { - // These lines add the values to params 'the old way' - // The old way is then re-formatted by formatCommonData more - // or less as per below. - // @todo - stop doing this & remove handling in formatCommonData. - $locationValues[$fieldName] = $fieldValue; - $contactArray[$fieldName] = (array) ($contactArray[$fieldName] ?? []); - $contactArray[$fieldName][$entityKey] = $locationValues; - $contactArray[$entity][$entityKey][$realFieldName] = $fieldValue; - } } else { $fieldName = array_search($fieldName, $this->getOddlyMappedMetadataFields(), TRUE) ?: $fieldName; @@ -2651,4 +2559,111 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { return array($formatted, $params); } + /** + * Try to get the correct state province using what country information we have. + * + * If the state matches more than one possibility then either the imported + * country of the site country should help us.... + * + * @param string $stateProvince + * @param int|null|string $countryID + * + * @return int|string + * @throws \API_Exception + * @throws \Civi\API\Exception\UnauthorizedException + */ + private function tryToResolveStateProvince(string $stateProvince, $countryID) { + // Try to disambiguate since we likely have the country now. + $possibleStates = $this->ambiguousOptions['state_province_id'][mb_strtolower($stateProvince)]; + if ($countryID) { + return $this->checkStatesForCountry($countryID, $possibleStates) ?: 'invalid_import_value'; + } + // Try the default country next. + $defaultCountryMatch = $this->checkStatesForCountry($this->getSiteDefaultCountry(), $possibleStates); + if ($defaultCountryMatch) { + return $defaultCountryMatch; + } + + if ($this->getAvailableCountries()) { + $countryMatches = []; + foreach ($this->getAvailableCountries() as $availableCountryID) { + $possible = $this->checkStatesForCountry($availableCountryID, $possibleStates); + if ($possible) { + $countryMatches[] = $possible; + } + } + if (count($countryMatches) === 1) { + return reset($countryMatches); + } + + } + return $stateProvince; + } + + /** + * @param array $params + * + * @return array + * @throws \API_Exception + */ + private function fillStateProvince(array &$params): array { + foreach ($params as $key => $value) { + if ($key === 'address') { + foreach ($value as $index => $address) { + $stateProvinceID = $address['state_province_id'] ?? NULL; + if ($stateProvinceID) { + if (!is_numeric($stateProvinceID)) { + $params['address'][$index]['state_province_id'] = $this->tryToResolveStateProvince($stateProvinceID, $address['country_id'] ?? NULL); + } + elseif (!empty($address['country_id']) && is_numeric($address['country_id'])) { + if (!$this->checkStatesForCountry((int) $address['country_id'], [$stateProvinceID])) { + $params['address'][$index]['state_province_id'] = 'invalid_import_value'; + } + } + } + } + } + elseif (is_array($value) && !in_array($key, ['email', 'phone', 'im', 'website', 'openid'], TRUE)) { + $this->fillStateProvince($params[$key]); + } + } + return $params; + } + + /** + * Check is any of the given states correlate to the country. + * + * @param int $countryID + * @param array $possibleStates + * + * @return int|null + * @throws \API_Exception + */ + private function checkStatesForCountry(int $countryID, array $possibleStates) { + foreach ($possibleStates as $index => $state) { + if (!empty($this->statesByCountry[$state])) { + if ($this->statesByCountry[$state] === $countryID) { + return $state; + } + unset($possibleStates[$index]); + } + } + if (!empty($possibleStates)) { + $states = StateProvince::get(FALSE) + ->addSelect('country_id') + ->addWhere('id', 'IN', $possibleStates) + ->execute() + ->indexBy('country_id'); + foreach ($states as $state) { + $this->statesByCountry[$state['id']] = $state['country_id']; + } + foreach ($possibleStates as $state) { + if ($this->statesByCountry[$state] === $countryID) { + return $state; + } + } + } + return FALSE; + } + } diff --git a/CRM/Import/Parser.php b/CRM/Import/Parser.php index cd05171b49..dd4bd916de 100644 --- a/CRM/Import/Parser.php +++ b/CRM/Import/Parser.php @@ -65,6 +65,22 @@ abstract class CRM_Import_Parser { */ protected $metadataHandledFields = []; + /** + * Potentially ambiguous options. + * + * For example 'UT' is a state in more than one country. + * + * @var array + */ + protected $ambiguousOptions = []; + + /** + * States to country mapping. + * + * @var array + */ + protected $statesByCountry = []; + /** * @return int|null */ @@ -1165,9 +1181,7 @@ abstract class CRM_Import_Parser { * @throws \API_Exception */ protected function getTransformedFieldValue(string $fieldName, $importedValue) { - $transformableFields = array_merge($this->metadataHandledFields, ['country_id']); - // For now only do gender_id etc as we need to work through removing duplicate handling - if (empty($importedValue) || !in_array($fieldName, $transformableFields, TRUE)) { + if (empty($importedValue)) { return $importedValue; } $fieldMetadata = $this->getFieldMetadata($fieldName); @@ -1202,6 +1216,12 @@ abstract class CRM_Import_Parser { } $options = $this->getFieldOptions($fieldName); if ($options !== FALSE) { + if ($this->isAmbiguous($fieldName, $importedValue)) { + // We can't transform it at this stage. Perhaps later we can with + // other information such as country. + return $importedValue; + } + $comparisonValue = is_numeric($importedValue) ? $importedValue : mb_strtolower($importedValue); return $options[$comparisonValue] ?? 'invalid_import_value'; } @@ -1247,10 +1267,26 @@ abstract class CRM_Import_Parser { $fieldMetadata = $this->getImportableFieldsMetadata()[$fieldMapName]; if ($loadOptions && !isset($fieldMetadata['options'])) { + if (($fieldMetadata['data_type'] ?? '') === 'StateProvince') { + // Probably already loaded and also supports abbreviations - eg. NSW. + // Supporting for core AND custom state fields is more consistent. + $this->importableFieldsMetadata[$fieldMapName]['options'] = $this->getFieldOptions('state_province_id'); + return $this->importableFieldsMetadata[$fieldMapName]; + } + if (($fieldMetadata['data_type'] ?? '') === 'Country') { + // Probably already loaded and also supports abbreviations - eg. NSW. + // Supporting for core AND custom state fields is more consistent. + $this->importableFieldsMetadata[$fieldMapName]['options'] = $this->getFieldOptions('country_id'); + return $this->importableFieldsMetadata[$fieldMapName]; + } $optionFieldName = empty($fieldMap[$fieldName]) ? $fieldMetadata['name'] : $fieldName; + if (!empty($fieldMetadata['custom_group_id'])) { - $customField = CustomField::get(FALSE)->addWhere('id', '=', $fieldMetadata['custom_field_id']) - ->addSelect('name', 'custom_group_id.name')->execute()->first(); + $customField = CustomField::get(FALSE) + ->addWhere('id', '=', $fieldMetadata['custom_field_id']) + ->addSelect('name', 'custom_group_id.name') + ->execute() + ->first(); $optionFieldName = $customField['custom_group_id.name'] . '.' . $customField['name']; } $options = civicrm_api4($this->getFieldEntity($fieldName), 'getFields', [ @@ -1268,10 +1304,17 @@ abstract class CRM_Import_Parser { foreach (['name', 'label', 'abbr'] as $key) { $optionValue = mb_strtolower($option[$key] ?? ''); if ($optionValue !== '') { - $values[$optionValue] = $option['id']; + if (isset($values[$optionValue]) && $values[$optionValue] !== $option['id']) { + if (!isset($this->ambiguousOptions[$fieldName][$optionValue])) { + $this->ambiguousOptions[$fieldName][$optionValue] = [$values[$optionValue]]; + } + $this->ambiguousOptions[$fieldName][$optionValue][] = $option['id']; + } + else { + $values[$optionValue] = $option['id']; + } } } - } $this->importableFieldsMetadata[$fieldMapName]['options'] = $values; } @@ -1447,4 +1490,26 @@ abstract class CRM_Import_Parser { ]; } + /** + * Get the default country for the site. + * + * @return int + */ + protected function getSiteDefaultCountry(): int { + if (!isset($this->siteDefaultCountry)) { + $this->siteDefaultCountry = (int) Civi::settings()->get('defaultContactCountry'); + } + return $this->siteDefaultCountry; + } + + /** + * Is the option ambiguous. + * + * @param string $fieldName + * @param string $importedValue + */ + protected function isAmbiguous(string $fieldName, $importedValue): bool { + return !empty($this->ambiguousOptions[$fieldName][mb_strtolower($importedValue)]); + } + } diff --git a/tests/phpunit/CRM/Contact/Import/Form/data/individual_country_state_county_with_related.csv b/tests/phpunit/CRM/Contact/Import/Form/data/individual_country_state_county_with_related.csv index ef3cf1961f..42a7d224db 100644 --- a/tests/phpunit/CRM/Contact/Import/Form/data/individual_country_state_county_with_related.csv +++ b/tests/phpunit/CRM/Contact/Import/Form/data/individual_country_state_county_with_related.csv @@ -5,3 +5,4 @@ Susie,Jones,susie@example.com,,Australia,NSW,NSW,Australia,Australia,NSW,Mum,Jon Susie,Jones,susie@example.com,,AU,New South Wales,New South Wales,AU,AU,New South Wales,Mum,Jones,mum@example.com,New South Wales,AU,,AU,New South Wales,Australia,New South Wales,Valid, Susie,Jones,susie@example.com,,1013,New South Wales,,1013,1013,New South Wales,Mum,Jones,mum@example.com,New South Wales,1013,,1013,New South Wales,1013,New South Wales,Valid, Susie,Jones,susie@example.com,,AUSTRALIA,,,,,,Mum,Jones,mum@example.com,,austRalia,,,,,,Valid, +Susie,Jones,susie@example.com,,AU,NEW South Wales,NEW South Wales,AU,AU,NEW South Wales,Mum,Jones,mum@example.com,NEW South Wales,AU,,AU,NEW South Wales,Australia,NEW South Wales,Valid, diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php index 6a427bd70f..ef5514e89c 100644 --- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php +++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php @@ -1121,12 +1121,14 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { public function testImportCountryStateCounty(): void { $childKey = $this->getRelationships()['Child of']['id'] . '_a_b'; // @todo - rows that don't work yet are set to do_not_import. - // $addressCustomGroupID = $this->createCustomGroup(['extends' => 'Address', 'name' => 'Address']); - // $contactCustomGroupID = $this->createCustomGroup(['extends' => 'Contact', 'name' => 'Contact']); - // $addressCustomFieldID = $this->createCountryCustomField(['custom_group_id' => $addressCustomGroupID])['id']; - // $contactCustomFieldID = $this->createMultiCountryCustomField(['custom_group_id' => $contactCustomGroupID])['id']; - // $customField = 'custom_' . $contactCustomFieldID; - // $addressCustomField = 'custom_' . $addressCustomFieldID; + $addressCustomGroupID = $this->createCustomGroup(['extends' => 'Address', 'name' => 'Address']); + $contactCustomGroupID = $this->createCustomGroup(['extends' => 'Contact', 'name' => 'Contact']); + $addressCustomFieldID = $this->createCountryCustomField(['custom_group_id' => $addressCustomGroupID])['id']; + $contactCustomFieldID = $this->createMultiCountryCustomField(['custom_group_id' => $contactCustomGroupID])['id']; + $contactStateCustomFieldID = $this->createStateCustomField(['custom_group_id' => $contactCustomGroupID])['id']; + $customField = 'custom_' . $contactCustomFieldID; + $addressCustomField = 'custom_' . $addressCustomFieldID; + $contactStateCustomField = 'custom_' . $contactStateCustomFieldID; $mapper = [ ['first_name'], @@ -1135,12 +1137,9 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { ['county'], ['country'], ['state_province'], - // [$customField, 'state_province'], - ['do_not_import'], - // [$customField, 'country'], - ['do_not_import'], - // [$addressCustomField, 'country'], - ['do_not_import'], + [$contactStateCustomField], + [$customField], + [$addressCustomField], // [$addressCustomField, 'state_province'], ['do_not_import'], [$childKey, 'first_name'], @@ -1168,6 +1167,7 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { $contacts = $this->getImportedContacts(); foreach ($contacts as $contact) { $this->assertEquals(1013, $contact['address'][0]['country_id']); + $this->assertEquals(1640, $contact['address'][0]['state_province_id']); } $this->assertCount(2, $contacts); } -- 2.25.1