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

CRM-17859 - import contacts from CSV: sanitise / cleanup non-breaking… #7813

Merged
merged 2 commits into from Apr 12, 2016

Conversation

saurabhbatra96
Copy link
Contributor

*/
function trim_nbsp($string) {
return trim($string, chr(0xC2) . chr(0xA0));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of placing this function in the global scope, I would suggest putting it in this class (or CRM_Utils_String). Or maybe easiest & simplest for now would be to make it an anonymous function and pass it straight into array_map.

@saurabhbatra96
Copy link
Contributor Author

@colemanw I shifted the function to the CRM_Utils_String class.

@colemanw
Copy link
Member

Ok now that it's a general utility function, I'm wondering... does this function do anything other than remove   characters? Would we want it to also remove other whitespace (spaces, line breaks, etc)?

@saurabhbatra96
Copy link
Contributor Author

The "usual" whitespace characters are handled well by the standard trim function. I think if any other peculiar whitespace characters were to be reported later on, those could easily be added to this function.
I did think about using an anonymous function with array_map but decided against it as this support was added in PHP 5.3.

@colemanw
Copy link
Member

FYI Civicrm requires php 5.3+

@saurabhbatra96
Copy link
Contributor Author

Well. Did not know that. In that case I'll shift it to an anonymous function. Neat!

@saurabhbatra96
Copy link
Contributor Author

I believe the test failures are unrelated.

@monishdeb
Copy link
Member

Jenkin, test this please

@eileenmcnaughton
Copy link
Contributor

#test #import I was able to replicate this by creating some html with &nbsp at the end of the email address & copying to an open office spreadsheet & saving as csv. the resulting csv would not import

@eileenmcnaughton
Copy link
Contributor

I can confirm that after applying this patch I was able to complete the import

@eileenmcnaughton eileenmcnaughton merged commit 5dcdc4d into civicrm:master Apr 12, 2016
@eileenmcnaughton
Copy link
Contributor

I had some success replicating this in a unit test - see https://github.com/civicrm/civicrm-core/pull/8113/files

@saurabhbatra96 saurabhbatra96 deleted the CRM-17859 branch April 15, 2016 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants