From 6e793248baf0ea5b37ad97a3cac42eae1ec52cdd Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 6 Mar 2019 10:42:16 +1300 Subject: [PATCH] Activity tab performance fix - switch to faster getActivities & getActivitiesCount The getActivitiesCount & getActivities functions are faster & call permission hooks but we weren't able to switch to them until we resolved some performance issues (done) and resolved acl inconsistencies (resolved in 5.12) -we can do this now. From a performance POV the difference is tab-crashes vs tab resolves quickly on contacts with > 10k activities --- CRM/Activity/BAO/Activity.php | 501 +----------------- CRM/Activity/Page/AJAX.php | 6 +- CRM/Activity/Selector/Activity.php | 4 +- CRM/Contact/BAO/Contact.php | 2 +- CRM/Utils/Date.php | 37 ++ templates/CRM/Activity/Selector/Selector.tpl | 6 +- .../phpunit/CRM/Activity/BAO/ActivityTest.php | 85 ++- 7 files changed, 85 insertions(+), 556 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index 4645121311..6235dfde0e 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -858,269 +858,6 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { return array('IN' => array_keys($activityTypes)); } - /** - * Get the list Activities. - * - * @deprecated - * - * @todo - use the api for this - this is working but have temporarily backed out - * due to performance issue to be resolved - CRM-20481. - * - * @param array $input - * Array of parameters. - * Keys include - * - contact_id int contact_id whose activities we want to retrieve - * - offset int which row to start from ? - * - rowCount int how many rows to fetch - * - sort object|array object or array describing sort order for sql query. - * - admin boolean if contact is admin - * - 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 - * - * @return array - * Relevant data object values of open activities - */ - public static function deprecatedGetActivities($input) { - // Step 1: Get the basic activity data. - $bulkActivityTypeID = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', - 'activity_type_id', - 'Bulk Email' - ); - - $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate'); - $sourceID = CRM_Utils_Array::key('Activity Source', $activityContacts); - $assigneeID = CRM_Utils_Array::key('Activity Assignees', $activityContacts); - $targetID = CRM_Utils_Array::key('Activity Targets', $activityContacts); - - $config = CRM_Core_Config::singleton(); - - $activityTempTable = CRM_Utils_SQL_TempTable::build()->setCategory('actdetail')->getName(); - - $tableFields = array( - 'activity_id' => 'int unsigned', - 'activity_date_time' => 'datetime', - 'source_record_id' => 'int unsigned', - 'status_id' => 'int unsigned', - 'subject' => 'varchar(255)', - 'source_contact_name' => 'varchar(255)', - 'activity_type_id' => 'int unsigned', - 'activity_type' => 'varchar(128)', - 'case_id' => 'int unsigned', - 'case_subject' => 'varchar(255)', - 'campaign_id' => 'int unsigned', - ); - - $sql = "CREATE TEMPORARY TABLE {$activityTempTable} ( "; - $insertValueSQL = $selectColumns = array(); - // The activityTempTable contains the sorted rows - // so in order to maintain the sort order as-is we add an auto_increment - // field; we can sort by this later to ensure the sort order stays correct. - $sql .= " fixed_sort_order INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,"; - foreach ($tableFields as $name => $desc) { - $sql .= "$name $desc,\n"; - $insertValueSQL[] = $name; - if ($name == 'source_contact_name' && CRM_Utils_SQL::supportsFullGroupBy()) { - $selectColumns[] = "ANY_VALUE(tbl.$name)"; - } - else { - $selectColumns[] = "tbl.$name"; - } - } - - // add unique key on activity_id just to be sure - // this cannot be primary key because we need that for the auto_increment - // fixed_sort_order field - $sql .= " - UNIQUE KEY ( activity_id ) - ) ENGINE=HEAP DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci - "; - - CRM_Core_DAO::executeQuery($sql); - - $insertSQL = "INSERT IGNORE INTO {$activityTempTable} (" . implode(',', $insertValueSQL) . " ) "; - - $order = $limit = $groupBy = ''; - $groupBy = " GROUP BY tbl.activity_id, tbl.activity_type, tbl.case_id, tbl.case_subject "; - - if (!empty($input['sort'])) { - if (is_a($input['sort'], 'CRM_Utils_Sort')) { - $orderBy = $input['sort']->orderBy(); - if (!empty($orderBy)) { - $order = " ORDER BY $orderBy"; - } - } - elseif (trim($input['sort'])) { - $sort = CRM_Utils_Type::escape($input['sort'], 'String'); - $order = " ORDER BY $sort "; - } - } - - if (empty($order)) { - // context = 'activity' in Activities tab. - $order = " ORDER BY tbl.activity_date_time desc "; - } - - if (!empty($input['rowCount']) && - $input['rowCount'] > 0 - ) { - $limit = " LIMIT {$input['offset']}, {$input['rowCount']} "; - } - - $input['count'] = FALSE; - list($sqlClause, $params) = self::deprecatedGetActivitySQLClause($input); - - $query = sprintf("{$insertSQL} \n SELECT DISTINCT %s from ( %s ) \n as tbl ", implode(', ', $selectColumns), $sqlClause); - - // Filter case activities - CRM-5761. - $components = self::activityComponents(); - if (!in_array('CiviCase', $components)) { - $query .= " -LEFT JOIN civicrm_case_activity ON ( civicrm_case_activity.activity_id = tbl.activity_id ) - WHERE civicrm_case_activity.id IS NULL"; - } - - $query = $query . $groupBy . $order . $limit; - - $dao = CRM_Core_DAO::executeQuery($query, $params); - - // step 2: Get target and assignee contacts for above activities - // create temp table for target contacts - $activityContactTempTable = CRM_Utils_SQL_TempTable::build()->setCategory('actcontact')->getName(); - $query = "CREATE TEMPORARY TABLE {$activityContactTempTable} ( - activity_id int unsigned, contact_id int unsigned, record_type_id varchar(16), - contact_name varchar(255), is_deleted int unsigned, counter int unsigned, INDEX index_activity_id( activity_id ) ) - ENGINE=InnoDB DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci"; - - CRM_Core_DAO::executeQuery($query); - - // note that we ignore bulk email for targets, since we don't show it in selector - $query = " -INSERT INTO {$activityContactTempTable} ( activity_id, contact_id, record_type_id, contact_name, is_deleted ) -SELECT ac.activity_id, - ac.contact_id, - ac.record_type_id, - c.sort_name, - c.is_deleted -FROM {$activityTempTable} -INNER JOIN civicrm_activity a ON ( a.id = {$activityTempTable}.activity_id ) -INNER JOIN civicrm_activity_contact ac ON ( ac.activity_id = {$activityTempTable}.activity_id ) -INNER JOIN civicrm_contact c ON c.id = ac.contact_id -WHERE ac.record_type_id != %1 -"; - $params = array(1 => array($targetID, 'Integer')); - CRM_Core_DAO::executeQuery($query, $params); - - $activityFields = array("ac.activity_id", "ac.contact_id", "ac.record_type_id", "c.sort_name", "c.is_deleted"); - $select = CRM_Contact_BAO_Query::appendAnyValueToSelect($activityFields, "ac.activity_id"); - - // for each activity insert one target contact - // if we load all target contacts the performance will suffer a lot for mass-activities. - $query = " -INSERT INTO {$activityContactTempTable} ( activity_id, contact_id, record_type_id, contact_name, is_deleted, counter ) -{$select}, count(ac.contact_id) -FROM {$activityTempTable} -INNER JOIN civicrm_activity a ON ( a.id = {$activityTempTable}.activity_id ) -INNER JOIN civicrm_activity_contact ac ON ( ac.activity_id = {$activityTempTable}.activity_id ) -INNER JOIN civicrm_contact c ON c.id = ac.contact_id -WHERE ac.record_type_id = %1 -GROUP BY ac.activity_id -"; - - CRM_Core_DAO::executeQuery($query, $params); - - // step 3: Combine all temp tables to get final query for activity selector - // sort by the original sort order, stored in fixed_sort_order - $query = " -SELECT {$activityTempTable}.*, - {$activityContactTempTable}.contact_id, - {$activityContactTempTable}.record_type_id, - {$activityContactTempTable}.contact_name, - {$activityContactTempTable}.is_deleted, - {$activityContactTempTable}.counter, - re.parent_id as is_recurring_activity -FROM {$activityTempTable} -INNER JOIN {$activityContactTempTable} on {$activityTempTable}.activity_id = {$activityContactTempTable}.activity_id -LEFT JOIN civicrm_recurring_entity re on {$activityContactTempTable}.activity_id = re.entity_id -ORDER BY fixed_sort_order - "; - - $dao = CRM_Core_DAO::executeQuery($query); - - // CRM-3553, need to check user has access to target groups. - $mailingIDs = CRM_Mailing_BAO_Mailing::mailingACLIDs(); - $accessCiviMail = ( - (CRM_Core_Permission::check('access CiviMail')) || - (CRM_Mailing_Info::workflowEnabled() && - CRM_Core_Permission::check('create mailings')) - ); - - // Get all campaigns. - $allCampaigns = CRM_Campaign_BAO_Campaign::getCampaigns(NULL, NULL, FALSE, FALSE, FALSE, TRUE); - $values = array(); - while ($dao->fetch()) { - $activityID = $dao->activity_id; - $values[$activityID]['activity_id'] = $dao->activity_id; - $values[$activityID]['source_record_id'] = $dao->source_record_id; - $values[$activityID]['activity_type_id'] = $dao->activity_type_id; - $values[$activityID]['activity_type'] = $dao->activity_type; - $values[$activityID]['activity_date_time'] = $dao->activity_date_time; - $values[$activityID]['status_id'] = $dao->status_id; - $values[$activityID]['subject'] = $dao->subject; - $values[$activityID]['campaign_id'] = $dao->campaign_id; - $values[$activityID]['is_recurring_activity'] = $dao->is_recurring_activity; - - if ($dao->campaign_id) { - $values[$activityID]['campaign'] = $allCampaigns[$dao->campaign_id]; - } - - if (empty($values[$activityID]['assignee_contact_name'])) { - $values[$activityID]['assignee_contact_name'] = array(); - } - - if (empty($values[$activityID]['target_contact_name'])) { - $values[$activityID]['target_contact_name'] = array(); - $values[$activityID]['target_contact_counter'] = $dao->counter; - } - - // if deleted, wrap in - if ($dao->is_deleted) { - $dao->contact_name = "{$dao->contact_name}"; - } - - if ($dao->record_type_id == $sourceID && $dao->contact_id) { - $values[$activityID]['source_contact_id'] = $dao->contact_id; - $values[$activityID]['source_contact_name'] = $dao->contact_name; - } - - if (!$bulkActivityTypeID || ($bulkActivityTypeID != $dao->activity_type_id)) { - // build array of target / assignee names - if ($dao->record_type_id == $targetID && $dao->contact_id) { - $values[$activityID]['target_contact_name'][$dao->contact_id] = $dao->contact_name; - } - if ($dao->record_type_id == $assigneeID && $dao->contact_id) { - $values[$activityID]['assignee_contact_name'][$dao->contact_id] = $dao->contact_name; - } - - // case related fields - $values[$activityID]['case_id'] = $dao->case_id; - $values[$activityID]['case_subject'] = $dao->case_subject; - } - else { - $values[$activityID]['recipients'] = ts('(%1 recipients)', array(1 => $dao->counter)); - $values[$activityID]['mailingId'] = FALSE; - if ( - $accessCiviMail && - ($mailingIDs === TRUE || in_array($dao->source_record_id, $mailingIDs)) - ) { - $values[$activityID]['mailingId'] = TRUE; - } - } - } - - return $values; - } - /** * @inheritDoc */ @@ -1210,234 +947,6 @@ ORDER BY fixed_sort_order return civicrm_api3('Activity', 'getcount', $activityParams); } - /** - * Get the activity Count. - * - * @deprecated - * - * @param array $input - * Array of parameters. - * Keys include - * - contact_id int contact_id whose activities we want to retrieve - * - admin boolean if contact is admin - * - caseId int case ID - * - context string page on which selector is build - * - activity_type_id int|string the activity types we want to restrict by - * - * @return int - * count of activities - */ - public static function deprecatedGetActivitiesCount($input) { - $input['count'] = TRUE; - list($sqlClause, $params) = self::deprecatedGetActivitySQLClause($input); - - //filter case activities - CRM-5761 - $components = self::activityComponents(); - if (!in_array('CiviCase', $components)) { - $query = " - SELECT COUNT(DISTINCT(tbl.activity_id)) as count - FROM ( {$sqlClause} ) as tbl -LEFT JOIN civicrm_case_activity ON ( civicrm_case_activity.activity_id = tbl.activity_id ) - WHERE civicrm_case_activity.id IS NULL"; - } - else { - $query = "SELECT COUNT(DISTINCT(activity_id)) as count from ( {$sqlClause} ) as tbl"; - } - - return CRM_Core_DAO::singleValueQuery($query, $params); - } - - /** - * Get the activity sql clause to pick activities. - * - * @deprecated - * - * @param array $input - * Array of parameters. - * Keys include - * - contact_id int contact_id whose activities we want to retrieve - * - admin boolean if contact is admin - * - caseId int case ID - * - context string page on which selector is build - * - count boolean are we interested in the count clause only? - * - activity_type_id int|string the activity types we want to restrict by - * - * @return int - * count of activities - */ - public static function deprecatedGetActivitySQLClause($input) { - $params = array(); - $sourceWhere = $targetWhere = $assigneeWhere = $caseWhere = 1; - - $config = CRM_Core_Config::singleton(); - if (!CRM_Utils_Array::value('admin', $input, FALSE)) { - $sourceWhere = ' ac.contact_id = %1 '; - $caseWhere = ' civicrm_case_contact.contact_id = %1 '; - - $params = array(1 => array($input['contact_id'], 'Integer')); - } - - $commonClauses = array( - "civicrm_option_group.name = 'activity_type'", - "civicrm_activity.is_deleted = 0", - "civicrm_activity.is_current_revision = 1", - "civicrm_activity.is_test= 0", - ); - - if (isset($input['activity_date_relative']) || - (!empty($input['activity_date_low']) || !empty($input['activity_date_high'])) - ) { - list($from, $to) = CRM_Utils_Date::getFromTo( - CRM_Utils_Array::value('activity_date_relative', $input, 0), - CRM_Utils_Array::value('activity_date_low', $input), - CRM_Utils_Array::value('activity_date_high', $input) - ); - $commonClauses[] = sprintf('civicrm_activity.activity_date_time BETWEEN "%s" AND "%s" ', $from, $to); - } - - if (!empty($input['activity_status_id'])) { - $commonClauses[] = sprintf("civicrm_activity.status_id IN (%s)", $input['activity_status_id']); - } - - // Filter on component IDs. - $components = self::activityComponents(); - if (!empty($components)) { - $componentsIn = implode(',', array_keys($components)); - $commonClauses[] = "( civicrm_option_value.component_id IS NULL OR civicrm_option_value.component_id IN ( $componentsIn ) )"; - } - else { - $commonClauses[] = "civicrm_option_value.component_id IS NULL"; - } - - // activity type ID clause - if (!empty($input['activity_type_id'])) { - if (is_array($input['activity_type_id'])) { - foreach ($input['activity_type_id'] as $idx => $value) { - $input['activity_type_id'][$idx] = CRM_Utils_Type::escape($value, 'Positive'); - } - $commonClauses[] = "civicrm_activity.activity_type_id IN ( " . implode(",", $input['activity_type_id']) . " ) "; - } - else { - $activityTypeID = CRM_Utils_Type::escape($input['activity_type_id'], 'Positive'); - $commonClauses[] = "civicrm_activity.activity_type_id = $activityTypeID"; - } - } - - // exclude by activity type clause - if (!empty($input['activity_type_exclude_id'])) { - if (is_array($input['activity_type_exclude_id'])) { - foreach ($input['activity_type_exclude_id'] as $idx => $value) { - $input['activity_type_exclude_id'][$idx] = CRM_Utils_Type::escape($value, 'Positive'); - } - $commonClauses[] = "civicrm_activity.activity_type_id NOT IN ( " . implode(",", $input['activity_type_exclude_id']) . " ) "; - } - else { - $activityTypeID = CRM_Utils_Type::escape($input['activity_type_exclude_id'], 'Positive'); - $commonClauses[] = "civicrm_activity.activity_type_id != $activityTypeID"; - } - } - - $commonClause = implode(' AND ', $commonClauses); - - $includeCaseActivities = FALSE; - if (in_array('CiviCase', $components)) { - $includeCaseActivities = TRUE; - } - - // build main activity table select clause - $sourceSelect = ''; - - $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate'); - $sourceID = CRM_Utils_Array::key('Activity Source', $activityContacts); - $sourceJoin = " -INNER JOIN civicrm_activity_contact ac ON ac.activity_id = civicrm_activity.id -INNER JOIN civicrm_contact contact ON ac.contact_id = contact.id -"; - - if (!$input['count']) { - $sourceSelect = ', - civicrm_activity.activity_date_time, - civicrm_activity.source_record_id, - civicrm_activity.status_id, - civicrm_activity.subject, - contact.sort_name as source_contact_name, - civicrm_option_value.value as activity_type_id, - civicrm_option_value.label as activity_type, - null as case_id, null as case_subject, - civicrm_activity.campaign_id as campaign_id - '; - - $sourceJoin .= " -LEFT JOIN civicrm_activity_contact src ON (src.activity_id = ac.activity_id AND src.record_type_id = {$sourceID} AND src.contact_id = contact.id) -"; - } - - $sourceClause = " - SELECT civicrm_activity.id as activity_id - {$sourceSelect} - from civicrm_activity - left join civicrm_option_value on - civicrm_activity.activity_type_id = civicrm_option_value.value - left join civicrm_option_group on - civicrm_option_group.id = civicrm_option_value.option_group_id - {$sourceJoin} - where - {$sourceWhere} - AND $commonClause - "; - - // Build case clause - // or else exclude Inbound Emails that have been filed on a case. - $caseClause = ''; - - if ($includeCaseActivities) { - $caseSelect = ''; - if (!$input['count']) { - $caseSelect = ', - civicrm_activity.activity_date_time, - civicrm_activity.source_record_id, - civicrm_activity.status_id, - civicrm_activity.subject, - contact.sort_name as source_contact_name, - civicrm_option_value.value as activity_type_id, - civicrm_option_value.label as activity_type, - null as case_id, null as case_subject, - civicrm_activity.campaign_id as campaign_id'; - } - - $caseClause = " - union all - - SELECT civicrm_activity.id as activity_id - {$caseSelect} - from civicrm_activity - inner join civicrm_case_activity on - civicrm_case_activity.activity_id = civicrm_activity.id - inner join civicrm_case on - civicrm_case_activity.case_id = civicrm_case.id - inner join civicrm_case_contact on - civicrm_case_contact.case_id = civicrm_case.id and {$caseWhere} - left join civicrm_option_value on - civicrm_activity.activity_type_id = civicrm_option_value.value - left join civicrm_option_group on - civicrm_option_group.id = civicrm_option_value.option_group_id - {$sourceJoin} - where - {$caseWhere} - AND $commonClause - and ( ( civicrm_case_activity.case_id IS NULL ) OR - ( civicrm_option_value.name <> 'Inbound Email' AND - civicrm_option_value.name <> 'Email' AND civicrm_case_activity.case_id - IS NOT NULL ) - ) - "; - } - - $returnClause = " {$sourceClause} {$caseClause} "; - - return array($returnClause, $params); - } - /** * Send the message to all the contacts. * @@ -2852,6 +2361,7 @@ INNER JOIN civicrm_option_group grp ON (grp.id = option_group_id AND grp.name = 'is_current_revision' => 1, 'is_test' => 0, 'contact_id' => CRM_Utils_Array::value('contact_id', $params), + 'activity_date_time' => CRM_Utils_Array::value('activity_date_time', $params), 'check_permissions' => 1, 'options' => [ 'offset' => CRM_Utils_Array::value('offset', $params, 0), @@ -2919,12 +2429,13 @@ INNER JOIN civicrm_option_group grp ON (grp.id = option_group_id AND grp.name = $activityIcons[$type['value']] = $type['icon']; } } + CRM_Utils_Date::convertFormDateToApiFormat($params, 'activity_date_time', FALSE); // Get contact activities. - $activities = CRM_Activity_BAO_Activity::deprecatedGetActivities($params); + $activities = CRM_Activity_BAO_Activity::getActivities($params); // Add total. - $params['total'] = CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($params); + $params['total'] = CRM_Activity_BAO_Activity::getActivitiesCount($params); // Format params and add links. $contactActivities = array(); @@ -2943,7 +2454,7 @@ INNER JOIN civicrm_option_group grp ON (grp.id = option_group_id AND grp.name = $mask = CRM_Core_Action::mask($permissions); foreach ($activities as $activityId => $values) { - $activity = array(); + $activity = ['source_contact_name' => '', 'target_contact_name' => '']; $activity['DT_RowId'] = $activityId; // Add class to this row if overdue. $activity['DT_RowClass'] = "crm-entity status-id-{$values['status_id']}"; @@ -2961,7 +2472,6 @@ INNER JOIN civicrm_option_group grp ON (grp.id = option_group_id AND grp.name = $activity['activity_type'] = (!empty($activityIcons[$values['activity_type_id']]) ? ' ' : '') . $values['activity_type']; $activity['subject'] = $values['subject']; - $activity['source_contact_name'] = ''; if ($params['contact_id'] == $values['source_contact_id']) { $activity['source_contact_name'] = $values['source_contact_name']; } @@ -2980,7 +2490,6 @@ INNER JOIN civicrm_option_group grp ON (grp.id = option_group_id AND grp.name = $activity['source_contact_name'] = 'n/a'; } - $activity['target_contact_name'] = ''; if (isset($values['mailingId']) && !empty($values['mailingId'])) { $activity['target_contact'] = CRM_Utils_System::href($values['recipients'], 'civicrm/mailing/report/event', diff --git a/CRM/Activity/Page/AJAX.php b/CRM/Activity/Page/AJAX.php index e8be4c76e3..e44818dec6 100644 --- a/CRM/Activity/Page/AJAX.php +++ b/CRM/Activity/Page/AJAX.php @@ -401,9 +401,9 @@ class CRM_Activity_Page_AJAX { 'activity_type_id' => 'Integer', 'activity_type_exclude_id' => 'Integer', 'activity_status_id' => 'String', - 'activity_date_relative' => 'String', - 'activity_date_low' => 'String', - 'activity_date_high' => 'String', + 'activity_date_time_relative' => 'String', + 'activity_date_time_low' => 'String', + 'activity_date_time_high' => 'String', ); $params = CRM_Core_Page_AJAX::defaultSortAndPagerParams(); diff --git a/CRM/Activity/Selector/Activity.php b/CRM/Activity/Selector/Activity.php index 544724ffde..405fd50d02 100644 --- a/CRM/Activity/Selector/Activity.php +++ b/CRM/Activity/Selector/Activity.php @@ -358,7 +358,7 @@ class CRM_Activity_Selector_Activity extends CRM_Core_Selector_Base implements C 'rowCount' => 0, 'sort' => NULL, ); - return CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($params); + return CRM_Activity_BAO_Activity::getActivitiesCount($params); } /** @@ -392,7 +392,7 @@ class CRM_Activity_Selector_Activity extends CRM_Core_Selector_Base implements C 'sort' => $sort, ); $config = CRM_Core_Config::singleton(); - $rows = CRM_Activity_BAO_Activity::deprecatedGetActivities($params); + $rows = CRM_Activity_BAO_Activity::getActivities($params); if (empty($rows)) { return $rows; diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php index 561f60777f..cea1e9da86 100644 --- a/CRM/Contact/BAO/Contact.php +++ b/CRM/Contact/BAO/Contact.php @@ -2734,7 +2734,7 @@ AND civicrm_openid.is_primary = 1"; 'caseId' => NULL, 'context' => 'activity', ); - return CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($input); + return CRM_Activity_BAO_Activity::getActivitiesCount($input); case 'mailing': $params = array('contact_id' => $contactId); diff --git a/CRM/Utils/Date.php b/CRM/Utils/Date.php index 4223ab35ef..e1fa77bcf4 100644 --- a/CRM/Utils/Date.php +++ b/CRM/Utils/Date.php @@ -2170,4 +2170,41 @@ class CRM_Utils_Date { return $month; } + + /** + * Convert a relative date format to an api field. + * + * @param array $params + * @param string $dateField + * @param bool $isDatePicker + * Non datepicker fields are deprecated. Exterminate Exterminate. + * (but for now handle them). + */ + public static function convertFormDateToApiFormat(&$params, $dateField, $isDatePicker = TRUE) { + if (!empty($params[$dateField . '_relative'])) { + $dates = CRM_Utils_Date::getFromTo($params[$dateField . '_relative'], NULL, NULL); + unset($params[$dateField . '_relative']); + } + if (!empty($params[$dateField . '_low'])) { + $dates[0] = $isDatePicker ? $params[$dateField . '_low'] : date('Y-m-d H:i:s', strtotime($params[$dateField . '_low'])); + unset($params[$dateField . '_low']); + } + if (!empty($params[$dateField . '_high'])) { + $dates[1] = $isDatePicker ? $params[$dateField . '_high'] : date('Y-m-d 23:59:59', strtotime($params[$dateField . '_high'])); + unset($params[$dateField . '_high']); + } + if (empty($dates)) { + return; + } + if (empty($dates[0])) { + $params[$dateField] = ['<=' => $dates[1]]; + } + elseif (empty($dates[1])) { + $params[$dateField] = ['>=' => $dates[0]]; + } + else { + $params[$dateField] = ['BETWEEN' => $dates]; + } + } + } diff --git a/templates/CRM/Activity/Selector/Selector.tpl b/templates/CRM/Activity/Selector/Selector.tpl index d3986fc816..03dd2fc38a 100644 --- a/templates/CRM/Activity/Selector/Selector.tpl +++ b/templates/CRM/Activity/Selector/Selector.tpl @@ -71,9 +71,9 @@ var status_id = $('.crm-activity-selector-' + context + ' select#status_id').val() || []; d.activity_type_id = $('.crm-activity-selector-' + context + ' select#activity_type_filter_id').val(), d.activity_type_exclude_id = $('.crm-activity-selector-' + context + ' select#activity_type_exclude_filter_id').val(), - d.activity_date_relative = $('select#activity_date_relative').val(), - d.activity_date_low = $('#activity_date_low').val(), - d.activity_date_high = $('#activity_date_high').val(), + d.activity_date_time_relative = $('select#activity_date_relative').val(), + d.activity_date_time_low = $('#activity_date_low').val(), + d.activity_date_time_high = $('#activity_date_high').val(), d.activity_status_id = status_id.join(',') } } diff --git a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php index 988148cf62..1921d5ef15 100644 --- a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php +++ b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php @@ -322,8 +322,6 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { */ public function testGetActivitiesCountForAdminDashboard() { $this->setUpForActivityDashboardTests(); - $activityCount = CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($this->_params); - $this->assertEquals(8, $activityCount); $activityCount = CRM_Activity_BAO_Activity::getActivitiesCount($this->_params); $this->assertEquals(8, $activityCount); } @@ -351,12 +349,8 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'sort' => NULL, ); - $activityCount = CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($params); - //since we are loading activities from dataset, we know total number of activities for this contact // 5 activities ( 2 scheduled, 3 Completed ), note that dashboard shows only scheduled activities - $count = 2; - $this->assertEquals($count, $activityCount); $this->assertEquals(2, CRM_Activity_BAO_Activity::getActivitiesCount($params)); } @@ -381,12 +375,9 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'rowCount' => 0, 'sort' => NULL, ); - $activityCount = CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($params); //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; - $this->assertEquals($count, $activityCount); $this->assertEquals(5, CRM_Activity_BAO_Activity::getActivitiesCount($params)); } @@ -455,11 +446,9 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'rowCount' => 0, 'sort' => NULL, ); - $activityCount = CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($params); //since we are loading activities from dataset, we know total number of activities for this contact // this contact does not have any activity - $this->assertEquals(0, $activityCount); $this->assertEquals(0, CRM_Activity_BAO_Activity::getActivitiesCount($params)); } @@ -468,14 +457,13 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { */ public function testGetActivitiesForAdminDashboard() { $this->setUpForActivityDashboardTests(); - $activitiesDeprecatedFn = CRM_Activity_BAO_Activity::deprecatedGetActivities($this->_params); $activitiesNew = CRM_Activity_BAO_Activity::getActivities($this->_params); // $this->assertEquals($activities, $activitiesDeprecatedFn); //since we are loading activities from dataset, we know total number of activities // with no contact ID and there should be 8 schedule activities shown on dashboard $count = 8; - foreach (array($activitiesNew, $activitiesDeprecatedFn) as $activities) { + foreach (array($activitiesNew) as $activities) { $this->assertEquals($count, count($activities)); foreach ($activities as $key => $value) { @@ -492,10 +480,9 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { public function testGetActivitiesForAdminDashboardNoViewContacts() { CRM_Core_Config::singleton()->userPermissionClass->permissions = array('access CiviCRM'); $this->setUpForActivityDashboardTests(); - $activitiesDeprecated = CRM_Activity_BAO_Activity::deprecatedGetActivities($this->_params); - foreach (array($activitiesDeprecated, CRM_Activity_BAO_Activity::getActivities($this->_params)) as $activities) { + foreach (array(CRM_Activity_BAO_Activity::getActivities($this->_params)) as $activities) { // Skipped until we get back to the upgraded version properly. - //$this->assertEquals(0, count($activities)); + $this->assertEquals(0, count($activities)); } } @@ -507,11 +494,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { $this->allowedContacts = array(1, 3, 4, 5); $this->hookClass->setHook('civicrm_aclWhereClause', array($this, 'aclWhereMultipleContacts')); $this->setUpForActivityDashboardTests(); - $activitiesDeprecated = CRM_Activity_BAO_Activity::deprecatedGetActivities($this->_params); - foreach (array($activitiesDeprecated, CRM_Activity_BAO_Activity::getActivities($this->_params)) as $activities) { - //$this->assertEquals(1, count($activities)); - } - + $this->assertEquals(7, count(CRM_Activity_BAO_Activity::getActivities($this->_params))); } /** @@ -537,9 +520,8 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'rowCount' => 0, 'sort' => NULL, ); - $activitiesDep = CRM_Activity_BAO_Activity::deprecatedGetActivities($params); - foreach (array($activitiesDep, CRM_Activity_BAO_Activity::getActivities($params)) as $activities) { + foreach (array(CRM_Activity_BAO_Activity::getActivities($params)) as $activities) { //since we are loading activities from dataset, we know total number of activities for this contact // 5 activities ( 2 scheduled, 3 Completed ), note that dashboard shows only scheduled activities $count = 2; @@ -581,8 +563,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'contact_id' => $contactId, 'context' => 'activity', ); - $activitiesDep = CRM_Activity_BAO_Activity::deprecatedGetActivities($params); - foreach (array($activitiesDep, CRM_Activity_BAO_Activity::getActivities($params)) as $activities) { + 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']); @@ -612,12 +593,11 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'rowCount' => 0, 'sort' => NULL, ); - $activitiesDep = CRM_Activity_BAO_Activity::deprecatedGetActivities($params); //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($activitiesDep, CRM_Activity_BAO_Activity::getActivities($params)) as $activities) { + foreach (array(CRM_Activity_BAO_Activity::getActivities($params)) as $activities) { $this->assertEquals($count, count($activities)); @@ -724,11 +704,10 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { ); foreach ($testCases as $caseName => $testCase) { - $activitiesDep = CRM_Activity_BAO_Activity::deprecatedGetActivities($testCase['params']); - $activityCount = CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($testCase['params']); + $activityCount = CRM_Activity_BAO_Activity::getActivitiesCount($testCase['params']); $activitiesNew = CRM_Activity_BAO_Activity::getActivities($testCase['params']); - foreach (array($activitiesDep, $activitiesNew) as $activities) { + foreach (array($activitiesNew) as $activities) { //$this->assertEquals($activityCount, CRM_Activity_BAO_Activity::getActivitiesCount($testCase['params'])); if ($caseName == 'with-no-activity') { $this->assertEquals(0, count($activities)); @@ -782,6 +761,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { * CRM-20793 : Test getActivities by using activity date and status filter */ public function testByActivityDateAndStatus() { + CRM_Core_Config::singleton()->userPermissionClass->permissions = ['view all contacts', 'access CiviCRM']; $op = new PHPUnit_Extensions_Database_Operation_Insert(); $op->execute($this->_dbconn, $this->createFlatXMLDataSet( @@ -829,7 +809,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'admin' => TRUE, 'caseId' => NULL, 'context' => 'activity', - 'activity_date_relative' => 'this.day', + 'activity_date_time_relative' => 'this.day', 'activity_type_id' => NULL, 'offset' => 0, 'rowCount' => 0, @@ -842,8 +822,8 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'admin' => TRUE, 'caseId' => NULL, 'context' => 'activity', - 'activity_date_low' => date('Y/m/d', strtotime('yesterday')), - 'activity_date_high' => date('Y/m/d'), + 'activity_date_time_low' => date('Y/m/d', strtotime('yesterday')), + 'activity_date_time_high' => date('Y/m/d'), 'activity_type_id' => NULL, 'offset' => 0, 'rowCount' => 0, @@ -856,7 +836,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'admin' => TRUE, 'caseId' => NULL, 'context' => 'activity', - 'activity_date_relative' => 'previous.week', + 'activity_date_time_relative' => 'previous.week', 'activity_type_id' => NULL, 'offset' => 0, 'rowCount' => 0, @@ -869,7 +849,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'admin' => TRUE, 'caseId' => NULL, 'context' => 'activity', - 'activity_date_relative' => 'this.quarter', + 'activity_date_time_relative' => 'this.quarter', 'activity_type_id' => NULL, 'offset' => 0, 'rowCount' => 0, @@ -892,34 +872,37 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { ); foreach ($testCases as $caseName => $testCase) { - $activitiesDep = CRM_Activity_BAO_Activity::deprecatedGetActivities($testCase['params']); - $activityCount = CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($testCase['params']); - asort($activitiesDep); - $activityIDs = array_keys($activitiesDep); + CRM_Utils_Date::convertFormDateToApiFormat($testCase['params'], 'activity_date_time', FALSE); + $activities = CRM_Activity_BAO_Activity::getActivities($testCase['params']); + $activityCount = CRM_Activity_BAO_Activity::getActivitiesCount($testCase['params']); + asort($activities); + $activityIDs = array_keys($activities); if ($caseName == 'todays-activity' || $caseName == 'todays-activity-filtered-by-range') { - $this->assertEquals(count($todayActivities), $activityCount); - $this->assertEquals(count($todayActivities), count($activitiesDep)); - $this->checkArrayEquals($todayActivities, $activityIDs); + // Only one of the 4 activities today relates to contact id 1. + $this->assertEquals(1, $activityCount); + $this->assertEquals(1, count($activities)); + $this->assertEquals([7], array_keys($activities)); } elseif ($caseName == 'last-week-activity') { - $this->assertEquals(count($lastWeekActivities), $activityCount); - $this->assertEquals(count($lastWeekActivities), count($activitiesDep)); - $this->checkArrayEquals($lastWeekActivities, $activityIDs); + // Only one of the 3 activities today relates to contact id 1. + $this->assertEquals(1, $activityCount); + $this->assertEquals(1, count($activities)); + $this->assertEquals([1], $activityIDs); } elseif ($caseName == 'lhis-quarter-activity') { $this->assertEquals(count($lastTwoMonthsActivities), $activityCount); - $this->assertEquals(count($lastTwoMonthsActivities), count($activitiesDep)); + $this->assertEquals(count($lastTwoMonthsActivities), count($activities)); $this->checkArrayEquals($lastTwoMonthsActivities, $activityIDs); } elseif ($caseName == 'last-or-next-year-activity') { $this->assertEquals(count($lastOrNextYearActivities), $activityCount); - $this->assertEquals(count($lastOrNextYearActivities), count($activitiesDep)); + $this->assertEquals(count($lastOrNextYearActivities), count($activities)); $this->checkArrayEquals($lastOrNextYearActivities, $activityIDs); } elseif ($caseName == 'activity-of-all-statuses') { - $this->assertEquals(16, $activityCount); - $this->assertEquals(16, count($activitiesDep)); + $this->assertEquals(3, $activityCount); + $this->assertEquals(3, count($activities)); } } } @@ -940,8 +923,8 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { foreach ($dates as $date) { $this->activityCreate(['activity_date_time' => $date]); } - $activitiesDep = CRM_Activity_BAO_Activity::deprecatedGetActivities($params); - $activityCount = CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($params); + $activitiesDep = CRM_Activity_BAO_Activity::getActivities($params); + $activityCount = CRM_Activity_BAO_Activity::getActivitiesCount($params); $this->assertEquals(count($activitiesDep), $activityCount); foreach ($activitiesDep as $activity) { $this->assertTrue(strtotime($activity['activity_date_time']) >= $expected['earliest'], $activity['activity_date_time'] . ' should be no earlier than ' . date('Y-m-d H:i:s', $expected['earliest'])); -- 2.25.1