dev/mail/15 deal better with spaces in from email address

[michaelmcandrew]

see https://lab.civicrm.org/dev/mail/issues/15

[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).

[eileenmcnaughton]

@michaelmcandrew a) thank you very much & b)regex changes are scary - can we spin the regex into it's own function & add a test that iterates through a few strings?

[seamuslee001]

Also @michaelmcandrew should we go through the databases and run this in an upgrade as well?

[michaelmcandrew]

@eileenmcnaughton - I've extracted it to a function and added a unit test.

@seamuslee001 - I don't think it is worth the risk of introducing a bug to the upgrade script since

1. it is a real edge case that people have two spaces in their from email (and as eileen said, could be argued this is a misconfiguration)
2. we are now santizing on input, and also correcting on output. the only time this would be an issue is if someone had already scheduled an email with a double space email address. they can fix it after upgrading by simply editing the email which will force the value to be recalculated. it will not affect any subsequent emails.

[eileenmcnaughton]

@michaelmcandrew test fail relates

PHP Fatal error: Call to protected method CRM_Admin_Form_Options::sanitizeFromEmailAddress() from context 'CRM_Core_OptionGroupTest' in /home/jenkins/buildkit/build/core-12346-

@michaelmcandrew @seamuslee001 I agree that it is probably enough to fix going forwards. Probably anyone experiencing this error has already been hurt & self-fixed it. We could add a system check but I'm not convinced it's worth the effort. We could also do a str_replace double space for single when actually sending - it might still not be a priority to do that though

[eileenmcnaughton]

@michaelmcandrew next rc gets cut on Wed - would be nice to get this merged since it seems stuck on something trivial

[seamuslee001]

Jenkins re test this please

[seamuslee001]

Change seems fine to myself and Eileen adding merge on pass thanks @michaelmcandrew

[seamuslee001]

Merging as per the tag