CRM-17753 - Change ReturnPath header in "Confirm Your Subscription" email

[BorislavZlatanov]

For more info see: https://issues.civicrm.org/jira/browse/CRM-17753

[highfalutin]

Note that this will not result in bounce-processing by CiviCRM. I understand that your concern is about email reputation. I'd suggest instead:

$return_path = CRM_Core_BAO_MailSettings::defaultReturnPath();
if (empty($return_path)) {
  $bounce_address = $localpart . 'b' . "@$emailDomain";
}

[eileenmcnaughton]

@seamuslee001 I feel like this & associated patches you may have an interest in / feel able to confirm that you think this approach is correct (or @xurizaemon ) - this PR seems stalled but if we are clear that @highfalutin suggestion is correct we should get up a replacement pr

Also #10647

[BorislavZlatanov]

@eileenmcnaughton Do you know why a test failed here? The test seems related to payments and not to this PR. I don't know if I have to change anything in the code.

Thanks for your help @eileenmcnaughton , I appreciate your effort in moving things forward with open PRs and the codebase.

[eileenmcnaughton]

test this please

[MailinhVu1]

Hi! I just wanted to check what the status of this is since we would need this for one of our clients (mainly because of gdpr concerns). Could you kindly let me know if there is anything outstanding or if you need any additional resourcing?

[eileenmcnaughton]

Good question @MailinhVu1 have you tested this patch (are you with the same org or a different org to @BorislavZlatanov ) @highfalutin I feel like this addressed your suggestion - do you think it should be merged now?

I'm not hugely up with this stuff - but the change seems consistent with elsewhere - e.g BAO_Mailing::getVerpAndUrlsAndHeaders

    $headers = array(
      'Reply-To' => $verp['reply'],
      'Return-Path' => $verp['bounce'],
      'From' => "\"{$this->from_name}\" <{$this->from_email}>",
      'Subject' => $this->subject,
      'List-Unsubscribe' => "<mailto:{$verp['unsubscribe']}>",
    );

[seamuslee001]

also ping @mfb Mark you might have an interest in this one

[mfb]

I would agree that do-not-reply should be a configurable address. However, we should fix this everywhere it's used:

 % grep -r do-not-reply
CRM/Campaign/BAO/Petition.php:    $replyTo = "do-not-reply@$emailDomain";
CRM/Mailing/Event/BAO/Subscribe.php:      'Return-Path' => "do-not-reply@$emailDomain",
CRM/Mailing/Event/BAO/Reply.php:    $fromEmail = 'do-not-reply@' . CRM_Core_BAO_MailSettings::defaultDomain();
CRM/Mailing/Event/BAO/Reply.php:    $fromEmail = 'do-not-reply@' . CRM_Core_BAO_MailSettings::defaultDomain();
CRM/Mailing/Event/BAO/Confirm.php:      'from' => "\"$domainEmailName\" <do-not-reply@$emailDomain>",
CRM/Mailing/Event/BAO/Confirm.php:      'replyTo' => "do-not-reply@$emailDomain",
CRM/Mailing/Event/BAO/Confirm.php:      'returnPath' => "do-not-reply@$emailDomain",
CRM/Mailing/Event/BAO/Resubscribe.php:      'From' => "\"$domainEmailName\" <do-not-reply@$emailDomain>",
CRM/Mailing/Event/BAO/Resubscribe.php:      'Reply-To' => "do-not-reply@$emailDomain",
CRM/Mailing/Event/BAO/Resubscribe.php:      'Return-Path' => "do-not-reply@$emailDomain",
CRM/Mailing/Event/BAO/Unsubscribe.php:      'From' => "\"$domainEmailName\" <do-not-reply@$emailDomain>",
CRM/Mailing/Event/BAO/Unsubscribe.php:      'Reply-To' => "do-not-reply@$emailDomain",
CRM/Mailing/Event/BAO/Unsubscribe.php:      'Return-Path' => "do-not-reply@$emailDomain",

[mfb]

Digging in more here, wouldn't $localpart . 'b' . "@$emailDomain" be a meaningless address, that would be ignored by the bounce processor? It doesn't have the correct formatting with hash etc., so would end up in the "ignored" IMAP folder?

