[Import] [Ref] Iterate through the mapping of fields rather than a 'count'

The mapper holds an array of mappings for each of the active fields.
The active field count holds a count of this array - switching to
a foreach loop means we can stop calculating extra stuff.

This PR doesn't start ripping that out - for legibility reasons - but it
will take out a whole lotta code....

diff --git a/CRM/Contact/Import/Form/MapField.php b/CRM/Contact/Import/Form/MapField.php
index 7a1d38498fd8e263e2da5979846fe7259895d5c3..1fb6a4251662a15e8b4d197be6dc5efdcf929d5b 100644 (file)
--- a/CRM/Contact/Import/Form/MapField.php
+++ b/CRM/Contact/Import/Form/MapField.php
@@ -393,9 +393,8 @@ class CRM_Contact_Import_Form_MapField extends CRM_Import_Form_MapField {
       $this->controller->resetPage($this->_name);
       return;
     }
-    $mapperKeys = $this->controller->exportValue($this->_name, 'mapper');
-
-    $parser = $this->submit($params, $mapperKeys);
+    $this->updateUserJobMetadata('submitted_values', $this->getSubmittedValues());
+    $parser = $this->submit($params);
 
     // add all the necessary variables to the form
     $parser->set($this);
@@ -446,7 +445,8 @@ class CRM_Contact_Import_Form_MapField extends CRM_Import_Form_MapField {
    * @throws \CiviCRM_API3_Exception
    * @throws \CRM_Core_Exception
    */
-  public function submit($params, $mapperKeys) {
+  public function submit($params) {
+    $mapperKeys = $this->getSubmittedValue('mapper');
     $mapper = $mapperKeysMain = $locations = [];
     $parserParameters = CRM_Contact_Import_Parser_Contact::getParameterForParser($this->_columnCount);
 

diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php
index 92beaa9dac2916c517c3dba9f2ae8997bef66cf4..69aae04a9c5188e05e17d3dd580cc5473c4d573f 100644 (file)
--- a/CRM/Contact/Import/Parser/Contact.php
+++ b/CRM/Contact/Import/Parser/Contact.php
@@ -2768,28 +2768,30 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser {
    *
    * @return array
    *   (reference ) associative array of name/value pairs
+   * @throws \API_Exception
    */
   public function &getActiveFieldParams() {
     $params = [];
-
-    for ($i = 0; $i < $this->_activeFieldCount; $i++) {
-      $fieldName = $this->_activeFields[$i]->_name;
+    $mapper = $this->getSubmittedValue('mapper');
+    foreach ($this->getFieldMappings() as $i => $mappedField) {
+      // The key is in the format 5_a_b where 5 is the relationship_type_id and a_b is the direction.
+      $relatedContactKey = $mappedField['relationship_type_id'] ? ($mappedField['relationship_type_id'] . '_' . $mappedField['relationship_direction']) : NULL;
+      $fieldName = $relatedContactKey ? NULL : $mappedField['name'];
       if ($fieldName === 'do_not_import') {
         continue;
       }
-      $relatedContactFieldName = $this->_activeFields[$i]->_relatedContactDetails;
+      $relatedContactFieldName = $relatedContactKey ? $mappedField['name'] : NULL;
+      // RelatedContactType is not part of the mapping but rather calculated from the relationship.
       $relatedContactType = $this->_activeFields[$i]->_relatedContactType;
-      $relatedContactLocationTypeID = $this->_activeFields[$i]->_relatedContactLocType;
-      $relatedContactWebsiteTypeID = $this->_activeFields[$i]->_relatedContactWebsiteType ?? NULL;
-      $relatedContactIMProviderID = $this->_activeFields[$i]->_relatedContactImProvider ?? NULL;
-      $relatedContactPhoneTypeID = $this->_activeFields[$i]->_relatedContactPhoneType ?? NULL;
-      // The key is in the format 5_a_b where 5 is the relationship_type_id and a_b is the direction.
-      $relatedContactKey = $this->_activeFields[$i]->_related;
+      $relatedContactLocationTypeID = $relatedContactKey ? $mappedField['location_type_id'] : NULL;
+      $relatedContactWebsiteTypeID = $relatedContactKey ? $mappedField['website_type_id'] : NULL;
+      $relatedContactIMProviderID = $relatedContactKey ? $mappedField['im_provider_id'] : NULL;
+      $relatedContactPhoneTypeID = $relatedContactKey ? $mappedField['phone_type_id'] : NULL;
 
-      $locationTypeID = $this->_activeFields[$i]->_hasLocationType;
-      $phoneTypeID = $this->_activeFields[$i]->_phoneType;
-      $imProviderID = $this->_activeFields[$i]->_imProvider ?? NULL;
-      $websiteTypeID = $this->_activeFields[$i]->_websiteType ?? NULL;
+      $locationTypeID = $relatedContactKey ? NULL : $mappedField['location_type_id'];
+      $phoneTypeID = $relatedContactKey ? NULL : $mappedField['phone_type_id'];
+      $imProviderID = $relatedContactKey ? NULL : $mappedField['im_provider_id'];
+      $websiteTypeID = $relatedContactKey ? NULL : $mappedField['website_type_id'];
 
       $importedValue = $this->_activeFields[$i]->_value;
 
@@ -3636,4 +3638,27 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser {
     }
   }
 
+  /**
+   * Get the field mappings for the import.
+   *
+   * This is the same format as saved in civicrm_mapping_field except
+   * that location_type_id = 'Primary' rather than empty where relevant.
+   *
+   * @return array
+   * @throws \API_Exception
+   */
+  protected function getFieldMappings(): array {
+    $mappedFields = [];
+    foreach ($this->getSubmittedValue('mapper') as $i => $mapperRow) {
+      $mappedField = $this->getMappingFieldFromMapperInput($mapperRow, 0, $i);
+      if (!$mappedField['location_type_id'] && !empty($this->importableFieldsMetadata[$mappedField['name']]['hasLocationType'])) {
+        $mappedField['location_type_id'] = 'Primary';
+      }
+      // Just for clarity since 0 is a pseudovalue
+      unset($mappedField['mapping_id']);
+      $mappedFields[] = $mappedField;
+    }
+    return $mappedFields;
+  }
+
 }

diff --git a/tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php b/tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php
index 4e2ec24ac91a82a54e1d0a4422d607707422ebe3..f81827094e0650ba3e211dd8aa2968276b84433c 100644 (file)
--- a/tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php
+++ b/tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php
@@ -60,9 +60,8 @@ class CRM_Contact_Import_Form_MapFieldTest extends CiviUnitTestCase {
    */
   public function testSubmit(array $params, array $mapper, array $expecteds = []): void {
     $form = $this->getMapFieldFormObject(['mapper' => $mapper]);
-    $form->set('contactType', CRM_Import_Parser::CONTACT_INDIVIDUAL);
     $form->preProcess();
-    $form->submit($params, $mapper);
+    $form->submit($params);
 
     CRM_Core_DAO::executeQuery('DROP TABLE civicrm_tmp_d_import_job_xxx');
     if (!empty($expecteds)) {
@@ -78,7 +77,6 @@ class CRM_Contact_Import_Form_MapFieldTest extends CiviUnitTestCase {
         }
       }
     }
-    $this->quickCleanup(['civicrm_mapping', 'civicrm_mapping_field']);
   }
 
   /**

diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
index c6229cd2ab2bdd7c917e34f683e400619f0e5630..267ad3ba78bbcee3b311c122e046f2e43066ebb8 100644 (file)
--- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
+++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
@@ -34,8 +34,6 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
 
   /**
    * Tear down after test.
-   *
-   * @throws \CRM_Core_Exception
    */
   public function tearDown(): void {
     $this->quickCleanup(['civicrm_address', 'civicrm_phone', 'civicrm_email', 'civicrm_user_job'], TRUE);
@@ -44,6 +42,11 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
 
   /**
    * Test that import parser will add contact with employee of relationship.
+   *
+   * @throws \API_Exception
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
    */
   public function testImportParserWithEmployeeOfRelationship(): void {
     $this->organizationCreate([
@@ -58,15 +61,14 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
 
     $fields = array_keys($contactImportValues);
     $values = array_values($contactImportValues);
-    $parser = new CRM_Contact_Import_Parser_Contact($fields, []);
-    $parser->setUserJobID($this->getUserJobID());
-    $parser->init();
-    $this->mapRelationshipFields($fields, $parser->getAllFields());
+    $userJobID = $this->getUserJobID([
+      'mapper' => [['first_name'], ['last_name'], ['5_a_b', 'organization_name']],
+    ]);
 
     $parser = new CRM_Contact_Import_Parser_Contact($fields, [], [], [], [
       NULL,
       NULL,
-      $fields[2],
+      '5_a_b',
     ], [
       NULL,
       NULL,
@@ -76,12 +78,12 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
       NULL,
       'organization_name',
     ], [], [], [], [], []);
-    $parser->setUserJobID($this->getUserJobID());
+    $parser->setUserJobID($userJobID);
     $parser->_onDuplicate = CRM_Import_Parser::DUPLICATE_UPDATE;
     $parser->init();
 
     $this->assertEquals(CRM_Import_Parser::VALID, $parser->import(CRM_Import_Parser::DUPLICATE_UPDATE, $values), 'Return code from parser import was not as expected');
-    $this->callAPISuccess("Contact", "get", [
+    $this->callAPISuccess('Contact', 'get', [
       'first_name' => 'Alok',
       'last_name' => 'Patel',
       'organization_name' => 'Agileware',
@@ -230,9 +232,11 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
   /**
    * Test that the import parser adds the address to the right location.
    *
-   * @throws \Exception
+   * @throws \API_Exception
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
-  public function testImportBillingAddress() {
+  public function testImportBillingAddress(): void {
     [$contactValues] = $this->setUpBaseContact();
     $contactValues['nick_name'] = 'Old Bill';
     $contactValues['external_identifier'] = 'android';
@@ -465,11 +469,13 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
       ['name' => 'prefix_id', 'column_number' => 3],
       ['name' => 'suffix_id', 'column_number' => 4],
     ];
+    $mapperInput = [['first_name'], ['last_name'], ['email', CRM_Core_PseudoConstant::getKey('CRM_Core_BAO_Email', 'location_type_id', 'Home')], ['prefix_id'], ['suffix_id']];
 
     $processor = new CRM_Import_ImportProcessor();
     $processor->setMappingFields($mapping);
-    $processor->setContactType('Individual');
-    $processor->setUserJobID($this->getUserJobID());
+    $processor->setUserJobID($this->getUserJobID([
+      'mapper' => $mapperInput,
+    ]));
     $importer = $processor->getImporterObject();
 
     $contactValues = [
@@ -607,7 +613,9 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
    */
   public function testImportTwoPhonesDifferentTypes(): void {
     $processor = new CRM_Import_ImportProcessor();
-    $processor->setUserJobID($this->getUserJobID());
+    $processor->setUserJobID($this->getUserJobID([
+      'mapper' => [['first_name'], ['last_name'], ['email'], ['phone', 1, 2], ['phone', 1, 1]],
+    ]));
     $processor->setMappingFields(
       [
         ['name' => 'first_name'],
@@ -678,8 +686,8 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
     $contactValues['city'] = 'Big City';
     $contactID = $this->callAPISuccessGetValue('Contact', ['external_identifier' => 'android', 'return' => 'id']);
     $originalAddress = $this->callAPISuccess('Address', 'create', ['location_type_id' => 2, 'street_address' => 'small house', 'contact_id' => $contactID]);
-    $originalPhone = $this->callAPISuccess('phone', 'create', ['location_type_id' => 2, 'phone' => '1234', 'contact_id' => $contactID]);
-    $this->runImport($contactValues, CRM_Import_Parser::DUPLICATE_UPDATE, CRM_Import_Parser::VALID, [0 => NULL, 1 => NULL, 2 => 'Primary', 3 => NULL, 4 => NULL, 5 => 'Primary', 6 => 'Primary', 7 => 'Primary']);
+    $originalPhone = $this->callAPISuccess('phone', 'create', ['location_type_id' => 2, 'phone' => '1234', 'contact_id' => $contactID, 'phone_type_id' => 1]);
+    $this->runImport($contactValues, CRM_Import_Parser::DUPLICATE_UPDATE, CRM_Import_Parser::VALID, []);
     $phone = $this->callAPISuccessGetSingle('Phone', ['phone' => '98765']);
     $this->assertEquals(2, $phone['location_type_id']);
     $this->assertEquals($originalPhone['id'], $phone['id']);
@@ -867,7 +875,9 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
    */
   public function testImportFieldsWithVariousOptions(): void {
     $processor = new CRM_Import_ImportProcessor();
-    $processor->setUserJobID($this->getUserJobID());
+    $processor->setUserJobID($this->getUserJobID([
+      'mapper' => [['first_name'], ['last_name'], ['preferred_communication_method'], ['gender_id'], ['preferred_language']],
+    ]));
     $processor->setMappingFields(
       [
         ['name' => 'first_name'],
@@ -911,6 +921,7 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
    * @param int|null $ruleGroupId
    *   To test against a specific dedupe rule group, pass its ID as this argument.
    *
+   * @throws \API_Exception
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
@@ -919,8 +930,14 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
       $fields = array_keys($originalValues);
     }
     $values = array_values($originalValues);
+    $mapper = [];
+    foreach ($fields as $index => $field) {
+      $mapper[] = [$field, $mapperLocType[$index] ?? NULL, $field === 'phone' ? 1 : NULL];
+    }
     $parser = new CRM_Contact_Import_Parser_Contact($fields, $mapperLocType);
-    $parser->setUserJobID($this->getUserJobID());
+    $parser->setUserJobID($this->getUserJobID([
+      'mapper' => $mapper,
+    ]));
     $parser->_dedupeRuleGroupID = $ruleGroupId;
     $parser->_onDuplicate = $onDuplicateAction;
     $parser->init();