From 52bd01f5feb25222e1c14ee1c66874e2acdf1694 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Sat, 30 Apr 2022 15:53:38 +1200 Subject: [PATCH] Fix master-only regression on showing fields for contact type --- CRM/Contact/Import/Form/MapField.php | 9 +-- CRM/Contact/Import/Form/Preview.php | 4 +- CRM/Contact/Import/Parser/Contact.php | 75 +++++++++++++------ CRM/Import/Forms.php | 14 ++++ CRM/Import/ImportProcessor.php | 23 +++++- CRM/Import/Parser.php | 32 +++++++- .../Contact/Import/Form/DataSourceTest.php | 2 + .../CRM/Contact/Import/Form/MapFieldTest.php | 1 + .../CRM/Contact/Import/Parser/ContactTest.php | 36 +++++++-- .../phpunit/CRM/Import/DataSource/CsvTest.php | 1 + 10 files changed, 159 insertions(+), 38 deletions(-) diff --git a/CRM/Contact/Import/Form/MapField.php b/CRM/Contact/Import/Form/MapField.php index 019059bcd1..f26d9264ea 100644 --- a/CRM/Contact/Import/Form/MapField.php +++ b/CRM/Contact/Import/Form/MapField.php @@ -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; diff --git a/CRM/Contact/Import/Form/Preview.php b/CRM/Contact/Import/Form/Preview.php index 5ffbc7c885..2447286886 100644 --- a/CRM/Contact/Import/Form/Preview.php +++ b/CRM/Contact/Import/Form/Preview.php @@ -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'), diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index 5694613ba5..afc29acda2 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -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
\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); diff --git a/CRM/Import/Forms.php b/CRM/Import/Forms.php index 2b2a364fec..08cd4c19ca 100644 --- a/CRM/Import/Forms.php +++ b/CRM/Import/Forms.php @@ -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(); + } + } diff --git a/CRM/Import/ImportProcessor.php b/CRM/Import/ImportProcessor.php index 27508e0aea..52357e2899 100644 --- a/CRM/Import/ImportProcessor.php +++ b/CRM/Import/ImportProcessor.php @@ -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; } diff --git a/CRM/Import/Parser.php b/CRM/Import/Parser.php index f7645c8c9f..598b567b2c 100644 --- a/CRM/Import/Parser.php +++ b/CRM/Import/Parser.php @@ -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; /** diff --git a/tests/phpunit/CRM/Contact/Import/Form/DataSourceTest.php b/tests/phpunit/CRM/Contact/Import/Form/DataSourceTest.php index 2d096185ab..d8f3f1fddf 100644 --- a/tests/phpunit/CRM/Contact/Import/Form/DataSourceTest.php +++ b/tests/phpunit/CRM/Contact/Import/Form/DataSourceTest.php @@ -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', diff --git a/tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php b/tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php index 04114569c1..94920164c1 100644 --- a/tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php +++ b/tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php @@ -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' => [ diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php index 7a0ac492cb..0bd3695705 100644 --- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php +++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php @@ -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; + } + } diff --git a/tests/phpunit/CRM/Import/DataSource/CsvTest.php b/tests/phpunit/CRM/Import/DataSource/CsvTest.php index a549a1bd07..beb103bd44 100644 --- a/tests/phpunit/CRM/Import/DataSource/CsvTest.php +++ b/tests/phpunit/CRM/Import/DataSource/CsvTest.php @@ -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(); -- 2.25.1