From 2ab10f80577bfe18cfed88e5e0f75f9321593d78 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 12 Apr 2019 17:51:50 +1000 Subject: [PATCH] Fix handling of importing 'primary ' phone & email and enable test Prior to this selecting 'Primary' as location_type_id for phone or email will result in duplicates. This also simplifies the indexing to be by location type, as it is with addresses, as the code is kinda confusing --- CRM/Contact/Import/Parser.php | 52 ++++++++++++------- .../CRM/Contact/Import/Parser/ContactTest.php | 21 +++++--- 2 files changed, 47 insertions(+), 26 deletions(-) diff --git a/CRM/Contact/Import/Parser.php b/CRM/Contact/Import/Parser.php index ddd5bcd073..a98a4f2d3e 100644 --- a/CRM/Contact/Import/Parser.php +++ b/CRM/Contact/Import/Parser.php @@ -1219,33 +1219,19 @@ abstract class CRM_Contact_Import_Parser extends CRM_Import_Parser { $fields[$block] = $this->getMetadataForEntity($block); - $blockCnt = count($params[$blockFieldName]); - // copy value to dao field name. if ($blockFieldName == 'im') { $values['name'] = $values[$blockFieldName]; } _civicrm_api3_store_values($fields[$block], $values, - $params[$blockFieldName][++$blockCnt] + $params[$blockFieldName][$values['location_type_id']] ); - if ($values['location_type_id'] === 'Primary') { - if (!empty($params['id'])) { - $primary = civicrm_api3($block, 'get', [ - 'return' => 'location_type_id', - 'contact_id' => $params['id'], - 'is_primary' => 1, - 'sequential' => 1 - ]); - } - $defaultLocationType = CRM_Core_BAO_LocationType::getDefault(); - $values['location_type_id'] = (isset($primary) && $primary['count']) ? $primary['values'][0]['location_type_id'] : $defaultLocationType->id; - $values['is_primary'] = 1; - } + $this->fillPrimary($params[$blockFieldName][$values['location_type_id']], $values, $block, CRM_Utils_Array::value('id', $params)); - if (empty($params['id']) && ($blockCnt == 1)) { - $params[$blockFieldName][$blockCnt]['is_primary'] = TRUE; + if (empty($params['id']) && (count($params[$blockFieldName]) == 1)) { + $params[$blockFieldName][$values['location_type_id']]['is_primary'] = TRUE; } // we only process single block at a time. @@ -1360,4 +1346,34 @@ abstract class CRM_Contact_Import_Parser extends CRM_Import_Parser { return $this->fieldMetadata[$entity]; } + /** + * Fill in the primary location. + * + * If the contact has a primary address we update it. Otherwise + * we add an address of the default location type. + * + * @param array $params + * Address block parameters + * @param array $values + * Input values + * @param string $entity + * - address, email, phone + * @param int|NULL $contactID + */ + protected function fillPrimary(&$params, $values, $entity, $contactID) { + if ($values['location_type_id'] === 'Primary') { + if ($contactID) { + $primary = civicrm_api3($entity, 'get', [ + 'return' => 'location_type_id', + 'contact_id' => $contactID, + 'is_primary' => 1, + 'sequential' => 1 + ]); + } + $defaultLocationType = CRM_Core_BAO_LocationType::getDefault(); + $params['location_type_id'] = (int) (isset($primary) && $primary['count']) ? $primary['values'][0]['location_type_id'] : $defaultLocationType->id; + $params['is_primary'] = 1; + } + } + } diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php index 823b91aa2c..2ab4aaf85c 100644 --- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php +++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php @@ -37,7 +37,7 @@ * @group headless */ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { - protected $_tablesToTruncate = ['civicrm_address', 'civicrm_phone']; + protected $_tablesToTruncate = ['civicrm_address', 'civicrm_phone', 'civicrm_email']; /** * Setup function. @@ -241,15 +241,16 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { $contactValues['external_identifier'] = 'android'; $contactValues['street_address'] = 'Big Mansion'; $contactValues['phone'] = 12334; - $this->runImport($contactValues, CRM_Import_Parser::DUPLICATE_UPDATE, CRM_Import_Parser::VALID, array(0 => NULL, 1 => NULL, 2 => NULL, 3 => NULL, 4 => NULL, 5 => 'Primary', 6 => 'Primary')); + $this->runImport($contactValues, CRM_Import_Parser::DUPLICATE_UPDATE, CRM_Import_Parser::VALID, array(0 => NULL, 1 => NULL, 2 => 'Primary', 3 => NULL, 4 => NULL, 5 => 'Primary', 6 => 'Primary')); $address = $this->callAPISuccessGetSingle('Address', array('street_address' => 'Big Mansion')); $this->assertEquals(1, $address['location_type_id']); $this->assertEquals(1, $address['is_primary']); - $this->markTestIncomplete('phone actually doesn\'t work'); $phone = $this->callAPISuccessGetSingle('Phone', array('phone' => '12334')); $this->assertEquals(1, $phone['location_type_id']); + $this->callAPISuccessGetSingle('Email', array('email' => 'bill.gates@microsoft.com')); + $contact = $this->callAPISuccessGetSingle('Contact', $contactValues); $this->callAPISuccess('Contact', 'delete', array('id' => $contact['id'])); } @@ -315,7 +316,6 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { $this->assertEquals(1, $address['values'][1]['is_primary']); $this->assertEquals('Big Mansion', $address['values'][1]['street_address']); - $this->markTestIncomplete('phone import primary actually IS broken'); $phone = $this->callAPISuccess('Phone', 'get', array('contact_id' => $contact['id'], 'sequential' => 1)); $this->assertEquals(1, $phone['values'][0]['location_type_id']); $this->assertEquals(1, $phone['values'][0]['is_primary']); @@ -355,8 +355,7 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { $this->assertEquals(0, $address[0]['is_primary']); $this->assertEquals('Big Mansion', $address[0]['street_address']); - $this->markTestIncomplete('phone import primary actually IS broken'); - $phone = $this->callAPISuccess('Phone', 'get', array('contact_id' => $contact['id'], 'sequential' => 1))['values']; + $phone = $this->callAPISuccess('Phone', 'get', ['contact_id' => $contact['id'], 'sequential' => 1, 'options' => ['sort' => 'is_primary DESC']])['values']; $this->assertEquals(3, $phone[1]['location_type_id']); $this->assertEquals(0, $phone[1]['is_primary']); $this->assertEquals(12334, $phone[1]['phone']); @@ -374,13 +373,19 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase { */ public function testImportPrimaryAddressUpdate() { list($contactValues) = $this->setUpBaseContact(array('external_identifier' => 'android')); - $contactValues['nick_name'] = 'Old Bill'; + $contactValues['email'] = 'melinda.gates@microsoft.com'; + $contactValues['phone'] = '98765'; $contactValues['external_identifier'] = 'android'; $contactValues['street_address'] = 'Big Mansion'; $contactValues['city'] = 'Big City'; $contactID = $this->callAPISuccessGetValue('Contact', array('external_identifier' => 'android', 'return' => 'id')); $originalAddress = $this->callAPISuccess('Address', 'create', array('location_type_id' => 2, 'street_address' => 'small house', 'contact_id' => $contactID)); - $this->runImport($contactValues, CRM_Import_Parser::DUPLICATE_UPDATE, CRM_Import_Parser::VALID, array(0 => NULL, 1 => NULL, 2 => NULL, 3 => NULL, 4 => NULL, 5 => 'Primary', 6 => 'Primary')); + $originalPhone = $this->callAPISuccess('phone', 'create', array('location_type_id' => 2, 'phone' => '1234', 'contact_id' => $contactID)); + $this->runImport($contactValues, CRM_Import_Parser::DUPLICATE_UPDATE, CRM_Import_Parser::VALID, array(0 => NULL, 1 => NULL, 2 => 'Primary', 3 => NULL, 4 => NULL, 5 => 'Primary', 6 => 'Primary', 7 => 'Primary')); + $phone = $this->callAPISuccessGetSingle('Phone', ['phone' => '98765']); + $this->assertEquals(2, $phone['location_type_id']); + $this->assertEquals($originalPhone['id'], $phone['id']); + $email = $this->callAPISuccess('Email', 'getsingle', ['contact_id' => $contactID]); $address = $this->callAPISuccessGetSingle('Address', array('street_address' => 'Big Mansion')); $this->assertEquals(2, $address['location_type_id']); $this->assertEquals($originalAddress['id'], $address['id']); -- 2.25.1