I think it'd be a good idea for CiviCRM to do bounce handling for transactional email (in addition to bulk email) to keep bounce rates down in case of bots or other abuse. But we'd have to use a correctly-generated bounce address, with the hash saved in civicrm_mailing_event_queue so it can be matched and acted on when a bounce is received.

[eileenmcnaughton]

Ouch - so bounce handling won't kick in - but it would wind up in the email box before the verp-separator (+) ie. sally+b@example.com goes to sally@example.com

@jitendrapurohit how much of a solution does your extension offer here?

[mfb]

I poked around, and looks like it'd be a big undertaking to add bounce handling for non-bulk email, since civicrm_mailing_event_queue is keyed to civicrm_mailing_job. So that would have to be overhauled (or a new event setup for non-bulk mail). But it'd be awesome if we could work on it... :)

One idea for a quick fix for do-not-reply would be to add a setting to the Mail Accounts form, below Email Domain.
Name: "System From address"
Description: "From address to use with system-generated emails (only the part before @)"
Default value: "do-not-reply" (to match how CiviCRM works now, but admin could change it to "sally")

The From and Reply-to headers that now use "do-not-reply" would use this new setting.

The Return-path headers that now use "do-not-reply" would use the Return-Path setting, and if Return-Path setting is empty then fallback to using the From header email address as the Return-path header.

So, nothing should change for existing installs, except Return-path header would use the Return-Path setting and "do-not-reply" would now be configurable.

[jitendrapurohit]

The extension modifies the Return-path of all the transactional email to the bounce address just like CiviMail does. See the code written here.

I think what is added in this PR might be possible with this extension. But it does not change the do-not-reply@.. sender of the email. Also agree on changing the hardcoded email address to be a configurable option.

[eileenmcnaughton]

So I guess it would be good to know if the extension can be used instead of changing core (which is a less risky approach).

If we do want to change through core we should probably create another possible account type in civicrm_mailing_settings for ReturnPath perhaps?

[BorislavZlatanov]

I don't think we have to worry about bounce processing in this situation. My understanding of the point of bounce processing is that you want to remove unreachable/dead/unavailable email addresses from your mailing list. I don't think there is any function the bounce processing serves. It just checks bounced emails for some common errors like "Mailbox over quota", "Mailbox unavailable", etc.

The way the proposed patch works right now, if someone submits your mailing list form and the email with confirmation link bounces, it will go to the mailbox that usually does your bounce processing. Then the bounce processor would process that email, and just delete it with no further action. The person would not be subscribed to your mailing list because they never clicked the confirmation link.

If the Welcome email (the one received after the user confirms their subscription) bounces - although this seems like an unlikely event - then you have a user added to your mailing list who has a potentially inactive mailbox. Even though they just subscribed. If that unlikely event occurs, then on your next bulk mail, the email to that user will bounce and the bounce processor will mark the email address so no future mailing will be sent to the person.

So it seems to me that we're good in terms of a workflow.

@mfb Bounce processing is not done for transactional emails on purpose. If someone makes a donation, for example, you always want to send them an email receipt and invoice, no matter if they're marked as inactive in your mailing list.

[JoeMurray]

I think it is very desirable to get rid of hardcoding do_not_reply in core. Using the domain email as the default is sensible, adding a setting would be nice.

I like the idea of adding bounce processing for transactional emails in an extension. Several years ago JMA added bounce processing for transactional emails to Mandrill Transactional Emails extension as it was needed by various clients. I would put that into a separate issue and PR.

It's a bad idea to delete bounced emails in many installs due to workflows that synchronize data repeatedly from other systems. It's important to retain the email addresses so they don't get added back in and retried. While it might seem to make sense to update the system providing the data, it's not always feasible.

It is not appropriate to send transactional emails to addresses that have opted out or bounced. It can be against the law and it can lower the site's spammer reputation. Workflows to snail mail paper receipt might be appropriate, or to check the bounced email address eg via another email for the contact or by phone.

[eileenmcnaughton]

#12270 is now merged - which brings all the places do-not-reply is used into one function. To progress this I think we need to make it configurable - probably by story do-not-reply address as a type in civicrm_mail_settings -although there might be other ideas.

I'm going to close this for now as it needs extra steps to be reviewable - but once there is a fix we should be able to change all do-not-reply instances in one place now