Skip to content

Commit

Permalink
Merge pull request #12453 from eileenmcnaughton/activity_report
Browse files Browse the repository at this point in the history 
Fix activity report to bring it under standardised report testing
  • Loading branch information
@eileenmcnaughton
eileenmcnaughton authored Jul 16, 2018
2 parents cd761cf + 5a14305 commit 6ac1a85
Show file tree
Hide file tree
Showing 3 changed files with 270 additions and 95 deletions.
21 changes: 21 additions & 0 deletions CRM/Report/Form.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,27 @@ public function cleanUpTemporaryTables() {
}
}

/**
* Create a temporary table.
*
* This function creates a table AND adds the details to the developer tab & $this->>temporary tables.
*
* @todo improve presentation on the developer tab since CREATE TEMPORARY is removed.
*
* @param string $identifier
* @param $sql
* @param bool $isTrueTemporary
* Is this a mysql temporary table or temporary in a less technical sense.
*
* @return string
*/
public function createTemporaryTable($identifier, $sql, $isTrueTemporary = TRUE) {
$this->addToDeveloperTab($sql);
$name = CRM_Utils_SQL_TempTable::build()->setUtf8(TRUE)->setDurable($isTrueTemporary)->createWithQuery($sql)->getName();
$this->temporaryTables[$identifier] = ['temporary' => $isTrueTemporary, 'name' => $name];
return $name;
}

