Convert bcc field to use an entity reference.

Note I've restricted to the bcc field for now as having  it different makes it easier to do comparitive
testing. The other fields will follow once this is merged

diff --git a/CRM/Contact/Form/Task/EmailTrait.php b/CRM/Contact/Form/Task/EmailTrait.php
index fd24e0423c845687c46c7271dbb814a8b3cc6992..0db21dbdf63dd15ace40754750fbee0a84521dea 100644 (file)
--- a/CRM/Contact/Form/Task/EmailTrait.php
+++ b/CRM/Contact/Form/Task/EmailTrait.php
@@ -15,6 +15,8 @@
  * @copyright CiviCRM LLC https://civicrm.org/licensing
  */
 
+use Civi\Api4\Email;
+
 /**
  * This class provides the common functionality for tasks that send emails.
  */
@@ -75,6 +77,8 @@ trait CRM_Contact_Form_Task_EmailTrait {
    */
   public $isSearchContext = TRUE;
 
+  public $contactEmails = [];
+
   /**
    * Getter for isSearchContext.
    *
@@ -135,7 +139,7 @@ trait CRM_Contact_Form_Task_EmailTrait {
     $this->assign('suppressForm', FALSE);
     $this->assign('emailTask', TRUE);
 
-    $toArray = $ccArray = $bccArray = [];
+    $toArray = $ccArray = [];
     $suppressedEmails = 0;
     //here we are getting logged in user id as array but we need target contact id. CRM-5988
     $cid = $this->get('cid');
@@ -152,7 +156,11 @@ trait CRM_Contact_Form_Task_EmailTrait {
     ];
     $to = $this->add('text', 'to', ts('To'), $emailAttributes, TRUE);
     $cc = $this->add('text', 'cc_id', ts('CC'), $emailAttributes);
-    $bcc = $this->add('text', 'bcc_id', ts('BCC'), $emailAttributes);
+
+    $this->addEntityRef('bcc_id', ts('BCC'), [
+      'entity' => 'Email',
+      'multiple' => TRUE,
+    ]);
 
     if ($to->getValue()) {
       $this->_toContactIds = $this->_contactIds = [];
@@ -162,7 +170,7 @@ trait CRM_Contact_Form_Task_EmailTrait {
       $setDefaults = FALSE;
     }
 
-    $elements = ['to', 'cc', 'bcc'];
+    $elements = ['to', 'cc'];
     $this->_allContactIds = $this->_toContactIds = $this->_contactIds;
     foreach ($elements as $element) {
       if ($$element->getValue()) {
@@ -180,10 +188,6 @@ trait CRM_Contact_Form_Task_EmailTrait {
               case 'cc':
                 $this->_ccContactIds[] = $contactId;
                 break;
-
-              case 'bcc':
-                $this->_bccContactIds[] = $contactId;
-                break;
             }
 
             $this->_allContactIds[] = $contactId;
@@ -253,12 +257,6 @@ trait CRM_Contact_Form_Task_EmailTrait {
               'id' => "$contactId::{$email}",
             ];
           }
-          elseif (in_array($contactId, $this->_bccContactIds)) {
-            $bccArray[] = [
-              'text' => '"' . $value['sort_name'] . '" <' . $email . '>',
-              'id' => "$contactId::{$email}",
-            ];
-          }
         }
       }
 
@@ -269,7 +267,6 @@ trait CRM_Contact_Form_Task_EmailTrait {
 
     $this->assign('toContact', json_encode($toArray));
     $this->assign('ccContact', json_encode($ccArray));
-    $this->assign('bccContact', json_encode($bccArray));
 
     $this->assign('suppressedEmails', $suppressedEmails);
 
@@ -403,6 +400,7 @@ trait CRM_Contact_Form_Task_EmailTrait {
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    * @throws \Civi\API\Exception\UnauthorizedException
+   * @throws \API_Exception
    */
   public function submit($formValues) {
     $this->saveMessageTemplate($formValues);
@@ -415,9 +413,9 @@ trait CRM_Contact_Form_Task_EmailTrait {
     $subject = $formValues['subject'];
 
     // CRM-13378: Append CC and BCC information at the end of Activity Details and format cc and bcc fields
-    $elements = ['cc_id', 'bcc_id'];
+    $elements = ['cc_id'];
     $additionalDetails = NULL;
-    $ccValues = $bccValues = [];
+    $ccValues = [];
     foreach ($elements as $element) {
       if (!empty($formValues[$element])) {
         $allEmails = explode(',', $formValues[$element]);
@@ -430,24 +428,19 @@ trait CRM_Contact_Form_Task_EmailTrait {
               $ccValues['details'][] = "<a href='{$contactURL}'>" . $this->_contactDetails[$contactId]['display_name'] . "</a>";
               break;
 
-            case 'bcc_id':
-              $bccValues['email'][] = '"' . $this->_contactDetails[$contactId]['sort_name'] . '" <' . $email . '>';
-              $bccValues['details'][] = "<a href='{$contactURL}'>" . $this->_contactDetails[$contactId]['display_name'] . "</a>";
-              break;
           }
         }
       }
     }
+    $cc = '';
 
-    $cc = $bcc = '';
     if (!empty($ccValues)) {
       $cc = implode(',', $ccValues['email']);
       $additionalDetails .= "\ncc : " . implode(", ", $ccValues['details']);
     }
-    if (!empty($bccValues)) {
-      $bcc = implode(',', $bccValues['email']);
-      $additionalDetails .= "\nbcc : " . implode(", ", $bccValues['details']);
-    }
+    $bccArray = explode(',', $formValues['bcc_id'] ?? '');
+    $bcc = $this->getEmailString($bccArray);
+    $additionalDetails .= empty($bccArray) ? '' : "\nbcc : " . $this->getEmailUrlString($bccArray);
 
     // CRM-5916: prepend case id hash to CiviCase-originating emails’ subjects
     if (isset($this->_caseId) && is_numeric($this->_caseId)) {
@@ -635,4 +628,48 @@ trait CRM_Contact_Form_Task_EmailTrait {
     return $return;
   }
 
+  /**
+   * Get the string for the email IDs.
+   *
+   * @param array $emailIDs
+   *   Array of email IDs.
+   *
+   * @return string
+   *   e.g. "Smith, Bob<bob.smith@example.com>".
+   *
+   * @throws \API_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
+   */
+  protected function getEmailString(array $emailIDs): string {
+    $emails = Email::get()
+      ->addWhere('id', 'IN', $emailIDs)
+      ->setCheckPermissions(FALSE)
+      ->setSelect(['contact_id', 'email', 'contact.sort_name', 'contact.display_name'])->execute();
+    $emailStrings = [];
+    foreach ($emails as $email) {
+      $this->contactEmails[$email['id']] = $email;
+      $emailStrings[] = '"' . $email['contact.sort_name'] . '" <' . $email['email'] . '>';
+    }
+    return implode(',', $emailStrings);
+  }
+
+  /**
+   * Get the url string.
+   *
+   * This is called after the contacts have been retrieved so we don't need to re-retrieve.
+   *
+   * @param array $emailIDs
+   *
+   * @return string
+   *   e.g. <a href='{$contactURL}'>Bob Smith</a>'
+   */
+  protected function getEmailUrlString(array $emailIDs): string {
+    $urlString = '';
+    foreach ($emailIDs as $email) {
+      $contactURL = CRM_Utils_System::url('civicrm/contact/view', ['reset' => 1, 'force' => 1, 'cid' => $this->contactEmails[$email]['contact_id']], TRUE);
+      $urlString .= "<a href='{$contactURL}'>" . $this->contactEmails[$email]['contact.display_name'] . '</a>';
+    }
+    return $urlString;
+  }
+
 }

diff --git a/CRM/Core/BAO/Email.php b/CRM/Core/BAO/Email.php
index c8bcb650cf88a2c32cf80f57d71473d0be288bf9..3ec6dea97f3ba6d973929f168d2f755878a9f774 100644 (file)
--- a/CRM/Core/BAO/Email.php
+++ b/CRM/Core/BAO/Email.php
@@ -345,4 +345,24 @@ AND    reset_date IS NULL
     return CRM_Contact_BAO_Contact::deleteObjectWithPrimary('Email', $id);
   }
 
+  /**
+   * Get filters for entity reference fields.
+   *
+   * @return array
+   */
+  public static function getEntityRefFilters() {
+    $contactFields = CRM_Contact_BAO_Contact::getEntityRefFilters();
+    foreach ($contactFields as $index => &$contactField) {
+      if (!empty($contactField['entity'])) {
+        // For now email_getlist can't parse state, country etc.
+        unset($contactFields[$index]);
+      }
+      elseif ($contactField['key'] !== 'contact_id') {
+        $contactField['entity'] = 'Contact';
+        $contactField['key'] = 'contact_id.' . $contactField['key'];
+      }
+    }
+    return $contactFields;
+  }
+
 }

diff --git a/api/v3/Email.php b/api/v3/Email.php
index d4c08250d8be79c935d90078f8bf741b0687f1bb..a20bd0dd688ed558937fc448e3959071994f47bc 100644 (file)
--- a/api/v3/Email.php
+++ b/api/v3/Email.php
@@ -91,7 +91,6 @@ function civicrm_api3_email_get($params) {
 function _civicrm_api3_email_getlist_defaults(&$request) {
   return [
     'description_field' => [
-      'contact_id.sort_name',
       'email',
     ],
     'params' => [
@@ -100,6 +99,10 @@ function _civicrm_api3_email_getlist_defaults(&$request) {
       'contact_id.is_deceased' => 0,
       'contact_id.do_not_email' => 0,
     ],
+    // Note that changing this to display name affects query performance. The label field is used
+    // for sorting & mysql will prefer to use the index on the ORDER BY field. So if this is changed
+    // to display name then the filtering will bypass the index. In testing this took around 30 times
+    // as long.
     'label_field' => 'contact_id.sort_name',
     // If no results from sort_name try email.
     'search_field' => 'contact_id.sort_name',

diff --git a/templates/CRM/Contact/Form/Task/Email.tpl b/templates/CRM/Contact/Form/Task/Email.tpl
index 825dfaf8bc44e59678e4e21bd6ec478287bd5f3f..9f0517df2b7f17d8968f79cfadfbf55af59a916b 100644 (file)
--- a/templates/CRM/Contact/Form/Task/Email.tpl
+++ b/templates/CRM/Contact/Form/Task/Email.tpl
@@ -125,12 +125,10 @@ CRM.$(function($) {
 
   {/literal}
   var toContact = {if $toContact}{$toContact}{else}''{/if},
-    ccContact = {if $ccContact}{$ccContact}{else}''{/if},
-    bccContact = {if $bccContact}{$bccContact}{else}''{/if};
+    ccContact = {if $ccContact}{$ccContact}{else}''{/if};
   {literal}
   emailSelect('#to', toContact);
   emailSelect('#cc_id', ccContact);
-  emailSelect('#bcc_id', bccContact);
 });
 
 

diff --git a/tests/phpunit/CRM/Contact/Form/Task/EmailCommonTest.php b/tests/phpunit/CRM/Contact/Form/Task/EmailCommonTest.php
index 10939b6235829cea9b297f9ed15a2dea085201f3..5671eb154b52b925f3c17f1f591823410e1b4b5c 100644 (file)
--- a/tests/phpunit/CRM/Contact/Form/Task/EmailCommonTest.php
+++ b/tests/phpunit/CRM/Contact/Form/Task/EmailCommonTest.php
@@ -8,10 +8,13 @@
  | and copyright information, see https://civicrm.org/licensing       |
  +--------------------------------------------------------------------+
  */
- /**
-  * Test class for CRM_Contact_Form_Task_EmailCommon.
-  * @group headless
-  */
+
+use Civi\Api4\Activity;
+
+/**
+ * Test class for CRM_Contact_Form_Task_EmailCommon.
+ * @group headless
+ */
 class CRM_Contact_Form_Task_EmailCommonTest extends CiviUnitTestCase {
 
   /**
@@ -31,6 +34,16 @@ class CRM_Contact_Form_Task_EmailCommonTest extends CiviUnitTestCase {
     ]);
   }
 
+  /**
+   * Cleanup after test class.
+   *
+   * Make sure the  setting is returned to 'stock'.
+   */
+  public function tearDown() {
+    Civi::settings()->set('allow_mail_from_logged_in_contact', 0);
+    parent::tearDown();
+  }
+
   /**
    * Test generating domain emails
    *
@@ -49,17 +62,27 @@ class CRM_Contact_Form_Task_EmailCommonTest extends CiviUnitTestCase {
   /**
    * Test email uses signature.
    *
+   * @throws \API_Exception
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    * @throws \Civi\API\Exception\UnauthorizedException
    */
   public function testPostProcessWithSignature() {
     $mut = new CiviMailUtils($this, TRUE);
+    $bcc1 = $this->individualCreate(['email' => 'bcc1@example.com']);
+    $bcc2 = $this->individualCreate(['email' => 'bcc2@example.com']);
+    $emails = $this->callAPISuccess('Email', 'getlist', ['input' => 'bcc'])['values'];
+    $bcc  = [];
+    foreach ($emails as $email) {
+      $bcc[] = $email['id'];
+    }
+    $bcc = implode(',', $bcc);
+
     Civi::settings()->set('allow_mail_from_logged_in_contact', 1);
     $loggedInContactID = $this->createLoggedInUser();
-    $form = new CRM_Contact_Form_Task_Email();
-    $_SERVER['REQUEST_METHOD'] = 'GET';
-    $form->controller = new CRM_Core_Controller();
+    /* @var CRM_Contact_Form_Task_Email $form*/
+    $form = $this->getFormObject('CRM_Contact_Form_Task_Email');
+
     for ($i = 0; $i < 27; $i++) {
       $email = 'spy' . $i . '@secretsquirrels.com';
       $contactID = $this->individualCreate(['email' => $email]);
@@ -85,12 +108,16 @@ class CRM_Contact_Form_Task_EmailCommonTest extends CiviUnitTestCase {
     $form->submit(array_merge($form->_defaultValues, [
       'from_email_address' => $loggedInEmail['id'],
       'subject' => 'Really interesting stuff',
+      'bcc_id' => $bcc,
     ]));
     $mut->checkMailLog([
       'This is a test Signature',
     ]);
     $mut->stop();
-    Civi::settings()->set('allow_mail_from_logged_in_contact', 0);
+    $activity = Activity::get()->setCheckPermissions(FALSE)->setSelect(['details'])->execute()->first();
+    $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 : <a href='" . $bccUrl1 . "'>Mr. Anthony Anderson II</a><a href='" . $bccUrl2 . "'>Mr. Anthony Anderson II</a>", $activity['details']);
   }
 
 }