From 2b810608b9f224f1064e7df98bbfaa6839705ab9 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Fri, 25 Sep 2020 20:45:25 +1200
Subject: [PATCH] Fix MailingJob::writeToDB to handle it's own activityTarget
 wrangling

Per https://github.com/civicrm/civicrm-core/pull/18567 it makes sense for activity to have basic support for activity targets
but form more nuance the function can DIY
---
 CRM/Mailing/BAO/MailingJob.php                | 25 +++++++++----------
 .../phpunit/CRM/Activity/BAO/ActivityTest.php |  4 +--
 .../phpunit/api/v3/JobProcessMailingTest.php  | 10 +++++++-
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/CRM/Mailing/BAO/MailingJob.php b/CRM/Mailing/BAO/MailingJob.php
index 7868dfb85f..39ed78bf40 100644
--- a/CRM/Mailing/BAO/MailingJob.php
+++ b/CRM/Mailing/BAO/MailingJob.php
@@ -8,6 +8,7 @@
  | and copyright information, see https://civicrm.org/licensing       |
  +--------------------------------------------------------------------+
  */
+use Civi\Api4\ActivityContact;
 
 /**
  *
@@ -985,14 +986,11 @@ AND    status IN ( 'Scheduled', 'Running', 'Paused' )
 
       $activity = [
         'source_contact_id' => $mailing->scheduled_id,
-        // CRM-9519
-        'target_contact_id' => array_unique($targetParams),
         'activity_type_id' => $activityTypeID,
         'source_record_id' => $this->mailing_id,
         'activity_date_time' => $job_date,
         'subject' => $mailing->subject,
         'status_id' => 'Completed',
-        'deleteActivityTarget' => FALSE,
         'campaign_id' => $mailing->campaign_id,
       ];
 
@@ -1010,36 +1008,37 @@ AND    civicrm_activity.source_record_id = %2
         2 => [$this->mailing_id, 'Integer'],
       ];
       $activityID = CRM_Core_DAO::singleValueQuery($query, $queryParams);
+      $targetRecordID = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_ActivityContact', 'record_type_id', 'Activity Targets');
 
+      $activityTargets = [];
+      foreach ($targetParams as $id) {
+        $activityTargets[$id] = ['contact_id' => (int) $id];
+      }
       if ($activityID) {
         $activity['id'] = $activityID;
 
         // CRM-9519
         if (CRM_Core_BAO_Email::isMultipleBulkMail()) {
-          static $targetRecordID = NULL;
-          if (!$targetRecordID) {
-            $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');
-            $targetRecordID = CRM_Utils_Array::key('Activity Targets', $activityContacts);
-          }
-
           // make sure we don't attempt to duplicate the target activity
-          foreach ($activity['target_contact_id'] as $key => $targetID) {
+          // @todo - we don't have to do one contact at a time....
+          foreach ($activityTargets as $key => $target) {
             $sql = "
 SELECT id
 FROM   civicrm_activity_contact
 WHERE  activity_id = $activityID
-AND    contact_id = $targetID
+AND    contact_id = {$target['contact_id']}
 AND    record_type_id = $targetRecordID
 ";
             if (CRM_Core_DAO::singleValueQuery($sql)) {
-              unset($activity['target_contact_id'][$key]);
+              unset($activityTargets[$key]);
             }
           }
         }
       }
 
       try {
-        civicrm_api3('Activity', 'create', $activity);
+        $activity = civicrm_api3('Activity', 'create', $activity);
+        ActivityContact::save(FALSE)->setRecords($activityTargets)->setDefaults(['activity_id' => $activity['id'], 'record_type_id' => $targetRecordID])->execute();
       }
       catch (Exception $e) {
         $result = FALSE;
diff --git a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php
index 20848b06d0..d89fd279a9 100644
--- a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php
+++ b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php
@@ -276,9 +276,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase {
   }
 
   /**
-   * Test case for deleteActivityTarget() method.
-   *
-   * deleteActivityTarget($activityId) method deletes activity target for given activity id.
+   * Test case for deleteActivityContact() method.
    */
   public function testDeleteActivityTarget() {
     $contactId = $this->individualCreate();
diff --git a/tests/phpunit/api/v3/JobProcessMailingTest.php b/tests/phpunit/api/v3/JobProcessMailingTest.php
index 98e8438d2d..23153c40e4 100644
--- a/tests/phpunit/api/v3/JobProcessMailingTest.php
+++ b/tests/phpunit/api/v3/JobProcessMailingTest.php
@@ -86,6 +86,7 @@ class api_v3_JobProcessMailingTest extends CiviUnitTestCase {
     //$this->_mut->clearMessages();
     $this->_mut->stop();
     CRM_Utils_Hook::singleton()->reset();
+    $this->quickCleanup(['civicrm_activity_contact', 'civicrm_activity']);
     // DGW
     CRM_Mailing_BAO_MailingJob::$mailsProcessed = 0;
     //$this->cleanupMailingTest();
@@ -474,11 +475,18 @@ class api_v3_JobProcessMailingTest extends CiviUnitTestCase {
    * Check that the earlier iterations's activity targets on the recorded
    * BulkEmail activity don't get wiped out by subsequent iterations when
    * using batches.
+   *
+   * @dataProvider getBooleanDataProvider
+   *
+   * @param bool $isBulk
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function testBatchActivityTargets() {
+  public function testBatchActivityTargets($isBulk) {
     $loggedInUserId = $this->createLoggedInUser();
 
     \Civi::settings()->set('mailerBatchLimit', 2);
+    \Civi::settings()->set('civimail_multiple_bulk_emails', $isBulk);
 
     // We have such small batches we need to lower this interval to get it
     // to create the activity.
-- 
2.25.1