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-21574 Allow to disable sending of email from source contact for tell a friend on pcp #12475

Merged

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Jul 14, 2018

Overview

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.

Ref: #11425


@civibot
Copy link

civibot bot commented Jul 14, 2018

(Standard links)

@seamuslee001
Copy link
Contributor

ping @mlutfy Mathieu i think you might know a thing or two on this

@mlutfy
Copy link
Member

mlutfy commented Jul 15, 2018

I tested and it did not seem to work because the email_from would end up empty, then somehow the contact's primary email still gets set.

This fixes the issue for me:

    if (Civi::settings()->get('allow_mail_from_logged_in_contact')) {
      // use contact email, CRM-4963
      if (empty($values['email_from'])) {
        $values['email_from'] = $email;
      }
    }

    // Added this ⏬⏬⏬⏬⏬⏬⏬⏬⏬⏬⏬⏬
    if (empty($values['email_from'])) {
      $domainValues = CRM_Core_BAO_Domain::getNameAndEmail();
      $values['email_from'] = $domainValues[1];
    }

The code later does:

            'from' => "$fromName (via {$values['domain']}) <{$values['email_from']}>",

So this seems safer imho.

@eileenmcnaughton
Copy link
Contributor

@mattwire this one is in your court

@mattwire mattwire force-pushed the CRM-21574_disable_contact_email_tellafriend branch from 14065de to 294e260 Compare July 23, 2018 16:26
@mattwire
Copy link
Contributor Author

@mlutfy Thanks for reviewing, I've updated the PR to reflect the changes you suggested - could you confirm if it is ok now?

@mlutfy
Copy link
Member

mlutfy commented Jul 24, 2018

@mattwire Tested and works great. Merging. Thank you!

@mlutfy mlutfy merged commit 9e82cf1 into civicrm:master Jul 24, 2018
@mattwire mattwire deleted the CRM-21574_disable_contact_email_tellafriend branch September 25, 2018 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants