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

diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php
index d60d201b0c12e1ed4ef07fde543a82cfb1eb8f4b..362f86982475a8982c2755168f0be3b7ce76e840 100644 (file)
--- 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 a7af6991c726b6c8b1966d7dd07677cb5414368d..e737a872dc96cdfc87b983a163eb58b988542456 100644 (file)
--- 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];
+  }
+
 }