Merge pull request #14238 from civicrm/5.14
authorSeamus Lee <seamuslee001@gmail.com>
Sun, 12 May 2019 02:32:34 +0000 (12:32 +1000)
committerGitHub <noreply@github.com>
Sun, 12 May 2019 02:32:34 +0000 (12:32 +1000)
5.14

CRM/Activity/BAO/Activity.php
CRM/Event/Form/Search.php
CRM/UF/Form/Group.php
api/v3/Activity.php
tests/phpunit/CRM/Activity/BAO/ActivityTest.php

index 8aeadbde9a7928da3f1a7e3c0e5a262df5b603de..56f12e5288fedc56aff4b034db6d47029e44dd54 100644 (file)
@@ -705,7 +705,6 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity {
       'source_contact_id',
       'source_contact_name',
       'assignee_contact_id',
-      'target_contact_id',
       'assignee_contact_name',
       'status_id',
       'subject',
@@ -719,7 +718,7 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity {
         $activityParams['return'][] = $attr;
       }
     }
-    $result = civicrm_api3('Activity', 'Get', $activityParams);
+    $result = civicrm_api3('Activity', 'Get', $activityParams)['values'];
 
     $bulkActivityTypeID = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Bulk Email');
     $allCampaigns = CRM_Campaign_BAO_Campaign::getCampaigns(NULL, NULL, FALSE, FALSE, FALSE, TRUE);
@@ -730,44 +729,83 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity {
       (CRM_Mailing_Info::workflowEnabled() && CRM_Core_Permission::check('create mailings'))
     );
 
+    // @todo - get rid of this & just handle in the array declaration like we do with 'subject' etc.
     $mappingParams = [
-      'id' => 'activity_id',
       'source_record_id' => 'source_record_id',
       'activity_type_id' => 'activity_type_id',
-      'activity_date_time' => 'activity_date_time',
       'status_id' => 'status_id',
-      'subject' => 'subject',
       'campaign_id' => 'campaign_id',
-      'assignee_contact_name' => 'assignee_contact_name',
-      'source_contact_id' => 'source_contact_id',
-      'source_contact_name' => 'source_contact_name',
       'case_id' => 'case_id',
     ];
 
