Skip to content

Commit

Permalink
Merge pull request #8820 from eileenmcnaughton/report
Browse files Browse the repository at this point in the history
Reports: CRM-19175 fix add to group, CRM-19170  performance on group filters
  • Loading branch information
eileenmcnaughton authored Aug 16, 2016
2 parents ca40f47 + eb7b405 commit c015fa2
Show file tree
Hide file tree
Showing 41 changed files with 875 additions and 270 deletions.
178 changes: 165 additions & 13 deletions CRM/Report/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,26 @@ class CRM_Report_Form extends CRM_Core_Form {
*/
protected $_groupFilter = FALSE;

/**
* Has the report been optimised for group filtering.
*
* The functionality for group filtering has been improved but not
* all reports have been adjusted to take care of it.
*
* This property exists to highlight the reports which are still using the
* slow method & allow group filtering to still work for them until they
* can be migrated.
*
* In order to protect extensions we have to default to TRUE - but I have
* separately marked every class with a groupFilter in the hope that will trigger
* people to fix them as they touch them.
*
* CRM-19170
*
* @var bool
*/
protected $groupFilterNotOptimised = TRUE;

/**
* Navigation fields
*
Expand Down Expand Up @@ -219,6 +239,13 @@ class CRM_Report_Form extends CRM_Core_Form {
protected $_selectAliases = array();
protected $_rollup = NULL;

/**
* Table containing list of contact IDs within the group filter.
*
* @var string
*/
protected $groupTempTable = '';

/**
* @var array
*/
Expand Down Expand Up @@ -1304,15 +1331,16 @@ protected function addToDeveloperTab($sql) {

$this->assignTabs();
$this->sqlArray[] = $sql;
foreach (array('LEFT JOIN') as $term) {
$sql = str_replace($term, '<br />&nbsp&nbsp' . $term, $sql);
}
foreach (array('FROM', 'WHERE', 'GROUP BY', 'ORDER BY', 'LIMIT', ';') as $term) {
$sql = str_replace($term, '<br /><br />' . $term, $sql);
foreach ($this->sqlArray as $sql) {
foreach (array('LEFT JOIN') as $term) {
$sql = str_replace($term, '<br>&nbsp&nbsp' . $term, $sql);
}
foreach (array('FROM', 'WHERE', 'GROUP BY', 'ORDER BY', 'LIMIT', ';') as $term) {
$sql = str_replace($term, '<br><br>' . $term, $sql);
}
$this->sqlFormattedArray[] = $sql;
$this->assign('sql', implode(';<br><br><br><br>', $this->sqlFormattedArray));
}
$this->sql .= $sql . "<br />";

$this->assign('sql', $this->sql);
}

/**
Expand Down Expand Up @@ -1847,7 +1875,7 @@ public function whereClause(&$field, $op, $value, $min, $max) {

case 'in':
case 'notin':
if (is_string($value) && strlen($value)) {
if ((is_string($value) || is_numeric($value)) && strlen($value)) {
$value = explode(',', $value);
}
if ($value !== NULL && is_array($value) && count($value) > 0) {
Expand Down Expand Up @@ -2620,6 +2648,7 @@ public function beginPostProcessCommon() {
* @return string
*/
public function buildQuery($applyLimit = TRUE) {
$this->buildGroupTempTable();
$this->select();
$this->from();
$this->customDataFrom();
Expand Down Expand Up @@ -3144,7 +3173,7 @@ public function endPostProcess(&$rows = NULL) {
else {
CRM_Core_Session::setStatus(ts("Report mail could not be sent."), ts('Mail Error'), 'error');
}
return TRUE;
return;
}
elseif ($this->_outputMode == 'print') {
echo $content;
Expand Down Expand Up @@ -3354,16 +3383,20 @@ public function setPager($rowCount = self::ROW_COUNT_LIMIT) {
}

/**
* Build where clause for groups.
* Build a group filter with contempt for large data sets.
*
* This function has been retained as it takes time to migrate the reports over
* to the new method which will not crash on large datasets.
*
* @deprecated
*
* @param string $field
* @param mixed $value
* @param string $op
*
* @return string
*/
public function whereGroupClause($field, $value, $op) {

public function legacySlowGroupFilterClause($field, $value, $op) {
$smartGroupQuery = "";

$group = new CRM_Contact_DAO_Group();
Expand Down Expand Up @@ -3406,6 +3439,83 @@ public function whereGroupClause($field, $value, $op) {
{$smartGroupQuery} ) ";
}

/**
* Build where clause for groups.
*
* @param string $field
* @param mixed $value
* @param string $op
*
* @return string
*/
public function whereGroupClause($field, $value, $op) {
if ($this->groupFilterNotOptimised) {
return $this->legacySlowGroupFilterClause($field, $value, $op);
}
if ($op === 'notin') {
return " group_temp_table.id IS NULL ";
}
// We will have used an inner join instead.
return "1";
}


/**
* Create a table of the contact ids included by the group filter.
*
* This function is called by both the api (tests) and the UI.
*/
public function buildGroupTempTable() {
if (!empty($this->groupTempTable) || empty ($this->_params['gid_value']) || $this->groupFilterNotOptimised) {
return;
}
$filteredGroups = (array) $this->_params['gid_value'];

$groups = civicrm_api3('Group', 'get', array(
'is_active' => 1,
'id' => array('IN' => $filteredGroups),
'saved_search_id' => array('>' => 0),
'return' => 'id',
));
$smartGroups = array_keys($groups['values']);

$query = "
SELECT group_contact.contact_id as id
FROM civicrm_group_contact group_contact
WHERE group_contact.group_id IN (" . implode(', ', $filteredGroups) . ")
AND group_contact.status = 'Added' ";

if (!empty($smartGroups)) {
CRM_Contact_BAO_GroupContactCache::check($smartGroups);
$smartGroups = implode(',', $smartGroups);
$query .= "
UNION DISTINCT
SELECT smartgroup_contact.contact_id as id
FROM civicrm_group_contact_cache smartgroup_contact
WHERE smartgroup_contact.group_id IN ({$smartGroups}) ";
}

$this->groupTempTable = 'civicrm_report_temp_group_' . date('Ymd_') . uniqid();
$this->executeReportQuery("
CREATE TEMPORARY TABLE $this->groupTempTable
$query
");
CRM_Core_DAO::executeQuery("ALTER TABLE $this->groupTempTable ADD INDEX i_id(id)");
}

/**
* Execute query and add it to the developer tab.
*
* @param string $query
* @param array $params
*
* @return \CRM_Core_DAO|object
*/
protected function executeReportQuery($query, $params = array()) {
$this->addToDeveloperTab($query);
return CRM_Core_DAO::executeQuery($query, $params);
}

/**
* Build where clause for tags.
*
Expand Down Expand Up @@ -4708,4 +4818,46 @@ public function selectivelyAddLocationTablesJoinsToFilterQuery() {
}
}

/**
* Set the base table for the FROM clause.
*
* Sets up the from clause, allowing for the possibility it might be a
* temp table pre-filtered by groups if a group filter is in use.
*
* @param string $baseTable
* @param string $field
* @param null $tableAlias
*/
public function setFromBase($baseTable, $field = 'id', $tableAlias = NULL) {
if (!$tableAlias) {
$tableAlias = $this->_aliases[$baseTable];
}
$this->_from = $this->_from = " FROM $baseTable $tableAlias ";
$this->joinGroupTempTable($baseTable, $field, $tableAlias);
$this->_from .= " {$this->_aclFrom} ";
}

/**
* Join the temp table contacting contacts who are members of the filtered groups.
*
* If we are using an IN filter we use an inner join, otherwise a left join.
*
* @param string $baseTable
* @param string $field
* @param string $tableAlias
*/
public function joinGroupTempTable($baseTable, $field, $tableAlias) {
if ($this->groupTempTable) {
if ($this->_params['gid_op'] == 'in') {
$this->_from = " FROM $this->groupTempTable group_temp_table INNER JOIN $baseTable $tableAlias
ON group_temp_table.id = $tableAlias.{$field} ";
}
else {
$this->_from .= "
LEFT JOIN $this->groupTempTable group_temp_table
ON $tableAlias.{$field} = group_temp_table.id ";
}
}
}

}
13 changes: 13 additions & 0 deletions CRM/Report/Form/Activity.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,19 @@ class CRM_Report_Form_Activity extends CRM_Report_Form {

protected $_nonDisplayFields = array();

/**
* This report has not been optimised for group filtering.
*
* The functionality for group filtering has been improved but not
* all reports have been adjusted to take care of it. This report has not
* and will run an inefficient query until fixed.
*
* CRM-19170
*
* @var bool
*/
protected $groupFilterNotOptimised = TRUE;

/**
* Class constructor.
*/
Expand Down
14 changes: 12 additions & 2 deletions CRM/Report/Form/ActivitySummary.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
*
* @package CRM
* @copyright CiviCRM LLC (c) 2004-2016
* $Id$
*
*/
class CRM_Report_Form_ActivitySummary extends CRM_Report_Form {

Expand All @@ -40,8 +38,20 @@ class CRM_Report_Form_ActivitySummary extends CRM_Report_Form {
protected $_tempDurationSumTableName;

/**
* This report has not been optimised for group filtering.
*
* The functionality for group filtering has been improved but not
* all reports have been adjusted to take care of it. This report has not
* and will run an inefficient query until fixed.
*
* CRM-19170
*
* @var bool
*/
protected $groupFilterNotOptimised = TRUE;

/**
* Class constructor.
*/
public function __construct() {
$this->_columns = array(
Expand Down
15 changes: 12 additions & 3 deletions CRM/Report/Form/Case/Demographics.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
*
* @package CRM
* @copyright CiviCRM LLC (c) 2004-2016
* $Id$
*
*/
class CRM_Report_Form_Case_Demographics extends CRM_Report_Form {

Expand All @@ -39,10 +37,21 @@ class CRM_Report_Form_Case_Demographics extends CRM_Report_Form {
protected $_emailField = FALSE;

protected $_phoneField = FALSE;

/**
* This report has not been optimised for group filtering.
*
* The functionality for group filtering has been improved but not
* all reports have been adjusted to take care of it. This report has not
* and will run an inefficient query until fixed.
*
* CRM-19170
*
* @var bool
*/
protected $groupFilterNotOptimised = TRUE;

/**
* Class constructor.
*/
public function __construct() {
$this->_columns = array(
Expand Down
5 changes: 1 addition & 4 deletions CRM/Report/Form/Case/Summary.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
*
* @package CRM
* @copyright CiviCRM LLC (c) 2004-2016
* $Id$
*
*/
class CRM_Report_Form_Case_Summary extends CRM_Report_Form {

Expand All @@ -41,8 +39,7 @@ class CRM_Report_Form_Case_Summary extends CRM_Report_Form {
protected $_customGroupExtends = array('Case');

/**
*/
/**
* Class constructor.
*/
public function __construct() {
$this->case_types = CRM_Case_PseudoConstant::caseType();
Expand Down
6 changes: 2 additions & 4 deletions CRM/Report/Form/Case/TimeSpent.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,11 @@
*
* @package CRM
* @copyright CiviCRM LLC (c) 2004-2016
* $Id$
*
*/
class CRM_Report_Form_Case_TimeSpent extends CRM_Report_Form {

/**
*/
/**
* Class constructor.
*/
public function __construct() {

Expand Down
16 changes: 14 additions & 2 deletions CRM/Report/Form/Contact/CurrentEmployer.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,22 @@
*
* @package CRM
* @copyright CiviCRM LLC (c) 2004-2016
* $Id$
*
*/
class CRM_Report_Form_Contact_CurrentEmployer extends CRM_Report_Form {

/**
* This report has not been optimised for group filtering.
*
* The functionality for group filtering has been improved but not
* all reports have been adjusted to take care of it. This report has not
* and will run an inefficient query until fixed.
*
* CRM-19170
*
* @var bool
*/
protected $groupFilterNotOptimised = TRUE;

protected $_summary = NULL;

protected $_customGroupExtends = array(
Expand All @@ -44,6 +55,7 @@ class CRM_Report_Form_Contact_CurrentEmployer extends CRM_Report_Form {
public $_drilldownReport = array('contact/detail' => 'Link to Detail Report');

/**
* Class constructor.
*/
public function __construct() {

Expand Down
Loading

0 comments on commit c015fa2

Please sign in to comment.