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

@@ -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;
+    }
+  }
+
 }

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']);