-    foreach ($result['values'] as $id => $activity) {
-
-      $activities[$id] = [];
-
-      $isBulkActivity = (!$bulkActivityTypeID || ($bulkActivityTypeID === $activity['activity_type_id']));
-      $activities[$id]['target_contact_counter'] = count($activity['target_contact_id']);
-      if ($activities[$id]['target_contact_counter']) {
-        try {
-          $activities[$id]['target_contact_name'][$activity['target_contact_id'][0]] = civicrm_api3('Contact', 'getvalue', ['id' => $activity['target_contact_id'][0], 'return' => 'sort_name']);
+    if (empty($result)) {
+      $targetCount = [];
+    }
+    else {
+      $targetCount = CRM_Core_DAO::executeQuery('
+      SELECT activity_id, count(*) as target_contact_count
+      FROM civicrm_activity_contact
+      INNER JOIN civicrm_contact c ON contact_id = c.id AND c.is_deleted = 0
+      WHERE activity_id IN (' . implode(',', array_keys($result)) . ')
+      AND record_type_id = %1
+      GROUP BY activity_id', [
+        1 => [
+          CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_ActivityContact', 'record_type_id', 'Activity Targets'),
+          'Integer'
+        ]
+      ])->fetchAll();
+    }
+    foreach ($targetCount as $activityTarget) {
+      $result[$activityTarget['activity_id']]['target_contact_count'] = $activityTarget['target_contact_count'];
+    }
+    // Iterate through & do basic mappings & determine which ones we want to retrieve target count for.
+    foreach ($result as $id => $activity) {
+      $activities[$id] = [
+        'activity_id' => $activity['id'],
+        'activity_date_time' => CRM_Utils_Array::value('activity_date_time', $activity),
+        'subject' => CRM_Utils_Array::value('subject', $activity),
+        'assignee_contact_name' => CRM_Utils_Array::value('assignee_contact_sort_name', $activity, []),
+        'source_contact_id' => CRM_Utils_Array::value('source_contact_id', $activity),
+        'source_contact_name' => CRM_Utils_Array::value('source_contact_sort_name', $activity),
+      ];
+      $activities[$id]['activity_type_name'] = CRM_Core_PseudoConstant::getName('CRM_Activity_BAO_Activity', 'activity_type_id', $activity['activity_type_id']);
+      $activities[$id]['activity_type'] = CRM_Core_PseudoConstant::getLabel('CRM_Activity_BAO_Activity', 'activity_type_id', $activity['activity_type_id']);
+      $activities[$id]['target_contact_count'] = CRM_Utils_Array::value('target_contact_count', $activity, 0);
+      if (!empty($activity['target_contact_count'])) {
+        $displayedTarget = civicrm_api3('ActivityContact', 'get', [
+          'activity_id' => $id,
+          'check_permissions' => TRUE,
+          'options' => ['limit' => 1],
+          'record_type_id' => 'Activity Targets',
+          'return' => ['contact_id.sort_name', 'contact_id'],
+          'sequential' => 1,
+        ])['values'];
+        if (empty($displayedTarget[0])) {
+          $activities[$id]['target_contact_name'] = [];
         }
-        catch (CiviCRM_API3_Exception $e) {
-          // Really they should have names but a fatal here feels wrong.
-          $activities[$id]['target_contact_name'] = '';
+        else {
+          $activities[$id]['target_contact_name'] = [$displayedTarget[0]['contact_id'] => $displayedTarget[0]['contact_id.sort_name']];
         }
       }
+      if ($activities[$id]['activity_type_name'] === 'Bulk Email') {
+        $bulkActivities[] = $id;
+        // Get the total without permissions being passed but only display names after permissioning.
+        $activities[$id]['recipients'] = ts('(%1 recipients)', [1 => $activities[$id]['target_contact_count']]);
+      }
+    }
+
+    // Eventually this second iteration should just handle the target contacts. It's a bit muddled at
+    // the moment as the bulk activity stuff needs unravelling & test coverage.
+    foreach ($result as $id => $activity) {
+      $isBulkActivity = (!$bulkActivityTypeID || ($bulkActivityTypeID === $activity['activity_type_id']));
       foreach ($mappingParams as $apiKey => $expectedName) {
         if (in_array($apiKey, [
-          'assignee_contact_name',
           'target_contact_name',
         ])) {
-          $activities[$id][$expectedName] = CRM_Utils_Array::value($apiKey, $activity, []);
 
           if ($isBulkActivity) {
-            $activities[$id]['recipients'] = ts('(%1 recipients)', [1 => count($activity['target_contact_name'])]);
+            // @todo  - how is this used? Couldn't we use 'is_bulk' or something clearer?
+            // or the calling function could handle
             $activities[$id]['mailingId'] = FALSE;
             if ($accessCiviMail &&
               ($mailingIDs === TRUE || in_array($activity['source_record_id'], $mailingIDs))
@@ -786,11 +824,9 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity {
           }
         }
         else {
+          // @todo this generic assign could just be handled in array declaration earlier.
           $activities[$id][$expectedName] = CRM_Utils_Array::value($apiKey, $activity);
-          if ($apiKey == 'activity_type_id') {
-            $activities[$id]['activity_type'] = CRM_Core_PseudoConstant::getName('CRM_Activity_BAO_Activity', 'activity_type_id', $activities[$id][$expectedName]);
-          }
-          elseif ($apiKey == 'campaign_id') {
+          if ($apiKey == 'campaign_id') {
             $activities[$id]['campaign'] = CRM_Utils_Array::value($activities[$id][$expectedName], $allCampaigns);
           }
         }
@@ -2525,7 +2561,7 @@ INNER JOIN  civicrm_option_group grp ON (grp.id = option_group_id AND grp.name =
         elseif (!empty($values['recipients'])) {
           $activity['target_contact_name'] = $values['recipients'];
         }
-        elseif (isset($values['target_contact_counter']) && $values['target_contact_counter']) {
+        elseif (isset($values['target_contact_count']) && $values['target_contact_count']) {
           $activity['target_contact_name'] = '';
           $firstTargetName = reset($values['target_contact_name']);
           $firstTargetContactID = key($values['target_contact_name']);
@@ -2542,7 +2578,7 @@ INNER JOIN  civicrm_option_group grp ON (grp.id = option_group_id AND grp.name =
             $activity['target_contact_name'] .= $targetLink;
           }
 
-          if ($extraCount = $values['target_contact_counter'] - 1) {
+          if ($extraCount = $values['target_contact_count'] - 1) {
             $activity['target_contact_name'] .= ";<br />" . "(" . ts('%1 more', [1 => $extraCount]) . ")";
           }
           if ($showContactOverlay) {
index 71e58e1f3fac64417391f98055ade153d66fc8e1..4a61657ee3088da531b5eaf057e08dc03b3514d6 100644 (file)
@@ -170,7 +170,7 @@ class CRM_Event_Form_Search extends CRM_Core_Form_Search {
           $seatClause[] = "( participant.is_test = {$this->_formValues['participant_test']} )";
         }
         if (!empty($this->_formValues['participant_status_id'])) {
-          $seatClause[] = CRM_Contact_BAO_Query::buildClause("participant.status_id", '=', $this->_formValues['participant_status_id'], 'Int');
+          $seatClause[] = CRM_Contact_BAO_Query::buildClause("participant.status_id", 'IN', $this->_formValues['participant_status_id'], 'Int');
           if ($status = CRM_Utils_Array::value('IN', $this->_formValues['participant_status_id'])) {
             $this->_formValues['participant_status_id'] = $status;
           }
index 9a5dfbec8ab874c66cc66f32bf0994b15b0a3ee8..e9bf93e0f6732f6dd69abdc9bb8692d2f15e0da7 100644 (file)
@@ -449,8 +449,8 @@ class CRM_UF_Form_Group extends CRM_Core_Form {
    */
   protected function getOtherModuleString() {
     $otherModules = CRM_Core_BAO_UFGroup::getUFJoinRecord($this->_id, TRUE, TRUE);
+    $otherModuleString = NULL;
     if (!empty($otherModules)) {
-      $otherModuleString = NULL;
       foreach ($otherModules as $key) {
         $otherModuleString .= " [ x ] <label>" . $key . "</label>";
       }
index 89402415080ab20af25349c347c453849845280f..4aa31a9da1baaa80b73205193ca0855023052516 100644 (file)
@@ -644,8 +644,10 @@ function _civicrm_api3_activity_fill_activity_contact_names(&$activities, $param
       'activity_id',
       'record_type_id',
       'contact_id.display_name',
+      'contact_id.sort_name',
       'contact_id',
     ],
+    'options' => ['limit' => 0],
     'check_permissions' => !empty($params['check_permissions']),
   ];
   if (count($activityContactTypes) < 3) {
@@ -658,10 +660,12 @@ function _civicrm_api3_activity_fill_activity_contact_names(&$activities, $param
     if (in_array($recordType, ['target', 'assignee'])) {
       $activities[$activityContact['activity_id']][$recordType . '_contact_id'][] = $contactID;
       $activities[$activityContact['activity_id']][$recordType . '_contact_name'][$contactID] = isset($activityContact['contact_id.display_name']) ? $activityContact['contact_id.display_name'] : '';
+      $activities[$activityContact['activity_id']][$recordType . '_contact_sort_name'][$contactID] = isset($activityContact['contact_id.sort_name']) ? $activityContact['contact_id.sort_name'] : '';
     }
     else {
       $activities[$activityContact['activity_id']]['source_contact_id'] = $contactID;
       $activities[$activityContact['activity_id']]['source_contact_name'] = isset($activityContact['contact_id.display_name']) ? $activityContact['contact_id.display_name'] : '';
+      $activities[$activityContact['activity_id']]['source_contact_sort_name'] = isset($activityContact['contact_id.sort_name']) ? $activityContact['contact_id.sort_name'] : '';
     }
   }
 }
index 71e1e8eac60ecbf8dd9d5186b4714a9353aafcf6..f6a7fed17903d411c3be0c46381f0daaf4b24ea2 100644 (file)
@@ -539,12 +539,12 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase {
       'contact_id' => $contactId,
       'context' => 'activity',
     );
-    foreach (array(CRM_Activity_BAO_Activity::getActivities($params)) as $activities) {
-      //verify target count
-      $this->assertEquals($targetCount, $activities[1]['target_contact_counter']);
-      $this->assertEquals([$targetContactIDs[0] => 'Anderson, Anthony'], $activities[1]['target_contact_name']);
-    }
-
+    $activities = CRM_Activity_BAO_Activity::getActivities($params);
+    //verify target count
+    $this->assertEquals($targetCount, $activities[1]['target_contact_count']);
+    $this->assertEquals([$targetContactIDs[0] => 'Anderson, Anthony'], $activities[1]['target_contact_name']);
+    $this->assertEquals('Anderson, Anthony', $activities[1]['source_contact_name']);
+    $this->assertEquals('Anderson, Anthony', $activities[1]['assignee_contact_name'][4]);
   }
 
   /**
@@ -574,7 +574,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase {
   /**
    * Test getActivities BAO method.
    */
-  public function testGetActivitiesforContactSummary() {
+  public function testGetActivitiesForContactSummary() {
     $this->createTestActivities();
 
     $contactID = 9;
@@ -591,33 +591,37 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase {
     //since we are loading activities from dataset, we know total number of activities for this contact
     // 5 activities, Contact Summary should show all activities
     $count = 5;
-    foreach (array(CRM_Activity_BAO_Activity::getActivities($params)) as $activities) {
-
-      $this->assertEquals($count, count($activities));
-
-      foreach ($activities as $key => $value) {
-        $this->assertEquals($value['subject'], "subject {$key}", 'Verify activity subject is correct.');
+    $activities = CRM_Activity_BAO_Activity::getActivities($params);
+    $this->assertEquals($count, count($activities));
+    foreach ($activities as $key => $value) {
+      $this->assertEquals($value['subject'], "subject {$key}", 'Verify activity subject is correct.');
 
-        if ($key > 8) {
-          $this->assertEquals($value['status_id'], 2, 'Verify all activities are scheduled.');
-        }
-        else {
-          $this->assertEquals($value['status_id'], 1, 'Verify all activities are scheduled.');
-        }
+      if ($key > 8) {
+        $this->assertEquals($value['status_id'], 2, 'Verify all activities are scheduled.');
+      }
+      else {
+        $this->assertEquals($value['status_id'], 1, 'Verify all activities are scheduled.');
+      }
 
-        if ($key > 8) {
-          $this->assertEquals($value['activity_type_id'], 1, 'Verify activity type is correct.');
-        }
-        else {
-          $this->assertEquals($value['activity_type_id'], 2, 'Verify activity type is correct.');
-        }
+      if ($key === 12) {
+        $this->assertEquals($value['activity_type'], 'Bulk Email', 'Verify activity type is correct.');
+        $this->assertEquals('(2 recipients)', $value['recipients']);
+        $targetContactID = key($value['target_contact_name']);
+        // The 2 targets have ids 10 & 11. Since they are not sorted it could be either on some systems.
+        $this->assertTrue(in_array($targetContactID, [10, 11]));
+      }
+      elseif ($key > 8) {
+        $this->assertEquals($value['activity_type_id'], 1, 'Verify activity type is correct.');
+      }
+      else {
+        $this->assertEquals($value['activity_type_id'], 2, 'Verify activity type is correct.');
+      }
 
-        if ($key == 3) {
-          $this->assertArrayHasKey($contactID, $value['target_contact_name']);
-        }
-        elseif ($key == 4) {
-          $this->assertArrayHasKey($contactID, $value['assignee_contact_name']);
-        }
+      if ($key == 3) {
+        $this->assertEquals([$contactID => 'Test Contact ' . $contactID], $value['target_contact_name']);
+      }
+      elseif ($key == 4) {
+        $this->assertArrayHasKey($contactID, $value['assignee_contact_name']);
       }
     }
   }
@@ -1319,6 +1323,8 @@ $text
         dirname(__FILE__) . '/activities_for_dashboard_count.xml'
       )
     );
+    // Make changes to improve variation in php since the xml method is brittle & relies on option values being unchanged.
+    $this->callAPISuccess('Activity', 'create', ['id' => 12, 'activity_type_id' => 'Bulk Email']);
   }
 
 }