From 0952020a3389b888bd2e5d72b4e878211e69ba23 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Sat, 4 Sep 2021 10:53:42 +1200 Subject: [PATCH] dev/core#2814 Fix activity:sendEmail to use renderTemplate Note that this does create a situation where more queries could occur but 1) this is only called in core from the email task - which is limited to 50 contacts so it is low-volume 2) this is really the start of a cleanup - there are other places where queries can be removed when we look at the total flow but I feel we need to get some simplification happening first & hence have picked on this focus as the first step (ie removing the calls to replaceContactTokens) 3) this seems to be called in extensions to some extent - and it should still work - there might be more queries but I think the 'support contract' for calling an unsupported function does not include it working in ways not required by core code --- CRM/Activity/BAO/Activity.php | 43 +++---- .../phpunit/CRM/Activity/BAO/ActivityTest.php | 106 ++++++++++++------ 2 files changed, 86 insertions(+), 63 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index d60d201b0c..362f869824 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -1083,7 +1083,6 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { ); } - // call token hook $tokens = []; CRM_Utils_Hook::tokens($tokens); $categories = array_keys($tokens); @@ -1128,25 +1127,9 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { $values = array_merge($values, $details["{$contactId}"]); } - $tokenSubject = CRM_Utils_Token::replaceContactTokens($subject, $values, FALSE, $subjectToken, FALSE, $escapeSmarty); - $tokenSubject = CRM_Utils_Token::replaceHookTokens($tokenSubject, $values, $categories, FALSE, $escapeSmarty); - - // CRM-4539 - if ($values['preferred_mail_format'] == 'Text' || $values['preferred_mail_format'] == 'Both') { - $tokenText = CRM_Utils_Token::replaceContactTokens($text, $values, FALSE, $messageToken, FALSE, $escapeSmarty); - $tokenText = CRM_Utils_Token::replaceHookTokens($tokenText, $values, $categories, FALSE, $escapeSmarty); - } - else { - $tokenText = NULL; - } - - if ($values['preferred_mail_format'] == 'HTML' || $values['preferred_mail_format'] == 'Both') { - $tokenHtml = CRM_Utils_Token::replaceContactTokens($html, $values, TRUE, $messageToken, FALSE, $escapeSmarty); - $tokenHtml = CRM_Utils_Token::replaceHookTokens($tokenHtml, $values, $categories, TRUE, $escapeSmarty); - } - else { - $tokenHtml = NULL; - } + $tokenSubject = $subject; + $tokenText = in_array($values['preferred_mail_format'], ['Both', 'Text'], TRUE) ? $text : ''; + $tokenHtml = in_array($values['preferred_mail_format'], ['Both', 'HTML'], TRUE) ? $html : ''; if ($caseId) { $tokenSubject = CRM_Utils_Token::replaceCaseTokens($caseId, $tokenSubject, $subjectToken, $escapeSmarty); @@ -1154,14 +1137,16 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { $tokenHtml = CRM_Utils_Token::replaceCaseTokens($caseId, $tokenHtml, $messageToken, $escapeSmarty); } - if (defined('CIVICRM_MAIL_SMARTY') && CIVICRM_MAIL_SMARTY) { - // also add the contact tokens to the template - $smarty->assign_by_ref('contact', $values); - - $tokenSubject = $smarty->fetch("string:$tokenSubject"); - $tokenText = $smarty->fetch("string:$tokenText"); - $tokenHtml = $smarty->fetch("string:$tokenHtml"); - } + $renderedTemplate = CRM_Core_BAO_MessageTemplate::renderTemplate([ + 'messageTemplate' => [ + 'msg_text' => $tokenText, + 'msg_html' => $tokenHtml, + 'msg_subject' => $tokenSubject, + ], + 'contactId' => $contactId, + 'disableSmarty' => !CRM_Utils_Constant::value('CIVICRM_MAIL_SMARTY'), + 'tplParams' => ['contact' => $values], + ]); $sent = FALSE; // To minimize storage requirements, only one copy of any file attachments uploaded to CiviCRM is kept, @@ -1171,7 +1156,7 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { } // Create email activity. - $activityID = self::createEmailActivity($userID, $tokenSubject, $tokenHtml, $tokenText, $additionalDetails, $campaignId, $attachments, $caseId); + $activityID = self::createEmailActivity($userID, $renderedTemplate['subject'], $renderedTemplate['html'], $renderedTemplate['text'], $additionalDetails, $campaignId, $attachments, $caseId); $activityIds[] = $activityID; if ($firstActivityCreated == FALSE && !empty($attachments)) { diff --git a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php index a7af6991c7..e737a872dc 100644 --- a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php +++ b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php @@ -1148,7 +1148,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { $sourceContactParams = [ 'first_name' => 'liz', 'last_name' => 'hurleey', - 'email' => substr(sha1(rand()), 0, 7) . '@testemail.com', + 'email' => 'liz@testemail.com', ]; $sourceContactID = $this->individualCreate($sourceContactParams); $sourceDisplayName = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $sourceContactID, 'display_name'); @@ -1156,10 +1156,10 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { // create an activity using API $params = [ 'source_contact_id' => $sourceContactID, - 'subject' => 'Scheduling Meeting ' . substr(sha1(rand()), 0, 4), - 'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Meeting'), + 'subject' => 'Scheduling Meeting', + 'activity_type_id' => 'Meeting', 'assignee_contact_id' => [$assigneeContactId], - 'activity_date_time' => date('Ymd'), + 'activity_date_time' => 'now', ]; $activity = $this->callAPISuccess('Activity', 'create', $params); @@ -1176,7 +1176,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { $sourceContactID = $this->individualCreate($withoutEmailParams); $params = [ 'source_contact_id' => $sourceContactID, - 'subject' => 'Scheduling Meeting ' . substr(sha1(rand()), 0, 4), + 'subject' => 'Scheduling Meeting 2', 'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Meeting'), 'activity_date_time' => date('Ymd'), ]; @@ -1186,11 +1186,11 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { $formAddress = CRM_Case_BAO_Case::getReceiptFrom($activity['id']); if (!empty($domainInfo['from_email'])) { - $expectedFromAddress = sprintf("%s <%s>", $domainInfo['from_name'], $domainInfo['from_email']); + $expectedFromAddress = sprintf('%s <%s>', $domainInfo['from_name'], $domainInfo['from_email']); } // Case 3: fetch default Organization Contact email address elseif (!empty($domainInfo['domain_email'])) { - $expectedFromAddress = sprintf("%s <%s>", $domainInfo['name'], $domainInfo['domain_email']); + $expectedFromAddress = sprintf('%s <%s>', $domainInfo['name'], $domainInfo['domain_email']); } $this->assertEquals($expectedFromAddress, $formAddress); @@ -1655,36 +1655,63 @@ $textValue } } + /** + * Test that smarty is rendered, if enabled. + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ + public function testSmartyEnabled(): void { + putenv('CIVICRM_MAIL_SMARTY=1'); + $this->createLoggedInUser(); + $contactID = $this->individualCreate(['last_name' => 'Red']); + CRM_Activity_BAO_Activity::sendEmail( + [ + $contactID => [ + 'preferred_mail_format' => 'Both', + 'contact_id' => $contactID, + 'email' => 'a@example.com', + ], + ], + '{contact.first_name} {$contact.first_name}', + '{contact.first_name} {$contact.first_name}', + '{contact.first_name} {$contact.first_name}', + NULL, + NULL, + 'mail@example.com', + NULL, + NULL, + NULL, + [$contactID] + ); + $activity = $this->callAPISuccessGetValue('Activity', ['return' => 'details']); + putenv('CIVICRM_MAIL_SMARTY=0'); + } + /** * Same as testSendEmailWillReplaceTokensUniquelyForEachContact but with * 3 recipients and an attachment. + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception */ - public function testSendEmailWillReplaceTokensUniquelyForEachContact3() { + public function testSendEmailWillReplaceTokensUniquelyForEachContact3(): void { $contactId1 = $this->individualCreate(['last_name' => 'Red']); $contactId2 = $this->individualCreate(['last_name' => 'Pink']); $contactId3 = $this->individualCreate(['last_name' => 'Ochre']); // create a logged in USER since the code references it for sendEmail user. - $this->createLoggedInUser(); - $session = CRM_Core_Session::singleton(); - $loggedInUser = $session->get('userID'); + $loggedInUser = $this->createLoggedInUser(); $contact = $this->callAPISuccess('Contact', 'get', ['sequential' => 1, 'id' => ['IN' => [$contactId1, $contactId2, $contactId3]]]); - // Create a campaign. - $result = $this->callAPISuccess('Campaign', 'create', [ - 'version' => $this->_apiversion, - 'title' => __FUNCTION__ . ' campaign', - ]); - $campaign_id = $result['id']; - // Add contact tokens in subject, html , text. $subject = __FUNCTION__ . ' subject' . '{contact.display_name}'; $html = __FUNCTION__ . ' html' . '{contact.display_name}'; - $text = __FUNCTION__ . ' text' . '{contact.display_name}'; - $userID = $loggedInUser; + // Check the smarty doesn't mess stuff up. + $text = ' text' . '{contact.display_name} {$contact.first_name}'; $filepath = Civi::paths()->getPath('[civicrm.files]/custom'); - $fileName = "test_email_create.txt"; + $fileName = 'test_email_create.txt'; $fileUri = "{$filepath}/{$fileName}"; // Create a file. CRM_Utils_File::createFakeFile($filepath, 'aaaaaa', $fileName); @@ -1703,17 +1730,17 @@ $textValue $text, $html, $contact['values'][0]['email'], - $userID, - $from = __FUNCTION__ . '@example.com', + $loggedInUser, + __FUNCTION__ . '@example.com', $attachments, - $cc = NULL, - $bcc = NULL, - $contactIds = array_column($contact['values'], 'id'), - $additionalDetails = NULL, NULL, - $campaign_id + NULL, + array_column($contact['values'], 'id'), + NULL, + NULL, + $this->getCampaignID() ); - $result = $this->callAPISuccess('activity', 'get', ['campaign_id' => $campaign_id]); + $result = $this->callAPISuccess('Activity', 'get', ['campaign_id' => $this->getCampaignID()]); // An activity created for each of the two contacts $this->assertEquals(3, $result['count']); $id = 0; @@ -2570,7 +2597,7 @@ $textValue * Similar to testSendEmailWillReplaceTokensUniquelyForEachContact but we're * checking the activity ids returned from sendEmail. */ - public function testSendEmailWithMultipleToRecipients() { + public function testSendEmailWithMultipleToRecipients(): void { $contactId1 = $this->individualCreate(['first_name' => 'Aaaa', 'last_name' => 'Bbbb']); $contactId2 = $this->individualCreate(['first_name' => 'Cccc', 'last_name' => 'Dddd']); @@ -2592,10 +2619,7 @@ $textValue $attachments = NULL, $cc = NULL, $bcc = NULL, - $contactIds = array_column($contacts['values'], 'id'), - $additionalDetails = NULL, - NULL, - $campaign_id = NULL + array_column($contacts['values'], 'id') ); // Get all activities for these contacts @@ -2663,4 +2687,18 @@ $textValue return $contactDetails; } + /** + * Get a campaign id - creating one if need be. + * + * @return int + */ + protected function getCampaignID() { + if (!isset($this->ids['Campaign'][0])) { + $this->ids['Campaign'][0] = $this->callAPISuccess('Campaign', 'create', [ + 'title' => 'campaign', + ])['id']; + } + return $this->ids['Campaign'][0]; + } + } -- 2.25.1