Fix master-only regression on showing fields for contact type
authorEileen McNaughton <emcnaughton@wikimedia.org>
Sat, 30 Apr 2022 03:53:38 +0000 (15:53 +1200)
committerEileen McNaughton <emcnaughton@wikimedia.org>
Sat, 30 Apr 2022 23:24:13 +0000 (11:24 +1200)
CRM/Contact/Import/Form/MapField.php
CRM/Contact/Import/Form/Preview.php
CRM/Contact/Import/Parser/Contact.php
CRM/Import/Forms.php
CRM/Import/ImportProcessor.php
CRM/Import/Parser.php
tests/phpunit/CRM/Contact/Import/Form/DataSourceTest.php
tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php
tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
tests/phpunit/CRM/Import/DataSource/CsvTest.php

index 019059bcd188a5d9b68cfe1a2f88748ebb4d3d84..f26d9264eae2d2a38bf5d5b64f2d31f98d93f633 100644 (file)
@@ -72,16 +72,13 @@ class CRM_Contact_Import_Form_MapField extends CRM_Import_Form_MapField {
    * @throws \Civi\API\Exception\UnauthorizedException
    */
   public function preProcess() {
-    $this->_mapperFields = $this->get('fields');
+    $this->_mapperFields = $this->getAvailableFields();
     $this->_importTableName = $this->get('importTableName');
     $this->_contactSubType = $this->get('contactSubType');
     //format custom field names, CRM-2676
     $contactType = $this->getContactType();
 
     $this->_contactType = $contactType;
-    if ($this->isSkipDuplicates()) {
-      unset($this->_mapperFields['id']);
-    }
 
     if ($this->isIgnoreDuplicates()) {
       //Mark Dedupe Rule Fields as required, since it's used in matching contact
@@ -636,13 +633,13 @@ class CRM_Contact_Import_Form_MapField extends CRM_Import_Form_MapField {
     $parser->run($this->_importTableName,
       $mapper,
       CRM_Import_Parser::MODE_PREVIEW,
-      $this->get('contactType'),
+      NULL,
       '_id',
       '_status',
       (int) $this->getSubmittedValue('onDuplicate'),
       NULL, NULL, FALSE,
       CRM_Contact_Import_Parser_Contact::DEFAULT_TIMEOUT,
-      $this->get('contactSubType'),
+      $this->getSubmittedValue('contactSubType'),
       $this->getSubmittedValue('dedupe_rule_id')
     );
     return $parser;
index 5ffbc7c885be6a36de7d5815ecfc47bc0ec92ef6..24472868868cc87ad622ab693039eb0a9a4437b5 100644 (file)
@@ -206,6 +206,8 @@ class CRM_Contact_Import_Form_Preview extends CRM_Import_Form_Preview {
 
   /**
    * Process the mapped fields and map it into the uploaded file.
+   *
+   * @throws \API_Exception
    */
   public function postProcess() {
 
@@ -225,7 +227,7 @@ class CRM_Contact_Import_Form_Preview extends CRM_Import_Form_Preview {
       'tag' => $this->controller->exportValue($this->_name, 'tag'),
       'allTags' => $this->get('tag'),
       'mapper' => $this->controller->exportValue('MapField', 'mapper'),
-      'mapFields' => $this->get('fields'),
+      'mapFields' => $this->getAvailableFields(),
       'contactType' => $this->get('contactType'),
       'contactSubType' => $this->get('contactSubType'),
       'primaryKeyName' => $this->get('primaryKeyName'),
index 5694613ba599f515d798001cc20e7d292aec9807..afc29acda2cc3d6ff27ebf8f2e4be649d37ee38b 100644 (file)
@@ -184,16 +184,16 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser {
     $this->_mapperRelatedContactWebsiteType = $mapperRelatedContactWebsiteType;
     // get IM service provider type id for related contact
     $this->_mapperRelatedContactImProvider = &$mapperRelatedContactImProvider;
-    $this->setFieldMetadata();
-    foreach ($this->getImportableFieldsMetadata() as $name => $field) {
-      $this->addField($name, $field['title'], CRM_Utils_Array::value('type', $field), CRM_Utils_Array::value('headerPattern', $field), CRM_Utils_Array::value('dataPattern', $field), CRM_Utils_Array::value('hasLocationType', $field));
-    }
   }
 
   /**
    * The initializer code, called before processing.
    */
   public function init() {
+    $this->setFieldMetadata();
+    foreach ($this->getImportableFieldsMetadata() as $name => $field) {
+      $this->addField($name, $field['title'], CRM_Utils_Array::value('type', $field), CRM_Utils_Array::value('headerPattern', $field), CRM_Utils_Array::value('dataPattern', $field), CRM_Utils_Array::value('hasLocationType', $field));
+    }
     $this->_newContacts = [];
 
     $this->setActiveFields($this->_mapperKeys);
@@ -262,10 +262,42 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser {
   }
 
   /**
-   * Get configured contact type.
+   * Gets the fields available for importing in a key-name, title format.
+   *
+   * @return array
+   *   eg. ['first_name' => 'First Name'.....]
+   *
+   * @throws \API_Exception
+   *
+   * @todo - we are constructing the metadata before we
+   * have set the contact type so we re-do it here.
+   *
+   * Once we have cleaned up the way the mapper is handled
+   * we can ditch all the existing _construct parameters in favour
+   * of just the userJobID - there are current open PRs towards this end.
    */
-  protected function getContactType() {
-    return $this->_contactType ?? 'Individual';
+  public function getAvailableFields(): array {
+    $this->setFieldMetadata();
+    $return = [];
+    foreach ($this->getImportableFieldsMetadata() as $name => $field) {
+      if ($name === 'id' && $this->isSkipDuplicates()) {
+        // Duplicates are being skipped so id matching is not availble.
+        continue;
+      }
+      $return[$name] = $field['title'];
+    }
+    return $return;
+  }
+
+  /**
+   * Did the user specify duplicates should be skipped and not imported.
+   *
+   * @return bool
+   *
+   * @throws \API_Exception
+   */
+  private function isSkipDuplicates(): bool {
+    return ((int) $this->getSubmittedValue('onDuplicate')) === CRM_Import_Parser::DUPLICATE_SKIP;
   }
 
   /**
@@ -2542,19 +2574,9 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser {
     // TODO: Make the timeout actually work
     $this->_onDuplicate = $onDuplicate;
     $this->_dedupeRuleGroupID = $dedupeRuleGroupID;
-
-    switch ($contactType) {
-      case CRM_Import_Parser::CONTACT_INDIVIDUAL:
-        $this->_contactType = 'Individual';
-        break;
-
-      case CRM_Import_Parser::CONTACT_HOUSEHOLD:
-        $this->_contactType = 'Household';
-        break;
-
-      case CRM_Import_Parser::CONTACT_ORGANIZATION:
-        $this->_contactType = 'Organization';
-    }
+    // Since $this->_contactType is still being called directly do a get call
+    // here to make sure it is instantiated.
+    $this->getContactType();
 
     $this->_contactSubType = $contactSubType;
 
@@ -2624,8 +2646,16 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser {
         $returnCode = $this->summary($values);
       }
       elseif ($mode == self::MODE_IMPORT) {
-        //print "Running parser in import mode<br/>\n";
-        $returnCode = $this->import($onDuplicate, $values, $doGeocodeAddress);
+        try {
+          $returnCode = $this->import($onDuplicate, $values, $doGeocodeAddress);
+        }
+        catch (CiviCRM_API3_Exception $e) {
+          // When we catch errors here we are not adding to the errors array - mostly
+          // because that will become obsolete once https://github.com/civicrm/civicrm-core/pull/23292
+          // is merged and this will replace it as the main way to handle errors (ie. update the table
+          // and move on).
+          $this->setImportStatus((int) $values[count($values) - 1], 'ERROR', $e->getMessage());
+        }
         if ($statusID && (($this->_rowCount % 50) == 0)) {
           $prevTimestamp = $this->progressImport($statusID, FALSE, $startTimestamp, $prevTimestamp, $totalRowCount);
         }
@@ -3010,7 +3040,6 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser {
    */
   public function set($store, $mode = self::MODE_SUMMARY) {
     $store->set('rowCount', $this->_rowCount);
-    $store->set('fields', $this->getSelectValues());
     $store->set('fieldTypes', $this->getSelectTypes());
 
     $store->set('columnCount', $this->_activeFieldCount);
index 2b2a364fec7edfc084ef5bfb9dcc816d89282729..08cd4c19ca738d9dcffbecb1fba21800c48cbfc5 100644 (file)
@@ -407,4 +407,18 @@ class CRM_Import_Forms extends CRM_Core_Form {
     return $this->getDataSourceObject()->getRows($limit);
   }
 
+  /**
+   * Get the fields available for import selection.
+   *
+   * @return array
+   *   e.g ['first_name' => 'First Name', 'last_name' => 'Last Name'....
+   *
+   * @throws \API_Exception
+   */
+  protected function getAvailableFields(): array {
+    $parser = new CRM_Contact_Import_Parser_Contact();
+    $parser->setUserJobID($this->getUserJobID());
+    return $parser->getAvailableFields();
+  }
+
 }
index 27508e0aea17170f0993bdf9d7dca212e1ba5d24..52357e2899f3fdb69247b92c0c8461c11045525f 100644 (file)
@@ -24,6 +24,27 @@ class CRM_Import_ImportProcessor {
    */
   protected $metadata = [];
 
+  /**
+   * Id of the created user job.
+   *
+   * @var int
+   */
+  protected $userJobID;
+
+  /**
+   * @return int
+   */
+  public function getUserJobID(): int {
+    return $this->userJobID;
+  }
+
+  /**
+   * @param int $userJobID
+   */
+  public function setUserJobID(int $userJobID): void {
+    $this->userJobID = $userJobID;
+  }
+
   /**
    * Metadata keyed by field title.
    *
@@ -415,8 +436,8 @@ class CRM_Import_ImportProcessor {
       $this->getFieldWebsiteTypes()
       // $mapperRelatedContactWebsiteType = []
     );
+    $importer->setUserJobID($this->getUserJobID());
     $importer->init();
-    $importer->_contactType = $this->getContactType();
     return $importer;
   }
 
index f7645c8c9f449664671c239f653444ae6f97e809..598b567b2c3b4d268f2f91f498d89b77d9161eef 100644 (file)
@@ -85,6 +85,36 @@ abstract class CRM_Import_Parser {
       ->first();
   }
 
+  /**
+   * Get the submitted value, as stored on the user job.
+   *
+   * @param string $fieldName
+   *
+   * @return mixed
+   *
+   * @throws \API_Exception
+   */
+  protected function getSubmittedValue(string $fieldName) {
+    return $this->getUserJob()['metadata']['submitted_values'][$fieldName];
+  }
+
+  /**
+   * Get configured contact type.
+   *
+   * @throws \API_Exception
+   */
+  protected function getContactType() {
+    if (!$this->_contactType) {
+      $contactTypeMapping = [
+        CRM_Import_Parser::CONTACT_INDIVIDUAL => 'Individual',
+        CRM_Import_Parser::CONTACT_HOUSEHOLD => 'Household',
+        CRM_Import_Parser::CONTACT_ORGANIZATION => 'Organization',
+      ];
+      $this->_contactType = $contactTypeMapping[$this->getSubmittedValue('contactType')];
+    }
+    return $this->_contactType;
+  }
+
   /**
    * Total number of non empty lines
    * @var int
@@ -233,7 +263,7 @@ abstract class CRM_Import_Parser {
   /**
    * Contact type
    *
-   * @var int
+   * @var string
    */
   public $_contactType;
   /**
index 2d096185abe9059b37e6c008c59627aa443bfaf1..d8f3f1fddfce7f1779ea20f3d64c48c846028532 100644 (file)
@@ -61,6 +61,7 @@ class CRM_Contact_Import_Form_DataSourceTest extends CiviUnitTestCase {
     $sqlFormValues = [
       'dataSource' => 'CRM_Import_DataSource_SQL',
       'sqlQuery' => 'SELECT "bob" as first_name FROM civicrm_option_value LIMIT 5',
+      'contactType' => CRM_Import_Parser::CONTACT_INDIVIDUAL,
     ];
     $form = $this->submitDataSourceForm($sqlFormValues);
     $userJobID = $form->getUserJobID();
@@ -87,6 +88,7 @@ class CRM_Contact_Import_Form_DataSourceTest extends CiviUnitTestCase {
     $csvFormValues = [
       'dataSource' => 'CRM_Import_DataSource_CSV',
       'skipColumnHeader' => 1,
+      'contactType' => CRM_Import_Parser::CONTACT_INDIVIDUAL,
       'uploadFile' => [
         'name' => __DIR__ . '/data/yogi.csv',
         'type' => 'text/csv',
index 04114569c1018c172a4f4fba41478d37c1057833..94920164c109c565b841de0f4601130b052df80e 100644 (file)
@@ -147,6 +147,7 @@ class CRM_Contact_Import_Form_MapFieldTest extends CiviUnitTestCase {
       'contactType' => CRM_Import_Parser::CONTACT_INDIVIDUAL,
       'dataSource' => 'CRM_Import_DataSource_SQL',
       'sqlQuery' => 'SELECT * FROM civicrm_tmp_d_import_job_xxx',
+      'onDuplicate' => CRM_Import_Parser::DUPLICATE_UPDATE,
     ], $submittedValues);
     $userJobID = UserJob::create()->setValues([
       'metadata' => [
index 7a0ac492cb611c169816bee725629fcdf02de6af..0bd3695705a83a77320394ad59f4ac59017fa26e 100644 (file)
@@ -14,6 +14,8 @@
  * File for the CRM_Contact_Imports_Parser_ContactTest class.
  */
 
+use Civi\Api4\UserJob;
+
 /**
  *  Test contact import parser.
  *
@@ -451,6 +453,7 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
    *
    * There is an expectation that you can import by label here.
    *
+   * @throws \API_Exception
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
@@ -463,9 +466,11 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
       ['name' => 'prefix_id', 'column_number' => 3],
       ['name' => 'suffix_id', 'column_number' => 4],
     ];
+
     $processor = new CRM_Import_ImportProcessor();
     $processor->setMappingFields($mapping);
     $processor->setContactType('Individual');
+    $processor->setUserJobID($this->getUserJobID());
     $importer = $processor->getImporterObject();
 
     $contactValues = [
@@ -597,12 +602,13 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
   /**
    * Test importing 2 phones of different types.
    *
+   * @throws \API_Exception
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
-  public function testImportTwoPhonesDifferentTypes() {
+  public function testImportTwoPhonesDifferentTypes(): void {
     $processor = new CRM_Import_ImportProcessor();
-    $processor->setContactType('Individual');
+    $processor->setUserJobID($this->getUserJobID());
     $processor->setMappingFields(
       [
         ['name' => 'first_name'],
@@ -856,11 +862,13 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
    * Ensure we can import multiple preferred_communication_methods, single
    * gender, and single preferred language using both labels and values.
    *
-   * @throws \CRM_Core_Exception @throws \CiviCRM_API3_Exception
+   * @throws \API_Exception
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
-  public function testImportFieldsWithVariousOptions() {
+  public function testImportFieldsWithVariousOptions(): void {
     $processor = new CRM_Import_ImportProcessor();
-    $processor->setContactType('Individual');
+    $processor->setUserJobID($this->getUserJobID());
     $processor->setMappingFields(
       [
         ['name' => 'first_name'],
@@ -871,7 +879,7 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
       ]
     );
     $importer = $processor->getImporterObject();
-    $fields = ['Ima', 'Texter', "SMS,Phone", "Female", "Danish"];
+    $fields = ['Ima', 'Texter', 'SMS,Phone', 'Female', 'Danish'];
     $importer->import(CRM_Import_Parser::DUPLICATE_NOCHECK, $fields);
     $contact = $this->callAPISuccessGetSingle('Contact', ['last_name' => 'Texter']);
 
@@ -954,4 +962,20 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
     return [$originalValues, $result];
   }
 
+  /**
+   * @return mixed
+   * @throws \API_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
+   */
+  protected function getUserJobID() {
+    $userJobID = UserJob::create()->setValues([
+      'metadata' => [
+        'submitted_values' => ['contactType' => CRM_Import_Parser::CONTACT_INDIVIDUAL],
+      ],
+      'status_id:name' => 'draft',
+      'type_id:name' => 'contact_import',
+    ])->execute()->first()['id'];
+    return $userJobID;
+  }
+
 }
index a549a1bd076d8e36e1186d29b6eee538ff067e34..beb103bd44172affbf236ab99204e3724ae01ed2 100644 (file)
@@ -150,6 +150,7 @@ class CRM_Import_DataSource_CsvTest extends CiviUnitTestCase {
         'name' => __DIR__ . '/' . $csvFileName,
       ],
       'skipColumnHeader' => TRUE,
+      'contactType' => CRM_Import_Parser::CONTACT_INDIVIDUAL,
     ]);
     $form->buildForm();
     $form->postProcess();