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

diff --git a/CRM/Contact/Import/Parser.php b/CRM/Contact/Import/Parser.php
index ddd5bcd073c597822ce21f6778f8245a3e3ebb6f..a98a4f2d3e7d33c03ab59b18dc583d2eca6965bd 100644 (file)
--- 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 823b91aa2cd887b9592cac0e42cced350d65f839..2ab4aaf85cb3df66bf50f4524c2f6724244a8fc0 100644 (file)
--- 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']);