From f17818904638668404d985cdfe885150854cdb6f Mon Sep 17 00:00:00 2001 From: eileen Date: Tue, 26 May 2020 13:26:29 +1200 Subject: [PATCH] [REF] Code simplification & unit test on suppressed emails in task --- CRM/Contact/Form/Task/EmailTrait.php | 56 +++++++++---------- .../CRM/Contact/Form/Task/EmailCommonTest.php | 18 ++++++ 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/CRM/Contact/Form/Task/EmailTrait.php b/CRM/Contact/Form/Task/EmailTrait.php index b0b140207d..a20843c587 100644 --- a/CRM/Contact/Form/Task/EmailTrait.php +++ b/CRM/Contact/Form/Task/EmailTrait.php @@ -79,6 +79,15 @@ trait CRM_Contact_Form_Task_EmailTrait { public $contactEmails = []; + /** + * Contacts form whom emails could not be sent. + * + * An array of contact ids and the relevant message details. + * + * @var array + */ + protected $suppressedEmails = []; + /** * Getter for isSearchContext. * @@ -208,31 +217,24 @@ trait CRM_Contact_Form_Task_EmailTrait { ]; // get the details for all selected contacts ( to, cc and bcc contacts ) - list($this->_contactDetails) = CRM_Utils_Token::getTokenDetails($this->_allContactIds, + list($this->_allContactDetails) = CRM_Utils_Token::getTokenDetails($this->_allContactIds, $returnProperties, FALSE, FALSE ); - // make a copy of all contact details - $this->_allContactDetails = $this->_contactDetails; - // perform all validations on unique contact Ids foreach (array_unique($this->_allContactIds) as $key => $contactId) { - $value = $this->_contactDetails[$contactId]; + $value = $this->_allContactDetails[$contactId]; if ($value['do_not_email'] || empty($value['email']) || !empty($value['is_deceased']) || $value['on_hold']) { - $suppressedEmails++; - - // unset contact details for contacts that we won't be sending email. This is prevent extra computation - // during token evaluation etc. - unset($this->_contactDetails[$contactId]); + $this->setSuppressedEmail($contactId, $value); } else { $email = $value['email']; // build array's which are used to setdefaults if (in_array($contactId, $this->_toContactIds)) { - $this->_toContactDetails[$contactId] = $this->_contactDetails[$contactId]; + $this->_toContactDetails[$contactId] = $this->_contactDetails[$contactId] = $this->_allContactDetails[$contactId]; // If a particular address has been specified as the default, use that instead of contact's primary email if (!empty($this->_toEmail) && $this->_toEmail['contact_id'] == $contactId) { $email = $this->_toEmail['email']; @@ -252,7 +254,7 @@ trait CRM_Contact_Form_Task_EmailTrait { $this->assign('toContact', json_encode($toArray)); - $this->assign('suppressedEmails', $suppressedEmails); + $this->assign('suppressedEmails', count($this->suppressedEmails)); $this->assign('totalSelectedContacts', count($this->_contactIds)); @@ -494,23 +496,10 @@ trait CRM_Contact_Form_Task_EmailTrait { ]) . $followupStatus, ts('Message Sent', ['plural' => 'Messages Sent', 'count' => $count_success]), 'success'); } - // Display the name and number of contacts for those email is not sent. - // php 5.4 throws out a notice since the values of these below arrays are arrays. - // the behavior is not documented in the php manual, but it does the right thing - // suppressing the notices to get things in good shape going forward - $emailsNotSent = @array_diff_assoc($this->_allContactDetails, $this->_contactDetails); - - if ($emailsNotSent) { - $not_sent = []; - foreach ($emailsNotSent as $contactId => $values) { - $displayName = $values['display_name']; - $email = $values['email']; - $contactViewUrl = CRM_Utils_System::url('civicrm/contact/view', "reset=1&cid=$contactId"); - $not_sent[] = "$displayName" . ($values['on_hold'] ? '(' . ts('on hold') . ')' : ''); - } - $status = '(' . ts('because no email address on file or communication preferences specify DO NOT EMAIL or Contact is deceased or Primary email address is On Hold') . ')'; + if (!empty($this->suppressedEmails)) { + $status = '(' . ts('because no email address on file or communication preferences specify DO NOT EMAIL or Contact is deceased or Primary email address is On Hold') . ')'; CRM_Core_Session::setStatus($status, ts('One Message Not Sent', [ - 'count' => count($emailsNotSent), + 'count' => count($this->suppressedEmails), 'plural' => '%count Messages Not Sent', ]), 'info'); } @@ -633,4 +622,15 @@ trait CRM_Contact_Form_Task_EmailTrait { return $urlString; } + /** + * Set the emails that are not to be sent out. + * + * @param int $contactID + * @param array $values + */ + protected function setSuppressedEmail($contactID, $values) { + $contactViewUrl = CRM_Utils_System::url('civicrm/contact/view', 'reset=1&cid=' . $contactID); + $this->suppressedEmails[$contactID] = "{$values['display_name']}" . ($values['on_hold'] ? '(' . ts('on hold') . ')' : ''); + } + } diff --git a/tests/phpunit/CRM/Contact/Form/Task/EmailCommonTest.php b/tests/phpunit/CRM/Contact/Form/Task/EmailCommonTest.php index 0064223601..65744e848e 100644 --- a/tests/phpunit/CRM/Contact/Form/Task/EmailCommonTest.php +++ b/tests/phpunit/CRM/Contact/Form/Task/EmailCommonTest.php @@ -89,6 +89,10 @@ class CRM_Contact_Form_Task_EmailCommonTest extends CiviUnitTestCase { $form->_contactIds[$contactID] = $contactID; $form->_toContactEmails[$this->callAPISuccessGetValue('Email', ['return' => 'id', 'email' => $email])] = $email; } + $deceasedContactID = $this->individualCreate(['is_deceased' => 1, 'email' => 'dead@example.com']); + $form->_contactIds[$deceasedContactID] = $deceasedContactID; + $form->_toContactEmails[$this->callAPISuccessGetValue('Email', ['return' => 'id', 'email' => 'dead@example.com'])] = 'dead@example.com'; + $loggedInEmail = $this->callAPISuccess('Email', 'create', [ 'email' => 'mickey@mouse.com', 'location_type_id' => 1, @@ -119,6 +123,20 @@ class CRM_Contact_Form_Task_EmailCommonTest extends CiviUnitTestCase { $bccUrl1 = CRM_Utils_System::url('civicrm/contact/view', ['reset' => 1, 'force' => 1, 'cid' => $bcc1], TRUE); $bccUrl2 = CRM_Utils_System::url('civicrm/contact/view', ['reset' => 1, 'force' => 1, 'cid' => $bcc2], TRUE); $this->assertContains("bcc : Mr. Anthony Anderson IIMr. Anthony Anderson II", $activity['details']); + $this->assertEquals([ + [ + 'text' => '27 messages were sent successfully. ', + 'title' => 'Messages Sent', + 'type' => 'success', + 'options' => NULL, + ], + [ + 'text' => '(because no email address on file or communication preferences specify DO NOT EMAIL or Contact is deceased or Primary email address is On Hold)', + 'title' => 'One Message Not Sent', + 'type' => 'info', + 'options' => NULL, + ], + ], CRM_Core_Session::singleton()->getStatus()); } } -- 2.25.1