Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REF] [Import] Remove another good intention - import 'conflicts' #23380

Merged
merged 1 commit into from
May 5, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented May 5, 2022

Overview

[REF] [Import] Remove another good intention - conflict

There is a lot of handling for the concept of 'conflict' in the import classes - but when I searched I was unable to find a way in which it would be invoked.

I actually couldn't figure out from the text when it should be invoked - what is an email with 'conflicting email addresses'?

CiviCRM has detected %1 records with conflicting email addresses within this data file. If you continue, these records will be skipped. OR, you can download a file with just these problem records - Download Conflicts. Then correct them in the original import file, cancel this import and begin again at step 1.

I've been all over this code and can not find a single place where it is set (but lots of places where it is handled) so am concluding it was a good intention. Also, on balance, I wonder how useful it really is to have different types of errors? Ie we have

  • invalid
  • no_match for mismatched contact subtypes (is that more useful that invalid)
  • conflict (until this is merged)
  • valid
  • parsed address error - this seems like a warning so OK
  • duplicate matches -

Of the above I think no_match is just another type of 'invalid'. Duplicate matches does feel special but I think 'no_match' could be folded into 'invalid'

Before

image

image

After

poof

Technical Details

Note that despite the huge amount of code what it is doing is quite simple - if an attempt to import a row resulted in a return code of CRM_Import_Parser::CONFLICT then the Parser class would output it to a csv file. Knowledge of the existence of the csv file would be passed around and assigned to the template - resulting in access to the csv

If this were to still happen in ANY class it would be the Contact class - but in addition to grepping I've been all over that code - it just doesn't happen

Comments

Oddly these functions propagated out from CRM_Contact_Import_Parser which was the in-between contact parser class folded into CRM_Contact_Import_Parser_Contact in 5.49 - that class says it was imported from svn - but the class doesn't seem to exist in svn over here - https://github.com/civicrm/civicrm-svn

@civibot
Copy link

civibot bot commented May 5, 2022

(Standard links)

@civibot civibot bot added the master label May 5, 2022
There is all sorts of handling for 'conflict' - but a conflict return code is never
generated
@eileenmcnaughton
Copy link
Contributor Author

Just saying ....

image

@eileenmcnaughton eileenmcnaughton changed the title [REF] [Import] Remove another good intention - conflict [REF] [Import] Remove another good intention - import 'conflicts' May 5, 2022
@seamuslee001 seamuslee001 merged commit aa3ca5c into civicrm:master May 5, 2022
@seamuslee001 seamuslee001 deleted the import_conflict branch May 5, 2022 23:06
@eileenmcnaughton
Copy link
Contributor Author

Woohoo - another 400 lines gone - thanks @seamuslee001

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants