CRM-20790 - ensure relationships are created on import

[jmcclelland]

Overview

When importing records that include relationships, the relationships are not created. The core problem seems to be that the error message returned when a duplicate is found was at one point (or sometimes) returns as a comma separated list of IDs, but now (or sometimes) is returned as an array. The import code doesn't check for the array in all places.

Before

Importing a CSV file listing an individual along with the name of the Employer failed to create the employee/employer relationship between the two contacts. This was true if both contacts existed or one existed (if neither existed than it works fine).

After

Now the import works as expected.

Comments

A better fix might be to track down why we sometimes return an comma separated list of ids and sometimes an array and make it consistent.

---

• CRM-20790: CIVICRM-159 - Import Contacts, Current Employer (Employee of field) is not being imported at all

[agileware]

I have tested this using the attached CSV file and confirm that contacts are now imported, with the "Employee Of" relationship correctly set and the Organisation contact created if it does not already exist.

Note when reviewing the import, must check every imported contact as the bug does randomly set the "Employee Of" relationship for some of the imported contacts, but not all. After the PR is applied the relationship is set consistently for all.

simple-csv-current-employer.txt

Ping @seamuslee001 as this is a pretty serious, user impacting bug can you or another maintainer please review.

[seamuslee001]

ping @eileenmcnaughton this looks fine to me from a code side,, I note that Agileware has tested this. This seems sane to merge. I think you have touched on the import code recently

[eileenmcnaughton]

OK - I think this has had an appropriate amount of review to the change being made (much as I aspire to get @jmcclelland to embrace unit tests) I will merge now

[jmcclelland]

I'm sorry! I embrace them - I really do!! I just don't always have the time and resources to write them. I know it's a time now vs. time later - will work harder on it.

[eileenmcnaughton]

Once you get used to them they actually can speed up fixing things. Less UI testing!

[agileware]

@eileenmcnaughton @jmcclelland if someone could provide a bit of guidance about how to write a unit test for an import process, I'd be happy to assign someone on team the task of writing it. Agree this would be extremely useful and it is important that this functionality does not break.

Are there any other unit tests available as examples which deal with the CiviCRM import process?

[eileenmcnaughton]

@agileware Now there is something I'm happy to help with! Great offer. I think the best starting point is

CRM_Contact_Imports_Parser_ContactTest class - there are a few others in
CRM_Contact_Imports_Form folder (looks like the class is misnamed & should have Form in the name :-()

Note the import class needs a LOT of refactoring.I would encourage your dev to do a small piece of refactoring in their commit as well, nothing ambitious, just extract a long chunk of code into a function or something like that, as part of de-toxing that code