From 52f4ecb2139597b15daef263c03adb200a7aa6b8 Mon Sep 17 00:00:00 2001 From: Seamus Lee Date: Tue, 26 May 2020 15:36:28 +1000 Subject: [PATCH] [REF] Use CRM_Utils_Mail::send for sending emails for confirming unsubscribe resubscribe auto replies and subscribing Fix sending issues found by matt and expand unit test to cover sending of subscription emails --- CRM/Mailing/Event/BAO/Reply.php | 31 +++++++++------------ CRM/Mailing/Event/BAO/Resubscribe.php | 32 +++++++++------------- CRM/Mailing/Event/BAO/Subscribe.php | 31 +++++++++------------ CRM/Mailing/Event/BAO/Unsubscribe.php | 33 +++++++++-------------- CRM/Utils/Mail.php | 4 ++- tests/phpunit/api/v3/MailingGroupTest.php | 20 ++++++++++++++ 6 files changed, 71 insertions(+), 80 deletions(-) diff --git a/CRM/Mailing/Event/BAO/Reply.php b/CRM/Mailing/Event/BAO/Reply.php index 25fb8f4fff..ed240b720f 100644 --- a/CRM/Mailing/Event/BAO/Reply.php +++ b/CRM/Mailing/Event/BAO/Reply.php @@ -227,17 +227,15 @@ class CRM_Mailing_Event_BAO_Reply extends CRM_Mailing_Event_DAO_Reply { $component->id = $mailing->reply_id; $component->find(TRUE); - $message = new Mail_Mime("\n"); - $domain = CRM_Core_BAO_Domain::getDomain(); list($domainEmailName, $_) = CRM_Core_BAO_Domain::getNameAndEmail(); - $headers = [ - 'Subject' => $component->subject, - 'To' => $to, - 'From' => "\"$domainEmailName\" <" . CRM_Core_BAO_Domain::getNoReplyEmailAddress() . '>', - 'Reply-To' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), - 'Return-Path' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), + $params = [ + 'subject' => $component->subject, + 'toEmail' => $to, + 'from' => "\"$domainEmailName\" <" . CRM_Core_BAO_Domain::getNoReplyEmailAddress() . '>', + 'replyTo' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), + 'returnPath' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), ]; // TODO: do we need reply tokens? @@ -257,24 +255,19 @@ class CRM_Mailing_Event_BAO_Reply extends CRM_Mailing_Event_DAO_Reply { if ($eq->format == 'HTML' || $eq->format == 'Both') { $html = CRM_Utils_Token::replaceDomainTokens($html, $domain, TRUE, $tokens['html']); $html = CRM_Utils_Token::replaceMailingTokens($html, $mailing, NULL, $tokens['html']); - $message->setHTMLBody($html); } if (!$html || $eq->format == 'Text' || $eq->format == 'Both') { $text = CRM_Utils_Token::replaceDomainTokens($text, $domain, FALSE, $tokens['text']); $text = CRM_Utils_Token::replaceMailingTokens($text, $mailing, NULL, $tokens['text']); - $message->setTxtBody($text); } + $params['html'] = $html; + $params['text'] = $text; - $b = CRM_Utils_Mail::setMimeParams($message); - $h = $message->headers($headers); - CRM_Mailing_BAO_Mailing::addMessageIdHeader($h, 'a', $eq->job_id, queue_id, $eq->hash); - - $mailer = \Civi::service('pear_mail'); - if (is_object($mailer)) { - $errorScope = CRM_Core_TemporaryErrorScope::ignoreException(); - $mailer->send($to, $h, $b); - unset($errorScope); + CRM_Mailing_BAO_Mailing::addMessageIdHeader($params, 'a', $eq->job_id, queue_id, $eq->hash); + if (CRM_Core_BAO_MailSettings::includeMessageId()) { + $params['messageId'] = $params['Message-ID']; } + CRM_Utils_Mail::send($params); } /** diff --git a/CRM/Mailing/Event/BAO/Resubscribe.php b/CRM/Mailing/Event/BAO/Resubscribe.php index e13ee159b6..3468c533b1 100644 --- a/CRM/Mailing/Event/BAO/Resubscribe.php +++ b/CRM/Mailing/Event/BAO/Resubscribe.php @@ -229,8 +229,6 @@ class CRM_Mailing_Event_BAO_Resubscribe { } } - $message = new Mail_mime("\n"); - list($addresses, $urls) = CRM_Mailing_BAO_Mailing::getVerpAndUrls($job, $queue_id, $eq->hash, $eq->email); $bao = new CRM_Mailing_BAO_Mailing(); $bao->body_text = $text; @@ -241,34 +239,28 @@ class CRM_Mailing_Event_BAO_Resubscribe { $html = CRM_Utils_Token::replaceResubscribeTokens($html, $domain, $groups, TRUE, $eq->contact_id, $eq->hash); $html = CRM_Utils_Token::replaceActionTokens($html, $addresses, $urls, TRUE, $tokens['html']); $html = CRM_Utils_Token::replaceMailingTokens($html, $dao, NULL, $tokens['html']); - $message->setHTMLBody($html); } if (!$html || $eq->format == 'Text' || $eq->format == 'Both') { $text = CRM_Utils_Token::replaceDomainTokens($text, $domain, TRUE, $tokens['text']); $text = CRM_Utils_Token::replaceResubscribeTokens($text, $domain, $groups, FALSE, $eq->contact_id, $eq->hash); $text = CRM_Utils_Token::replaceActionTokens($text, $addresses, $urls, FALSE, $tokens['text']); $text = CRM_Utils_Token::replaceMailingTokens($text, $dao, NULL, $tokens['text']); - $message->setTxtBody($text); } - $headers = [ - 'Subject' => $component->subject, - 'From' => "\"$domainEmailName\" <" . CRM_Core_BAO_Domain::getNoReplyEmailAddress() . '>', - 'To' => $eq->email, - 'Reply-To' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), - 'Return-Path' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), + $params = [ + 'subject' => $component->subject, + 'from' => "\"$domainEmailName\" <" . CRM_Core_BAO_Domain::getNoReplyEmailAddress() . '>', + 'toEmail' => $eq->email, + 'replyTo' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), + 'returnPath' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), + 'html' => $html, + 'text' => $text, ]; - CRM_Mailing_BAO_Mailing::addMessageIdHeader($headers, 'e', $job, $queue_id, $eq->hash); - $b = CRM_Utils_Mail::setMimeParams($message); - $h = $message->headers($headers); - - $mailer = \Civi::service('pear_mail'); - - if (is_object($mailer)) { - $errorScope = CRM_Core_TemporaryErrorScope::ignoreException(); - $mailer->send($eq->email, $h, $b); - unset($errorScope); + CRM_Mailing_BAO_Mailing::addMessageIdHeader($params, 'e', $job, $queue_id, $eq->hash); + if (CRM_Core_BAO_MailSettings::includeMessageId()) { + $params['messageId'] = $params['Message-ID']; } + CRM_Utils_Mail::send($params); } } diff --git a/CRM/Mailing/Event/BAO/Subscribe.php b/CRM/Mailing/Event/BAO/Subscribe.php index 1dfad54a4a..71baa417fe 100644 --- a/CRM/Mailing/Event/BAO/Subscribe.php +++ b/CRM/Mailing/Event/BAO/Subscribe.php @@ -205,12 +205,12 @@ SELECT civicrm_email.id as email_id $component->find(TRUE); - $headers = [ - 'Subject' => $component->subject, - 'From' => "\"{$domainEmailName}\" <{$domainEmailAddress}>", - 'To' => $email, - 'Reply-To' => $confirm, - 'Return-Path' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), + $params = [ + 'subject' => $component->subject, + 'from' => "\"{$domainEmailName}\" <{$domainEmailAddress}>", + 'toEmail' => $email, + 'replyTo' => $confirm, + 'returnPath' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), ]; $url = CRM_Utils_System::url('civicrm/mailing/confirm', @@ -246,24 +246,17 @@ SELECT civicrm_email.id as email_id // render the & entities in text mode, so that the links work $text = str_replace('&', '&', $text); - $message = new Mail_mime("\n"); - - $message->setHTMLBody($html); - $message->setTxtBody($text); - $b = CRM_Utils_Mail::setMimeParams($message); - $h = $message->headers($headers); - CRM_Mailing_BAO_Mailing::addMessageIdHeader($h, 's', + CRM_Mailing_BAO_Mailing::addMessageIdHeader($params, 's', $this->contact_id, $this->id, $this->hash ); - $mailer = \Civi::service('pear_mail'); - - if (is_object($mailer)) { - $errorScope = CRM_Core_TemporaryErrorScope::ignoreException(); - $mailer->send($email, $h, $b); - unset($errorScope); + $params['html'] = $html; + $params['text'] = $text; + if (CRM_Core_BAO_MailSettings::includeMessageId()) { + $params['messageId'] = $params['Message-ID']; } + CRM_Utils_Mail::send($params); } /** diff --git a/CRM/Mailing/Event/BAO/Unsubscribe.php b/CRM/Mailing/Event/BAO/Unsubscribe.php index 920ae7d3bc..ce92b388e2 100644 --- a/CRM/Mailing/Event/BAO/Unsubscribe.php +++ b/CRM/Mailing/Event/BAO/Unsubscribe.php @@ -365,8 +365,6 @@ WHERE email = %2 } } - $message = new Mail_mime("\n"); - list($addresses, $urls) = CRM_Mailing_BAO_Mailing::getVerpAndUrls($job, $queue_id, $eq->hash, $eq->email); $bao = new CRM_Mailing_BAO_Mailing(); $bao->body_text = $text; @@ -377,37 +375,30 @@ WHERE email = %2 $html = CRM_Utils_Token::replaceUnsubscribeTokens($html, $domain, $groups, TRUE, $eq->contact_id, $eq->hash); $html = CRM_Utils_Token::replaceActionTokens($html, $addresses, $urls, TRUE, $tokens['html']); $html = CRM_Utils_Token::replaceMailingTokens($html, $dao, NULL, $tokens['html']); - $message->setHTMLBody($html); } if (!$html || $eq->format == 'Text' || $eq->format == 'Both') { $text = CRM_Utils_Token::replaceDomainTokens($text, $domain, FALSE, $tokens['text']); $text = CRM_Utils_Token::replaceUnsubscribeTokens($text, $domain, $groups, FALSE, $eq->contact_id, $eq->hash); $text = CRM_Utils_Token::replaceActionTokens($text, $addresses, $urls, FALSE, $tokens['text']); $text = CRM_Utils_Token::replaceMailingTokens($text, $dao, NULL, $tokens['text']); - $message->setTxtBody($text); } $emailDomain = CRM_Core_BAO_MailSettings::defaultDomain(); - $headers = [ - 'Subject' => $component->subject, - 'From' => "\"$domainEmailName\" <" . CRM_Core_BAO_Domain::getNoReplyEmailAddress() . '>', - 'To' => $eq->email, - 'Reply-To' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), - 'Return-Path' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), + $params = [ + 'subject' => $component->subject, + 'from' => "\"$domainEmailName\" <" . CRM_Core_BAO_Domain::getNoReplyEmailAddress() . '>', + 'toEmail' => $eq->email, + 'replyTo' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), + 'returnPath' => CRM_Core_BAO_Domain::getNoReplyEmailAddress(), + 'html' => $html, + 'text' => $text, ]; - CRM_Mailing_BAO_Mailing::addMessageIdHeader($headers, 'u', $job, $queue_id, $eq->hash); - - $b = CRM_Utils_Mail::setMimeParams($message); - $h = $message->headers($headers); - - $mailer = \Civi::service('pear_mail'); - - if (is_object($mailer)) { - $errorScope = CRM_Core_TemporaryErrorScope::ignoreException(); - $mailer->send($eq->email, $h, $b); - unset($errorScope); + CRM_Mailing_BAO_Mailing::addMessageIdHeader($params, 'u', $job, $queue_id, $eq->hash); + if (CRM_Core_BAO_MailSettings::includeMessageId()) { + $params['messageId'] = $params['Message-ID']; } + CRM_Utils_Mail::send($params); } /** diff --git a/CRM/Utils/Mail.php b/CRM/Utils/Mail.php index 2b6e5120a7..4312391efc 100644 --- a/CRM/Utils/Mail.php +++ b/CRM/Utils/Mail.php @@ -152,6 +152,8 @@ class CRM_Utils_Mail { * text : text of the message * html : html version of the message * replyTo : reply-to header in the email + * returnpath : email address for bounces to be sent to + * messageId : Message ID for this email mesage * attachments: an associative array of * fullPath : complete pathname to the file * mime_type: mime type of the attachment @@ -223,7 +225,7 @@ class CRM_Utils_Mail { } $headers['Date'] = date('r'); if ($includeMessageId) { - $headers['Message-ID'] = '<' . uniqid('civicrm_', TRUE) . "@$emailDomain>"; + $headers['Message-ID'] = $params['messageId'] ?? '<' . uniqid('civicrm_', TRUE) . "@$emailDomain>"; } if (!empty($params['autoSubmitted'])) { $headers['Auto-Submitted'] = "Auto-Generated"; diff --git a/tests/phpunit/api/v3/MailingGroupTest.php b/tests/phpunit/api/v3/MailingGroupTest.php index ed769640ea..e1b09ea923 100644 --- a/tests/phpunit/api/v3/MailingGroupTest.php +++ b/tests/phpunit/api/v3/MailingGroupTest.php @@ -118,6 +118,18 @@ class api_v3_MailingGroupTest extends CiviUnitTestCase { * Test civicrm_mailing_group_event_subscribe and civicrm_mailing_event_confirm functions - success expected. */ public function testMailerProcess() { + $this->callAPISuccess('MailSettings', 'create', [ + 'domain_id' => 1, + 'name' => "my mail setting", + 'domain' => 'setting.com', + 'localpart' => 'civicrm+', + 'server' => "localhost", + 'username' => 'sue', + 'password' => 'pass', + 'is_default' => 1, + ]); + $mut = new CiviMailUtils($this, TRUE); + Civi::settings()->set('include_message_id', 1); $params = [ 'first_name' => 'Test', 'last_name' => 'Test', @@ -134,6 +146,13 @@ class api_v3_MailingGroupTest extends CiviUnitTestCase { 'time_stamp' => '20101212121212', ]; $result = $this->callAPISuccess('mailing_event_subscribe', 'create', $params); + // Check that subscription email has been sent. + $msgs = $mut->getAllMessages(); + $this->assertCount(1, $msgs, 'Subscription email failed to send'); + $mut->checkMailLog([ + 'Message-ID: assertEquals($result['values'][$result['id']]['contact_id'], $contactID); @@ -147,6 +166,7 @@ class api_v3_MailingGroupTest extends CiviUnitTestCase { $this->callAPISuccess('mailing_event_confirm', 'create', $params); $this->contactDelete($contactID); + Civi::settings()->set('include_message_id', 0); } } -- 2.25.1