Handle error when creating related contact

[eileenmcnaughton]

Overview

Handle error when creating related contact

Before

I just triedPer #23643 (comment) doing an import like this where the ext id can't be found causes an e-notice as the exception is not being caught where it should be

After

The e-notice is gone, the code is simplified because a lot of it was duplicating the dedupe lookup already done in processContact

Technical Details

While there is no support currently for calling import()
outside of run() - the intent is to replace run() with something
more sane - which WILL be callable from the outside (likely
via api).

The right behaviour for import() is to catch all errors

Note this also contains a couple of other cleanup PRs - they are minor PRs with no expected behaviour change that can be veried by doing an r-run on 'any' contact import so I figured I should include them for r-run simplicity

The actual commit for this PR is 9bd3911

Comments

@darrick are you able to test this

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[darrick]

I got this to pass. But I don't understand why I had to add the getImportTrackingFields part. As it looks like your tests are passing without that. My branch must be out of sync somewhere. I still have a lot of trouble with that.

diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
index 32ae637ce3..992cbe9129 100644
--- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
+++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
@@ -1973,7 +1973,9 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
     else {
       $dataSource = new CRM_Import_DataSource_SQL($userJobID);
     }
-    $dataSource->initialize();
+    $parser = new CRM_Contact_Import_Parser_Contact();
+    $dataSource->setImportSpecificFields($parser->getImportTrackingFields())->initialize();
+
     return $userJobID;
   }
 
@@ -2120,14 +2122,14 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
       'first_name' => 'Alok',
       'last_name' => 'Patel',
       'Employee of' => 'related external identifier',
-      'organization_name' => 'Big shop',
+     // 'organization_name' => 'Big shop',
     ];
 
     $mapper = [
       ['first_name'],
       ['last_name'],
       ['5_a_b', 'external_identifier'],
-      ['5_a_b', 'organization_name'],
+     // ['5_a_b', 'organization_name'],
     ];
     $fields = array_keys($contactImportValues);
     $values = array_values($contactImportValues);
@@ -2140,7 +2142,7 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
     $parser->init();
 
     $parser->import(CRM_Import_Parser::DUPLICATE_UPDATE, $values);
-    $this->callAPISuccessGetCount('Contact', ['organization_name' => 'Big shop'], 2);
+    $this->callAPISuccessGetCount('Contact', ['organization_name' => 'Big shop'], 0);
   }
 
 }

[eileenmcnaughton]

@darrick if you started testing with the import tracking & then I removed it it might have messed you up!

I think I can turn your diff above into an extra test & then you think this is mergeable? (My preference is to get stuff merged before the rc is cut then if we find any issues we will be doing it on saner code :-)

[darrick]

@eileenmcnaughton sounds good. And true when I step through dedupe is always called twice for each contact. Getting rid of that should speed things up. I think that is the biggest performance hit. But I'm just guessing.

[eileenmcnaughton]

@darrick I've added your test as a dataprovider

[eileenmcnaughton]

test this please

[eileenmcnaughton]

@seamuslee001 did this just fail on the package issue?

We have a gitlab for that but I didn't expect it to affect tests?

https://test.civicrm.org/job/CiviCRM-Core-PR/49362/consoleFull

[eileenmcnaughton]

@seamuslee001 @colemanw @demeritcowboy is one of you able to merge this based on the collaboration with @darrick ?

[eileenmcnaughton]

merged via another PR - @darrick if you can focus your testing on master now it has this fix