From 7bb73d50fba8b6c86348575731e27e665cd4f7e7 Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 29 Apr 2017 10:25:31 +1200 Subject: [PATCH] CRM-20481 temporarily switch back while we assess / fix performance. We have been seeing performance issues with the use of the api for the dashboard. This is a back-out so we can figure that out before re-adding. Note this should not be merged to master as there are commits that 'add up to this' in master. There has been some git mistake this round :-( I'll figure out how to exclude --- CRM/Activity/BAO/Activity.php | 530 ++++++++++++++++-- CRM/Activity/Selector/Activity.php | 4 +- CRM/Contact/BAO/Contact.php | 2 +- .../phpunit/CRM/Activity/BAO/ActivityTest.php | 340 ++++++----- .../BAO/activities_for_dashboard_count.xml | 20 - tests/phpunit/CiviTest/CiviUnitTestCase.php | 36 ++ 6 files changed, 724 insertions(+), 208 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index 885a47be91..ef3b3725cc 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -665,47 +665,15 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { public static function getActivities($params, $getCount = FALSE) { $activities = array(); - // fetch all activity IDs whose target/assignee/source contact id is $params['contact_id'] - // currently cannot be done via Activity.Get API so we are using SQL query instead - if (!empty($params['contact_id'])) { - $activityIDs = CRM_Core_DAO::singleValueQuery("SELECT GROUP_CONCAT(DISTINCT activity_id SEPARATOR ',') - FROM civicrm_activity_contact - WHERE contact_id = %1", array(1 => array($params['contact_id'], 'Integer'))); - - // if no activities found for given $params['contact_id'] then return empty array - if (empty($activityIDs)) { - return $getCount ? count($activities) : $activities; - } - $activityIDs = explode(',', $activityIDs); - } - - // CRM-20441 Check if user has access to the activities. - // This is a temporary fix we need to figure out the rules around - // the right permissions to access Activities. - // This attpemts to reduce fatal errors in 4.7.19 RC. - if (!empty($activityIDs)) { - foreach ($activityIDs as $key => $activityId) { - try { - civicrm_api3('Activity', 'get', array('id' => $activityId, 'check_permissions' => 1)); - } - catch (Exception $e) { - unset($activityIDs[$key]); - } - } - } - if (empty($activityIDs)) { - return $getCount ? count($activities) : $activities; - } - // fetch all active activity types $activityTypes = CRM_Core_OptionGroup::values('activity_type'); // Activity.Get API params $activityParams = array( - 'id' => (!empty($activityIDs)) ? array('IN' => $activityIDs) : NULL, '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', @@ -882,6 +850,266 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { return $getCount ? count($activities) : $activities; } + /** + * 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_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name'); + $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(); + + $randomNum = md5(uniqid()); + $activityTempTable = "civicrm_temp_activity_details_{$randomNum}"; + + $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 = 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; + } + + // 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 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 = (CRM_Utils_Array::value('context', $input) == 'activity') ? " ORDER BY tbl.activity_date_time desc " : " ORDER BY tbl.status_id asc, tbl.activity_date_time asc "; + } + + if (!empty($input['rowCount']) && + $input['rowCount'] > 0 + ) { + $limit = " LIMIT {$input['offset']}, {$input['rowCount']} "; + } + + $input['count'] = FALSE; + list($sqlClause, $params) = self::deprecatedGetActivitySQLClause($input); + + $query = "{$insertSQL} + SELECT DISTINCT tbl.* from ( {$sqlClause} ) +as tbl "; + + // 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 = "civicrm_temp_activity_contact_{$randomNum}"; + $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; + } + /** * Get the component id and name if those are enabled and allowed. * @@ -918,6 +1146,238 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { return $components; } + /** + * Get the activity Count. + * + * @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 getActivitiesCount($input) { + return self::getActivities($input, TRUE); + } + + /** + * Get the activity Count. + * + * @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. + * + * @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 ($input['context'] != 'activity') { + $commonClauses[] = "civicrm_activity.status_id = 1"; + } + + // 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_Core_OptionGroup::values('activity_contacts', FALSE, FALSE, FALSE, NULL, 'name'); + $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. * @@ -2273,10 +2733,10 @@ INNER JOIN civicrm_option_group grp ON ( grp.id = val.option_group_id AND grp.n } // Get contact activities. - $activities = CRM_Activity_BAO_Activity::getActivities($params); + $activities = CRM_Activity_BAO_Activity::deprecatedGetActivities($params); // Add total. - $params['total'] = count($activities); + $params['total'] = CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($params); // Format params and add links. $contactActivities = array(); diff --git a/CRM/Activity/Selector/Activity.php b/CRM/Activity/Selector/Activity.php index 298d855b9b..69972ea110 100644 --- a/CRM/Activity/Selector/Activity.php +++ b/CRM/Activity/Selector/Activity.php @@ -352,7 +352,7 @@ class CRM_Activity_Selector_Activity extends CRM_Core_Selector_Base implements C 'rowCount' => 0, 'sort' => NULL, ); - return CRM_Activity_BAO_Activity::getActivities($params, TRUE); + return CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($params); } /** @@ -386,7 +386,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::getActivities($params); + $rows = CRM_Activity_BAO_Activity::deprecatedGetActivities($params); if (empty($rows)) { return $rows; diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php index e58be5ec3d..b310eaf5f0 100644 --- a/CRM/Contact/BAO/Contact.php +++ b/CRM/Contact/BAO/Contact.php @@ -2616,7 +2616,7 @@ AND civicrm_openid.is_primary = 1"; 'caseId' => NULL, 'context' => 'activity', ); - return CRM_Activity_BAO_Activity::getActivities($input, TRUE); + return CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($input); case 'mailing': $params = array('contact_id' => $contactId); diff --git a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php index 644e175b37..4002b83a77 100644 --- a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php +++ b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php @@ -7,12 +7,18 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { public function setUp() { parent::setUp(); + $this->prepareForACLs(); + CRM_Core_Config::singleton()->userPermissionClass->permissions = array('view all contacts', 'access CiviCRM'); } + /** + * Clean up after tests. + */ public function tearDown() { - // truncate a few tables - $tablesToTruncate = array('civicrm_contact', 'civicrm_activity', 'civicrm_activity_contact'); + $tablesToTruncate = array('civicrm_activity', 'civicrm_activity_contact', 'civicrm_email'); $this->quickCleanup($tablesToTruncate); + $this->cleanUpAfterACLs(); + parent::tearDown(); } /** @@ -33,8 +39,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'subject', 'Database check for created activity.' ); - // Now call create() to modify an existing Activity - + // Now call create() to modify an existing Activity. $params = array( 'id' => $activityId, 'source_contact_id' => $contactId, @@ -114,11 +119,11 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { CRM_Activity_BAO_Activity::create($params); - $activityId = $this->assertDBNotNull('CRM_Activity_DAO_Activity', 'Scheduling Meeting', 'id', + $this->assertDBNotNull('CRM_Activity_DAO_Activity', 'Scheduling Meeting', 'id', 'subject', 'Database check for created activity.' ); - $activityTargetId = $this->assertDBNotNull('CRM_Activity_DAO_ActivityContact', $targetContactId, + $this->assertDBNotNull('CRM_Activity_DAO_ActivityContact', $targetContactId, 'id', 'contact_id', 'Database check for created activity target.' ); @@ -163,11 +168,11 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { CRM_Activity_BAO_Activity::create($params); - $activityId = $this->assertDBNotNull('CRM_Activity_DAO_Activity', 'Scheduling Meeting', 'id', + $this->assertDBNotNull('CRM_Activity_DAO_Activity', 'Scheduling Meeting', 'id', 'subject', 'Database check for created activity.' ); - $activityTargetId = $this->assertDBNotNull('CRM_Activity_DAO_ActivityContact', $targetContactId, + $this->assertDBNotNull('CRM_Activity_DAO_ActivityContact', $targetContactId, 'id', 'contact_id', 'Database check for created activity target.' ); @@ -214,7 +219,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'subject', 'Database check for created activity.' ); - $activityTargetId = $this->assertDBNotNull('CRM_Activity_DAO_ActivityContact', $targetContactId, + $this->assertDBNotNull('CRM_Activity_DAO_ActivityContact', $targetContactId, 'id', 'contact_id', 'Database check for created activity target.' ); @@ -256,7 +261,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'subject', 'Database check for created activity.' ); - $activityAssignmentId = $this->assertDBNotNull('CRM_Activity_DAO_ActivityContact', + $this->assertDBNotNull('CRM_Activity_DAO_ActivityContact', $assigneeContactId, 'id', 'contact_id', 'Database check for created activity assignment.' ); @@ -274,30 +279,12 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { /** * Test getActivities BAO method for getting count. */ - public function testGetActivitiesCountforAdminDashboard() { - $op = new PHPUnit_Extensions_Database_Operation_Insert(); - $op->execute($this->_dbconn, - $this->createFlatXMLDataSet( - dirname(__FILE__) . '/activities_for_dashboard_count.xml' - ) - ); - - $params = array( - 'contact_id' => NULL, - 'admin' => TRUE, - 'caseId' => NULL, - 'context' => 'home', - 'activity_type_id' => NULL, - 'offset' => 0, - 'rowCount' => 0, - 'sort' => NULL, - ); - $activityCount = CRM_Activity_BAO_Activity::getActivities($params, TRUE); - - //since we are loading activities from dataset, we know total number of activities - // 8 schedule activities that should be shown on dashboard - $count = 8; - $this->assertEquals($count, $activityCount); + 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); } /** @@ -322,12 +309,13 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'sort' => NULL, ); - $activityCount = CRM_Activity_BAO_Activity::getActivities($params, TRUE); + $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)); } /** @@ -351,12 +339,13 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'rowCount' => 0, 'sort' => NULL, ); - $activityCount = CRM_Activity_BAO_Activity::getActivities($params, TRUE); + $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)); } /** @@ -381,7 +370,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { $activities = $obj->getContactActivity(); // This should include activities of type Meeting only. - foreach ($activities['data'] as $key => $value) { + foreach ($activities['data'] as $value) { $this->assertContains('Meeting', $value['activity_type']); } unset($_GET['activity_type_id']); @@ -389,7 +378,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { $_GET['activity_type_exclude_id'] = 1; $activities = $obj->getContactActivity(); // None of the activities should be of type Meeting. - foreach ($activities['data'] as $key => $value) { + foreach ($activities['data'] as $value) { $this->assertNotEquals('Meeting', $value['activity_type']); } } @@ -415,48 +404,65 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'rowCount' => 0, 'sort' => NULL, ); - $activityCount = CRM_Activity_BAO_Activity::getActivities($params, TRUE); + $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)); } /** * Test getActivities BAO method. */ - public function testGetActivitiesforAdminDashboard() { - $op = new PHPUnit_Extensions_Database_Operation_Insert(); - $op->execute($this->_dbconn, - $this->createFlatXMLDataSet( - dirname(__FILE__) . '/activities_for_dashboard_count.xml' - ) - ); - - $params = array( - 'contact_id' => NULL, - 'admin' => TRUE, - 'caseId' => NULL, - 'context' => 'home', - 'activity_type_id' => NULL, - 'offset' => 0, - 'rowCount' => 0, - 'sort' => NULL, - ); - $activities = CRM_Activity_BAO_Activity::getActivities($params); + 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; - $this->assertEquals($count, count($activities)); + foreach (array($activitiesDeprecatedFn) as $activities) { + $this->assertEquals($count, count($activities)); + + foreach ($activities as $key => $value) { + $this->assertEquals($value['subject'], "subject {$key}", 'Verify activity subject is correct.'); + $this->assertEquals($value['activity_type_id'], 2, 'Verify activity type is correct.'); + $this->assertEquals($value['status_id'], 1, 'Verify all activities are scheduled.'); + } + } + } - foreach ($activities as $key => $value) { - $this->assertEquals($value['subject'], "subject {$key}", 'Verify activity subject is correct.'); - $this->assertEquals($value['activity_type_id'], 2, 'Verify activity type is correct.'); - $this->assertEquals($value['status_id'], 1, 'Verify all activities are scheduled.'); + /** + * Test getActivities BAO method. + */ + 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) as $activities) { + // Skipped until we get back to the upgraded version properly. + //$this->assertEquals(0, count($activities)); } } + /** + * Test getActivities BAO method. + */ + public function testGetActivitiesForAdminDashboardAclLimitedViewContacts() { + CRM_Core_Config::singleton()->userPermissionClass->permissions = array('access CiviCRM'); + $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) as $activities) { + //$this->assertEquals(1, count($activities)); + } + + } + /** * Test getActivities BAO method. */ @@ -479,23 +485,25 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'rowCount' => 0, 'sort' => NULL, ); - $activities = CRM_Activity_BAO_Activity::getActivities($params); + $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 ( 2 scheduled, 3 Completed ), note that dashboard shows only scheduled activities - $count = 2; - $this->assertEquals($count, count($activities)); + foreach (array($activitiesDep) 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; + $this->assertEquals($count, count($activities)); - foreach ($activities as $key => $value) { - $this->assertEquals($value['subject'], "subject {$key}", 'Verify activity subject is correct.'); - $this->assertEquals($value['activity_type_id'], 2, 'Verify activity type is correct.'); - $this->assertEquals($value['status_id'], 1, 'Verify all activities are scheduled.'); + foreach ($activities as $key => $value) { + $this->assertEquals($value['subject'], "subject {$key}", 'Verify activity subject is correct.'); + $this->assertEquals($value['activity_type_id'], 2, 'Verify activity type is correct.'); + $this->assertEquals($value['status_id'], 1, 'Verify all activities are scheduled.'); - if ($key == 3) { - $this->assertArrayHasKey($contactID, $value['target_contact_name']); - } - elseif ($key == 4) { - $this->assertArrayHasKey($contactID, $value['assignee_contact_name']); + if ($key == 3) { + $this->assertArrayHasKey($contactID, $value['target_contact_name']); + } + elseif ($key == 4) { + $this->assertArrayHasKey($contactID, $value['assignee_contact_name']); + } } } } @@ -506,10 +514,11 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { public function testTargetCountforContactSummary() { $targetCount = 5; $contactId = $this->individualCreate(); + $targetContactIDs = array(); for ($i = 0; $i < $targetCount; $i++) { $targetContactIDs[] = $this->individualCreate(array(), $i); } - // create activities with 5 target contacts + // Create activities with 5 target contacts. $activityParams = array( 'source_contact_id' => $contactId, 'target_contact_id' => $targetContactIDs, @@ -520,10 +529,12 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'contact_id' => $contactId, 'context' => 'activity', ); - $activities = CRM_Activity_BAO_Activity::getActivities($params); + $activitiesDep = CRM_Activity_BAO_Activity::deprecatedGetActivities($params); + foreach (array($activitiesDep) as $activities) { + //verify target count + $this->assertEquals($targetCount, $activities[1]['target_contact_counter']); + } - //verify target count - $this->assertEquals($targetCount, $activities[1]['target_contact_counter']); } /** @@ -548,35 +559,38 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { 'rowCount' => 0, 'sort' => NULL, ); - $activities = CRM_Activity_BAO_Activity::getActivities($params); + $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; - $this->assertEquals($count, count($activities)); + foreach (array($activitiesDep) as $activities) { - foreach ($activities as $key => $value) { - $this->assertEquals($value['subject'], "subject {$key}", 'Verify activity subject is correct.'); + $this->assertEquals($count, count($activities)); - 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.'); - } + foreach ($activities as $key => $value) { + $this->assertEquals($value['subject'], "subject {$key}", 'Verify activity subject is correct.'); - 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 > 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 == 3) { - $this->assertArrayHasKey($contactID, $value['target_contact_name']); - } - elseif ($key == 4) { - $this->assertArrayHasKey($contactID, $value['assignee_contact_name']); + 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 == 3) { + $this->assertArrayHasKey($contactID, $value['target_contact_name']); + } + elseif ($key == 4) { + $this->assertArrayHasKey($contactID, $value['assignee_contact_name']); + } } } } @@ -592,7 +606,7 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { ) ); - // parameters for different test casess, check each array key for the specific test-case + // parameters for different test cases, check each array key for the specific test-case $testCases = array( 'with-no-activity' => array( 'params' => array( @@ -657,48 +671,55 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { ); foreach ($testCases as $caseName => $testCase) { - $activities = CRM_Activity_BAO_Activity::getActivities($testCase['params']); - $activityCount = CRM_Activity_BAO_Activity::getActivities($testCase['params'], TRUE); - if ($caseName == 'with-no-activity') { - $this->assertEquals(0, count($activities)); - $this->assertEquals(0, $activityCount); - } - elseif ($caseName == 'with-activity') { - // contact id 1 is assigned as source, target and assignee for activity id 1, 7 and 8 respectively - $this->assertEquals(3, count($activities)); - $this->assertEquals(3, $activityCount); - $this->assertEquals(1, $activities[1]['source_contact_id']); - $this->assertEquals(TRUE, array_key_exists(1, $activities[7]['target_contact_name'])); - $this->assertEquals(TRUE, array_key_exists(1, $activities[8]['assignee_contact_name'])); - } - elseif ($caseName == 'with-activity_type') { - // contact id 3 for activity type 2 is assigned as assignee, source and target for - // activity id 1, 3 and 8 respectively - $this->assertEquals(3, count($activities)); - $this->assertEquals(3, $activityCount); - // ensure activity type id is 2 - $this->assertEquals(2, $activities[1]['activity_type_id']); - $this->assertEquals(3, $activities[3]['source_contact_id']); - $this->assertEquals(TRUE, array_key_exists(3, $activities[8]['target_contact_name'])); - $this->assertEquals(TRUE, array_key_exists(3, $activities[1]['assignee_contact_name'])); - } - if ($caseName == 'exclude-all-activity_type') { - $this->assertEquals(0, count($activities)); - $this->assertEquals(0, $activityCount); - } - if ($caseName == 'sort-by-subject') { - $this->assertEquals(3, count($activities)); - $this->assertEquals(3, $activityCount); - // activities should be order by 'subject DESC' - $subjectOrder = array( - 'subject 8', - 'subject 7', - 'subject 1', - ); - $count = 0; - foreach ($activities as $activity) { - $this->assertEquals($subjectOrder[$count], $activity['subject']); - $count++; + $activitiesDep = CRM_Activity_BAO_Activity::deprecatedGetActivities($testCase['params']); + $activityCount = CRM_Activity_BAO_Activity::deprecatedGetActivitiesCount($testCase['params']); + //$activitiesNew = CRM_Activity_BAO_Activity::getActivities($testCase['params']); + + foreach (array($activitiesDep) as $activities) { + //$this->assertEquals($activityCount, CRM_Activity_BAO_Activity::getActivitiesCount($testCase['params'])); + if ($caseName == 'with-no-activity') { + $this->assertEquals(0, count($activities)); + $this->assertEquals(0, $activityCount); + } + elseif ($caseName == 'with-activity') { + // contact id 1 is assigned as source, target and assignee for activity id 1, 7 and 8 respectively + $this->assertEquals(3, count($activities)); + $this->assertEquals(3, $activityCount); + $this->assertEquals(1, $activities[1]['source_contact_id']); + // @todo - this is a discrepancy between old & new - review. + //$this->assertEquals(TRUE, array_key_exists(1, $activities[7]['target_contact_name'])); + $this->assertEquals(TRUE, array_key_exists(1, $activities[8]['assignee_contact_name'])); + } + elseif ($caseName == 'with-activity_type') { + // contact id 3 for activity type 2 is assigned as assignee, source and target for + // activity id 1, 3 and 8 respectively + $this->assertEquals(3, count($activities)); + $this->assertEquals(3, $activityCount); + // ensure activity type id is 2 + $this->assertEquals(2, $activities[1]['activity_type_id']); + $this->assertEquals(3, $activities[3]['source_contact_id']); + // @todo review inconsistency between 2 versions. + // $this->assertEquals(TRUE, array_key_exists(3, $activities[8]['target_contact_name'])); + $this->assertEquals(TRUE, array_key_exists(3, $activities[1]['assignee_contact_name'])); + } + if ($caseName == 'exclude-all-activity_type') { + $this->assertEquals(0, count($activities)); + $this->assertEquals(0, $activityCount); + } + if ($caseName == 'sort-by-subject') { + $this->assertEquals(3, count($activities)); + $this->assertEquals(3, $activityCount); + // activities should be order by 'subject DESC' + $subjectOrder = array( + 'subject 8', + 'subject 7', + 'subject 1', + ); + $count = 0; + foreach ($activities as $activity) { + $this->assertEquals($subjectOrder[$count], $activity['subject']); + $count++; + } } } } @@ -733,7 +754,6 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { $formAddress = CRM_Case_BAO_Case::getReceiptFrom($activity['id']); $expectedFromAddress = sprintf("%s <%s>", $sourceDisplayName, $sourceContactParams['email']); $this->assertEquals($expectedFromAddress, $formAddress); - // ----------------------- End of Case 1 --------------------------- // Case 2: System Default From Address // but first erase the email address of existing source contact ID @@ -759,13 +779,33 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { elseif (!empty($domainInfo['domain_email'])) { $expectedFromAddress = sprintf("%s <%s>", $domainInfo['name'], $domainInfo['domain_email']); } - // TODO: due to unknown reason the following assertion fails on - // test.civicrm.org test build but works fine on local - // $this->assertEquals($expectedFromAddress, $formAddress); - // ----------------------- End of Case 2 --------------------------- + $this->assertEquals($expectedFromAddress, $formAddress); // TODO: Case 4 about checking the $formAddress on basis of logged contact ID respectively needs, // to change the domain setting, which isn't straight forward in test environment } + /** + * Set up for testing activity queries. + */ + protected function setUpForActivityDashboardTests() { + $op = new PHPUnit_Extensions_Database_Operation_Insert(); + $op->execute($this->_dbconn, + $this->createFlatXMLDataSet( + dirname(__FILE__) . '/activities_for_dashboard_count.xml' + ) + ); + + $this->_params = array( + 'contact_id' => NULL, + 'admin' => TRUE, + 'caseId' => NULL, + 'context' => 'home', + 'activity_type_id' => NULL, + 'offset' => 0, + 'rowCount' => 0, + 'sort' => NULL, + ); + } + } diff --git a/tests/phpunit/CRM/Activity/BAO/activities_for_dashboard_count.xml b/tests/phpunit/CRM/Activity/BAO/activities_for_dashboard_count.xml index 4ce2c20d4a..f954a3b7e4 100644 --- a/tests/phpunit/CRM/Activity/BAO/activities_for_dashboard_count.xml +++ b/tests/phpunit/CRM/Activity/BAO/activities_for_dashboard_count.xml @@ -1,26 +1,6 @@ - - - - userPermissionClass->permissions = array(); + } + + /** + * Reset after ACLs. + */ + protected function cleanUpAfterACLs() { + CRM_Utils_Hook::singleton()->reset(); + $tablesToTruncate = array( + 'civicrm_acl', + 'civicrm_acl_cache', + 'civicrm_acl_entity_role', + 'civicrm_acl_contact_cache', + ); + $this->quickCleanup($tablesToTruncate); + $config = CRM_Core_Config::singleton(); + unset($config->userPermissionClass->permissions); + } /** * Create a smart group. * @@ -3800,6 +3823,19 @@ AND ( TABLE_NAME LIKE 'civicrm_value_%' ) public function aclWhereHookNoResults($type, &$tables, &$whereTables, &$contactID, &$where) { } + /** + * Only specified contact returned. + * @implements CRM_Utils_Hook::aclWhereClause + * @param $type + * @param $tables + * @param $whereTables + * @param $contactID + * @param $where + */ + public function aclWhereMultipleContacts($type, &$tables, &$whereTables, &$contactID, &$where) { + $where = " contact_a.id IN (" . implode(', ', $this->allowedContacts) . ")"; + } + /** * @implements CRM_Utils_Hook::selectWhereClause * -- 2.25.1