From 84be264ee612d101858d16ae78085b8460b8c7b7 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 3 Aug 2018 12:10:30 +1200 Subject: [PATCH] Extract getcount for performance --- CRM/Activity/BAO/Activity.php | 118 ++++++++++++++++++---------------- api/v3/Activity.php | 7 +- 2 files changed, 66 insertions(+), 59 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index af76376b7d..055db5144b 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -671,59 +671,22 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { * - caseId int case ID * - context string page on which selector is build * - activity_type_id int|string the activitiy types we want to restrict by - * @param bool $getCount - * Get count of the activities * - * @return array|int + * @return array * Relevant data object values of open activities * @throws \CiviCRM_API3_Exception */ - public static function getActivities($params, $getCount = FALSE) { + public static function getActivities($params) { $activities = array(); // Activity.Get API params - $activityParams = array( - 'is_deleted' => 0, - 'is_current_revision' => 1, - 'is_test' => 0, - 'contact_id' => CRM_Utils_Array::value('contact_id', $params), - 'return' => array( - 'activity_date_time', - 'source_record_id', - 'source_contact_id', - 'source_contact_name', - 'assignee_contact_id', - 'target_contact_id', - 'target_contact_name', - 'assignee_contact_name', - 'status_id', - 'subject', - 'activity_type_id', - 'activity_type', - 'case_id', - 'campaign_id', - ), - 'check_permissions' => 1, - 'options' => array( - 'offset' => CRM_Utils_Array::value('offset', $params, 0), - ), - ); - - if (!empty($params['activity_status_id'])) { - $activityParams['activity_status_id'] = array('IN' => explode(',', $params['activity_status_id'])); - } - - $activityParams['activity_type_id'] = self::filterActivityTypes($params); + $activityParams = self::getActivityParamsForDashboardFunctions($params); if (!empty($params['rowCount']) && $params['rowCount'] > 0 ) { $activityParams['options']['limit'] = $params['rowCount']; } - // set limit = 0 if we need to fetch the activity count - elseif ($getCount) { - $activityParams['options']['limit'] = 0; - } if (!empty($params['sort'])) { if (is_a($params['sort'], 'CRM_Utils_Sort')) { @@ -736,13 +699,29 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { $activityParams['options']['sort'] = empty($order) ? "activity_date_time DESC" : str_replace('activity_type ', 'activity_type_id.label ', $order); - //TODO : - // 1. we should use Activity.Getcount for fetching count only, but in order to check that - // current logged in user has permission to view Case activities we are performing filtering out those activities from list (see below). - // This logic need to be incorporated in Activity.get definition + $activityParams['return'] = [ + 'activity_date_time', + 'source_record_id', + 'source_contact_id', + 'source_contact_name', + 'assignee_contact_id', + 'target_contact_id', + 'target_contact_name', + 'assignee_contact_name', + 'status_id', + 'subject', + 'activity_type_id', + 'activity_type', + 'case_id', + 'campaign_id', + ]; + foreach (['case_id' => 'CiviCase', 'campaign_id' => 'CiviCampaign'] as $attr => $component) { + if (in_array($component, self::activityComponents())) { + $activityParams['return'][] = $attr; + } + } $result = civicrm_api3('Activity', 'Get', $activityParams); - $enabledComponents = self::activityComponents(); $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); @@ -768,19 +747,11 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { ); foreach ($result['values'] as $id => $activity) { - // skip case activities if CiviCase is not enabled OR those actvities which are - if (!empty($activity['case_id']) && !in_array('CiviCase', $enabledComponents)) { - continue; - } $activities[$id] = array(); - // if count is needed, no need to populate the array list with attributes - if ($getCount) { - continue; - } - $isBulkActivity = (!$bulkActivityTypeID || ($bulkActivityTypeID === $activity['activity_type_id'])); + foreach ($mappingParams as $apiKey => $expectedName) { if (in_array($apiKey, array('assignee_contact_name', 'target_contact_name'))) { $activities[$id][$expectedName] = CRM_Utils_Array::value($apiKey, $activity, array()); @@ -826,7 +797,7 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { $activities[$id]['is_recurring_activity'] = CRM_Core_BAO_RecurringEntity::getParentFor($id, 'civicrm_activity'); } - return $getCount ? count($activities) : $activities; + return $activities; } /** @@ -1198,7 +1169,8 @@ ORDER BY fixed_sort_order * count of activities */ public static function getActivitiesCount($input) { - return self::getActivities($input, TRUE); + $activityParams = self::getActivityParamsForDashboardFunctions($input); + return civicrm_api3('Activity', 'getcount', $activityParams); } /** @@ -2858,6 +2830,40 @@ INNER JOIN civicrm_option_group grp ON ( grp.id = val.option_group_id AND grp.n return FALSE; } + /** + * @param $params + * @return array + */ + protected static function getActivityParamsForDashboardFunctions($params) { + $activityParams = [ + 'is_deleted' => 0, + 'is_current_revision' => 1, + 'is_test' => 0, + 'contact_id' => CRM_Utils_Array::value('contact_id', $params), + 'check_permissions' => 1, + 'options' => [ + 'offset' => CRM_Utils_Array::value('offset', $params, 0), + ], + ]; + + if (!empty($params['activity_status_id'])) { + $activityParams['activity_status_id'] = ['IN' => explode(',', $params['activity_status_id'])]; + } + + $activityParams['activity_type_id'] = self::filterActivityTypes($params); + $enabledComponents = self::activityComponents(); + // @todo - should we move this to activity get api. + foreach ([ + 'case_id' => 'CiviCase', + 'campaign_id' => 'CiviCampaign' + ] as $attr => $component) { + if (!in_array($component, $enabledComponents)) { + $activityParams[$attr] = ['IS NULL' => 1]; + } + } + return $activityParams; + } + /** * Checks if user has permissions to edit inbound e-mails, either bsic info * or both basic information and content. diff --git a/api/v3/Activity.php b/api/v3/Activity.php index 67b01d965f..0a8bf5133d 100644 --- a/api/v3/Activity.php +++ b/api/v3/Activity.php @@ -341,6 +341,10 @@ function civicrm_api3_activity_get($params) { } $activities = _civicrm_api3_basic_get(_civicrm_api3_get_BAO(__FUNCTION__), $params, FALSE, 'Activity', $sql); + if ($options['is_count']) { + return civicrm_api3_create_success($activities, $params, 'Activity', 'get'); + } + if (!empty($params['check_permissions']) && !CRM_Core_Permission::check('view all activities')) { // @todo get this to work at the query level - see contact_id join above. foreach ($activities as $activity) { @@ -349,9 +353,6 @@ function civicrm_api3_activity_get($params) { } } } - if ($options['is_count']) { - return civicrm_api3_create_success($activities, $params, 'Activity', 'get'); - } $activities = _civicrm_api3_activity_get_formatResult($params, $activities, $options); //legacy custom data get - so previous formatted response is still returned too -- 2.25.1