/**
* Add columns to report.
*/
Expand Down
218 changes: 124 additions & 94 deletions CRM/Report/Form/Activity.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ public function __construct() {
$condition = " AND ( v.component_id IS NULL {$include} )";
$this->activityTypes = CRM_Core_OptionGroup::values('activity_type', FALSE, FALSE, FALSE, $condition);
asort($this->activityTypes);

// @todo split the 3 different contact tables into their own array items.
// this will massively simplify the needs of this report.
$this->_columns = array(
'civicrm_contact' => array(
'dao' => 'CRM_Contact_DAO_Contact',
Expand Down Expand Up @@ -394,7 +397,12 @@ public function addressFields($orderBy = FALSE) {
/**
* Build select clause.
*
* @param null $recordType
* @todo get rid of $recordType param. It's only because 3 separate contact tables
* are mis-declared as one that we need it.
*
* @param null $recordType deprecated
* Parameter to hack around the bad decision made in construct to misrepresent
* different tables as the same table.
*/
public function select($recordType = 'target') {
if (!array_key_exists("contact_{$recordType}", $this->_params['fields']) &&
Expand All @@ -416,6 +424,7 @@ public function select($recordType = 'target') {

$removeKeys = array();
if ($recordType == 'target') {
// @todo - fix up the way the tables are declared in construct & remove this.
foreach ($this->_selectClauses as $key => $clause) {
if (strstr($clause, 'civicrm_contact_assignee.') ||
strstr($clause, 'civicrm_contact_source.') ||
Expand All @@ -430,6 +439,7 @@ public function select($recordType = 'target') {
}
}
elseif ($recordType == 'assignee') {
// @todo - fix up the way the tables are declared in construct & remove this.
foreach ($this->_selectClauses as $key => $clause) {
if (strstr($clause, 'civicrm_contact_target.') ||
strstr($clause, 'civicrm_contact_source.') ||
Expand All @@ -444,6 +454,7 @@ public function select($recordType = 'target') {
}
}
elseif ($recordType == 'source') {
// @todo - fix up the way the tables are declared in construct & remove this.
foreach ($this->_selectClauses as $key => $clause) {
if (strstr($clause, 'civicrm_contact_target.') ||
strstr($clause, 'civicrm_contact_assignee.') ||
Expand All @@ -460,6 +471,7 @@ public function select($recordType = 'target') {
elseif ($recordType == 'final') {
$this->_selectClauses = $this->_selectAliasesTotal;
foreach ($this->_selectClauses as $key => $clause) {
// @todo - fix up the way the tables are declared in construct & remove this.
if (strstr($clause, 'civicrm_contact_contact_target') ||
strstr($clause, 'civicrm_contact_contact_assignee') ||
strstr($clause, 'civicrm_contact_contact_source') ||
Expand Down Expand Up @@ -496,98 +508,45 @@ public function select($recordType = 'target') {

/**
* Build from clause.
*
* @param string $recordType
* @todo remove this function & declare the 3 contact tables separately
*/
public function from($recordType = 'target') {
public function from() {
$activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');
$activityTypeId = CRM_Core_DAO::getFieldValue("CRM_Core_DAO_OptionGroup", 'activity_type', 'id', 'name');
$assigneeID = CRM_Utils_Array::key('Activity Assignees', $activityContacts);
$targetID = CRM_Utils_Array::key('Activity Targets', $activityContacts);
$sourceID = CRM_Utils_Array::key('Activity Source', $activityContacts);

if ($recordType == 'target') {
$this->_from = "
FROM civicrm_activity {$this->_aliases['civicrm_activity']}
INNER JOIN civicrm_activity_contact {$this->_aliases['civicrm_activity_contact']}
ON {$this->_aliases['civicrm_activity']}.id = {$this->_aliases['civicrm_activity_contact']}.activity_id AND
{$this->_aliases['civicrm_activity_contact']}.record_type_id = {$targetID}
INNER JOIN civicrm_contact civicrm_contact_target
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_contact_target.id
{$this->_aclFrom}";

if ($this->isTableSelected('civicrm_email')) {
$this->_from .= "
LEFT JOIN civicrm_email civicrm_email_target
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_email_target.contact_id AND
civicrm_email_target.is_primary = 1";
}

if ($this->isTableSelected('civicrm_phone')) {
$this->_from .= "
LEFT JOIN civicrm_phone civicrm_phone_target
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_phone_target.contact_id AND
civicrm_phone_target.is_primary = 1 ";
}
$this->_aliases['civicrm_contact'] = 'civicrm_contact_target';
}

if ($recordType == 'assignee') {
$this->_from = "
FROM civicrm_activity {$this->_aliases['civicrm_activity']}
INNER JOIN civicrm_activity_contact {$this->_aliases['civicrm_activity_contact']}
ON {$this->_aliases['civicrm_activity']}.id = {$this->_aliases['civicrm_activity_contact']}.activity_id AND
{$this->_aliases['civicrm_activity_contact']}.record_type_id = {$assigneeID}
INNER JOIN civicrm_contact civicrm_contact_assignee
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_contact_assignee.id
{$this->_aclFrom}";

if ($this->isTableSelected('civicrm_email')) {
$this->_from .= "
LEFT JOIN civicrm_email civicrm_email_assignee
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_email_assignee.contact_id AND
civicrm_email_assignee.is_primary = 1";
}
if ($this->isTableSelected('civicrm_phone')) {
$this->_from .= "
LEFT JOIN civicrm_phone civicrm_phone_assignee
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_phone_assignee.contact_id AND
civicrm_phone_assignee.is_primary = 1 ";
}
$this->_aliases['civicrm_contact'] = 'civicrm_contact_assignee';
$this->_from = "
FROM civicrm_activity {$this->_aliases['civicrm_activity']}
INNER JOIN civicrm_activity_contact {$this->_aliases['civicrm_activity_contact']}
ON {$this->_aliases['civicrm_activity']}.id = {$this->_aliases['civicrm_activity_contact']}.activity_id AND
{$this->_aliases['civicrm_activity_contact']}.record_type_id = {$targetID}
INNER JOIN civicrm_contact civicrm_contact_target
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_contact_target.id
{$this->_aclFrom}";

if ($this->isTableSelected('civicrm_email')) {
$this->_from .= "
LEFT JOIN civicrm_email civicrm_email_target
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_email_target.contact_id AND
civicrm_email_target.is_primary = 1";
}

if ($recordType == 'source') {
$this->_from = "
FROM civicrm_activity {$this->_aliases['civicrm_activity']}
INNER JOIN civicrm_activity_contact {$this->_aliases['civicrm_activity_contact']}
ON {$this->_aliases['civicrm_activity']}.id = {$this->_aliases['civicrm_activity_contact']}.activity_id AND
{$this->_aliases['civicrm_activity_contact']}.record_type_id = {$sourceID}
INNER JOIN civicrm_contact civicrm_contact_source
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_contact_source.id
{$this->_aclFrom}";

if ($this->isTableSelected('civicrm_email')) {
$this->_from .= "
LEFT JOIN civicrm_email civicrm_email_source
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_email_source.contact_id AND
civicrm_email_source.is_primary = 1";
}
if ($this->isTableSelected('civicrm_phone')) {
$this->_from .= "
LEFT JOIN civicrm_phone civicrm_phone_source
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_phone_source.contact_id AND
civicrm_phone_source.is_primary = 1 ";
}
$this->_aliases['civicrm_contact'] = 'civicrm_contact_source';
if ($this->isTableSelected('civicrm_phone')) {
$this->_from .= "
LEFT JOIN civicrm_phone civicrm_phone_target
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_phone_target.contact_id AND
civicrm_phone_target.is_primary = 1 ";
}
$this->_aliases['civicrm_contact'] = 'civicrm_contact_target';

$this->joinAddressFromContact();
}

/**
* Build where clause.
*
* @todo get rid of $recordType param. It's only because 3 separate contact tables
* are mis-declared as one that we need it.
*
* @param string $recordType
*/
public function where($recordType = NULL) {
Expand Down Expand Up @@ -731,7 +690,7 @@ public function add2group($groupID) {
$new_having = ' addtogroup_contact_id';
$having = str_ireplace(' civicrm_contact_contact_target_id', $new_having, $this->_having);
$query = "$select
FROM civireport_activity_temp_target tar
FROM {$this->temporaryTables['activity_temp_table']} tar
GROUP BY civicrm_activity_id $having {$this->_orderBy}";
$select = 'AS addtogroup_contact_id';
$query = str_ireplace('AS civicrm_contact_contact_target_id', $select, $query);
Expand Down Expand Up @@ -805,21 +764,21 @@ public function buildQuery($applyLimit = TRUE) {
}
}

// @todo - all this temp table stuff is here because pre 4.4 the activity contact
// form did not exist.
// Fixing the way the construct method declares them will make all this redundant.
// 1. fill temp table with target results
$this->buildACLClause(array('civicrm_contact_target'));
$this->select('target');
$this->from('target');
$this->from();
$this->customDataFrom();
$this->where('target');
$insertCols = implode(',', $this->_selectAliases);
$tempQuery = "CREATE TEMPORARY TABLE civireport_activity_temp_target {$this->_databaseAttributes} AS
{$this->_select} {$this->_from} {$this->_where} ";
$this->executeReportQuery($tempQuery);
$tempTableName = $this->createTemporaryTable('activity_temp_table', "{$this->_select} {$this->_from} {$this->_where}");

// 2. add new columns to hold assignee and source results
// fixme: add when required
$tempQuery = "
ALTER TABLE civireport_activity_temp_target
ALTER TABLE $tempTableName
MODIFY COLUMN civicrm_contact_contact_target_id VARCHAR(128),
ADD COLUMN civicrm_contact_contact_assignee VARCHAR(128),
ADD COLUMN civicrm_contact_contact_source VARCHAR(128),
Expand All @@ -834,23 +793,24 @@ public function buildQuery($applyLimit = TRUE) {
// 3. fill temp table with assignee results
$this->buildACLClause(array('civicrm_contact_assignee'));
$this->select('assignee');
$this->from('assignee');
$this->buildAssigneeFrom();

$this->customDataFrom();
$this->where('assignee');
$insertCols = implode(',', $this->_selectAliases);
$tempQuery = "INSERT INTO civireport_activity_temp_target ({$insertCols})
$tempQuery = "INSERT INTO $tempTableName ({$insertCols})
{$this->_select}
{$this->_from} {$this->_where}";
$this->executeReportQuery($tempQuery);

// 4. fill temp table with source results
$this->buildACLClause(array('civicrm_contact_source'));
$this->select('source');
$this->from('source');
$this->buildSourceFrom();
$this->customDataFrom();
$this->where('source');
$insertCols = implode(',', $this->_selectAliases);
$tempQuery = "INSERT INTO civireport_activity_temp_target ({$insertCols})
$tempQuery = "INSERT INTO $tempTableName ({$insertCols})
{$this->_select}
{$this->_from} {$this->_where}";
$this->executeReportQuery($tempQuery);
Expand All @@ -865,7 +825,7 @@ public function buildQuery($applyLimit = TRUE) {
$this->orderBy();
foreach ($this->_sections as $alias => $section) {
if (!empty($section) && $section['name'] == 'activity_date_time') {
$this->alterSectionHeaderForDateTime('civireport_activity_temp_target', $section['tplField']);
$this->alterSectionHeaderForDateTime($tempTableName, $section['tplField']);
}
}

Expand All @@ -882,7 +842,7 @@ public function buildQuery($applyLimit = TRUE) {
}

$sql = "{$this->_select}
FROM civireport_activity_temp_target tar
FROM $tempTableName tar
INNER JOIN civicrm_activity {$this->_aliases['civicrm_activity']} ON {$this->_aliases['civicrm_activity']}.id = tar.civicrm_activity_id
INNER JOIN civicrm_activity_contact {$this->_aliases['civicrm_activity_contact']} ON {$this->_aliases['civicrm_activity_contact']}.activity_id = {$this->_aliases['civicrm_activity']}.id
AND {$this->_aliases['civicrm_activity_contact']}.record_type_id = {$sourceID}
Expand Down Expand Up @@ -930,7 +890,9 @@ public function alterDisplay(&$rows) {
$activityStatus = CRM_Core_PseudoConstant::activityStatus();
$priority = CRM_Core_PseudoConstant::get('CRM_Activity_DAO_Activity', 'priority_id');
$viewLinks = FALSE;
$context = CRM_Utils_Request::retrieve('context', 'String', $this, FALSE, 'report');
// Would we ever want to retrieve from the form controller??
$form = $this->noController ? NULL : $this;
$context = CRM_Utils_Request::retrieve('context', 'Alphanumeric', $form, FALSE, 'report');
$actUrl = '';

if (CRM_Core_Permission::check('access CiviCRM')) {
Expand Down Expand Up @@ -1114,7 +1076,7 @@ public function sectionTotals() {
$this->_select = CRM_Contact_BAO_Query::appendAnyValueToSelect($ifnulls, $sectionAliases);

$query = $this->_select .
", count(DISTINCT civicrm_activity_id) as ct from civireport_activity_temp_target group by " .
", count(DISTINCT civicrm_activity_id) as ct from {$this->temporaryTables['activity_temp_table']} group by " .
implode(", ", $sectionAliases);

// initialize array of total counts
Expand Down Expand Up @@ -1147,4 +1109,72 @@ public function sectionTotals() {
}
}

/**
* @todo remove this function & declare the 3 contact tables separately
*
* (Currently the construct method incorrectly melds them - this is an interim
* refactor in order to get this under ReportTemplateTests)
*/
protected function buildAssigneeFrom() {
$activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');
$assigneeID = CRM_Utils_Array::key('Activity Assignees', $activityContacts);
$this->_from = "
FROM civicrm_activity {$this->_aliases['civicrm_activity']}
INNER JOIN civicrm_activity_contact {$this->_aliases['civicrm_activity_contact']}
ON {$this->_aliases['civicrm_activity']}.id = {$this->_aliases['civicrm_activity_contact']}.activity_id AND
{$this->_aliases['civicrm_activity_contact']}.record_type_id = {$assigneeID}
INNER JOIN civicrm_contact civicrm_contact_assignee
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_contact_assignee.id
{$this->_aclFrom}";

if ($this->isTableSelected('civicrm_email')) {
$this->_from .= "
LEFT JOIN civicrm_email civicrm_email_assignee
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_email_assignee.contact_id AND
civicrm_email_assignee.is_primary = 1";
}
if ($this->isTableSelected('civicrm_phone')) {
$this->_from .= "
LEFT JOIN civicrm_phone civicrm_phone_assignee
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_phone_assignee.contact_id AND
civicrm_phone_assignee.is_primary = 1 ";
}
$this->_aliases['civicrm_contact'] = 'civicrm_contact_assignee';
$this->joinAddressFromContact();
}

/**
* @todo remove this function & declare the 3 contact tables separately
*
* (Currently the construct method incorrectly melds them - this is an interim
* refactor in order to get this under ReportTemplateTests)
*/
protected function buildSourceFrom() {
$activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate');
$sourceID = CRM_Utils_Array::key('Activity Source', $activityContacts);
$this->_from = "
FROM civicrm_activity {$this->_aliases['civicrm_activity']}
INNER JOIN civicrm_activity_contact {$this->_aliases['civicrm_activity_contact']}
ON {$this->_aliases['civicrm_activity']}.id = {$this->_aliases['civicrm_activity_contact']}.activity_id AND
{$this->_aliases['civicrm_activity_contact']}.record_type_id = {$sourceID}
INNER JOIN civicrm_contact civicrm_contact_source
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_contact_source.id
{$this->_aclFrom}";

if ($this->isTableSelected('civicrm_email')) {
$this->_from .= "
LEFT JOIN civicrm_email civicrm_email_source
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_email_source.contact_id AND
civicrm_email_source.is_primary = 1";
}
if ($this->isTableSelected('civicrm_phone')) {
$this->_from .= "
LEFT JOIN civicrm_phone civicrm_phone_source
ON {$this->_aliases['civicrm_activity_contact']}.contact_id = civicrm_phone_source.contact_id AND
civicrm_phone_source.is_primary = 1 ";
}
$this->_aliases['civicrm_contact'] = 'civicrm_contact_source';
$this->joinAddressFromContact();
}

}
Loading

0 comments on commit 6ac1a85

Please sign in to comment.