CRM-21574 Allow to disable sending of email from source contact for tell a friend on pcp

[mattwire]

Overview

I've pulled out the NFC changes in this PR into #12355 which should be reviewed/merged first.

When sending emails for personal campaign pages and enabling the "Tell a Friend" functionality it tries to send email from the contact IDs email address. However this is not always desirable as it can break SPF/DMARC and lead to bounced/failed email delivery.

@bgm I think you might be interested in - as it's not something I actually use :-) But thought I'd take a quick look as I needed to for activity assignees

---

• CRM-21574: Allow to disable sending of email from source contact for tell a friend on pcp
• CRM-21244: Enhancements to "FROM email addresses"

[mattwire]

@mlutfy Would you take a look at this when you get a moment? As I know you have an interest.

[mattwire]

Jenkins test this please

[mattwire]

@yashodha Would you have time to look at this? It's the last piece of the emails from source contact stuff.

[stesi561]

Hey @mattwire, spotted your msg about this on chat and got inspired to try a review.

I'm getting an error when "Allow Mail to be sent from logged in contact's email address" is unticked.

Undefined index: email_from in CRM_Friend_BAO_Friend::sendMail() (line 328 of CRM/Friend/BAO/Friend.php)

My reading is this is not an issue with the patch - but that the previous behaviour was masking the issue. Before the patch the test for an empty value in $values['email_from'] was not wrapped in the conditional so this error didn't occur.

I think if we add an else on line 195 that catches the case for a contribution page where the receipt email is not set that inserts a sensible from address value?

The other issue I think we might need to think about with this is that while this changes what happens in the code - what is displayed to a user is no longer accurate - I think we should probably update the template at the same time so that the "from" shows where the email will actually come from.

[stesi561]

I assume grabbing the default that would apply for non events/contribution pages is a sensible option so would be something like:

if (!empty($default['is_email_receipt']) && !empty($default['receipt_from_email'])) {
        $mailParams['email_from'] = $default['receipt_from_email'];
} 
// Addition follows
else {
       $mailParams['email_from'] = $domainDetails['1'];
}

[mattwire]

@stesi561 Thanks for the review. I've pushed an updated patch which should fix the issue you found (and it also adds some other cleanup on that class). If you get a chance would you mind reviewing once again?

[eileenmcnaughton]

@mattwire clean up PR merged now - closing this - please re-open / open anew when rebased etc