diff --git a/CRM/Report/Form.php b/CRM/Report/Form.php index ed8309b1776..5e278fdd081 100644 --- a/CRM/Report/Form.php +++ b/CRM/Report/Form.php @@ -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 * @@ -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 */ @@ -1304,15 +1331,16 @@ protected function addToDeveloperTab($sql) { $this->assignTabs(); $this->sqlArray[] = $sql; - foreach (array('LEFT JOIN') as $term) { - $sql = str_replace($term, '
  ' . $term, $sql); - } - foreach (array('FROM', 'WHERE', 'GROUP BY', 'ORDER BY', 'LIMIT', ';') as $term) { - $sql = str_replace($term, '

' . $term, $sql); + foreach ($this->sqlArray as $sql) { + foreach (array('LEFT JOIN') as $term) { + $sql = str_replace($term, '
  ' . $term, $sql); + } + foreach (array('FROM', 'WHERE', 'GROUP BY', 'ORDER BY', 'LIMIT', ';') as $term) { + $sql = str_replace($term, '

' . $term, $sql); + } + $this->sqlFormattedArray[] = $sql; + $this->assign('sql', implode(';



', $this->sqlFormattedArray)); } - $this->sql .= $sql . "
"; - - $this->assign('sql', $this->sql); } /** @@ -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) { @@ -2620,6 +2648,7 @@ public function beginPostProcessCommon() { * @return string */ public function buildQuery($applyLimit = TRUE) { + $this->buildGroupTempTable(); $this->select(); $this->from(); $this->customDataFrom(); @@ -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; @@ -3354,7 +3383,12 @@ 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 @@ -3362,8 +3396,7 @@ public function setPager($rowCount = self::ROW_COUNT_LIMIT) { * * @return string */ - public function whereGroupClause($field, $value, $op) { - + public function legacySlowGroupFilterClause($field, $value, $op) { $smartGroupQuery = ""; $group = new CRM_Contact_DAO_Group(); @@ -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. * @@ -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 "; + } + } + } + } diff --git a/CRM/Report/Form/Activity.php b/CRM/Report/Form/Activity.php index a5d6181be9b..ddb17452ffd 100644 --- a/CRM/Report/Form/Activity.php +++ b/CRM/Report/Form/Activity.php @@ -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. */ diff --git a/CRM/Report/Form/ActivitySummary.php b/CRM/Report/Form/ActivitySummary.php index fc83ff1e51f..d0bdef0a7d0 100644 --- a/CRM/Report/Form/ActivitySummary.php +++ b/CRM/Report/Form/ActivitySummary.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_ActivitySummary extends CRM_Report_Form { @@ -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( diff --git a/CRM/Report/Form/Case/Demographics.php b/CRM/Report/Form/Case/Demographics.php index 3badc118d50..ece79d69b6c 100644 --- a/CRM/Report/Form/Case/Demographics.php +++ b/CRM/Report/Form/Case/Demographics.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Case_Demographics extends CRM_Report_Form { @@ -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( diff --git a/CRM/Report/Form/Case/Summary.php b/CRM/Report/Form/Case/Summary.php index ea39d69d363..a5ef3c4f6a2 100644 --- a/CRM/Report/Form/Case/Summary.php +++ b/CRM/Report/Form/Case/Summary.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Case_Summary extends CRM_Report_Form { @@ -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(); diff --git a/CRM/Report/Form/Case/TimeSpent.php b/CRM/Report/Form/Case/TimeSpent.php index c9027057647..e8d7f62def7 100644 --- a/CRM/Report/Form/Case/TimeSpent.php +++ b/CRM/Report/Form/Case/TimeSpent.php @@ -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() { diff --git a/CRM/Report/Form/Contact/CurrentEmployer.php b/CRM/Report/Form/Contact/CurrentEmployer.php index e5ea4b76d59..15858960874 100644 --- a/CRM/Report/Form/Contact/CurrentEmployer.php +++ b/CRM/Report/Form/Contact/CurrentEmployer.php @@ -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( @@ -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() { diff --git a/CRM/Report/Form/Contact/Detail.php b/CRM/Report/Form/Contact/Detail.php index d03432e435d..dc9b766ac0c 100644 --- a/CRM/Report/Form/Contact/Detail.php +++ b/CRM/Report/Form/Contact/Detail.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Contact_Detail extends CRM_Report_Form { const ROW_COUNT_LIMIT = 10; @@ -44,6 +42,19 @@ class CRM_Report_Form_Contact_Detail extends CRM_Report_Form { 'Organization', ); + /** + * 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. */ diff --git a/CRM/Report/Form/Contact/Log.php b/CRM/Report/Form/Contact/Log.php index 3cdc9f06b24..5c8833c57da 100644 --- a/CRM/Report/Form/Contact/Log.php +++ b/CRM/Report/Form/Contact/Log.php @@ -29,16 +29,13 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Contact_Log extends CRM_Report_Form { protected $_summary = NULL; /** - */ - /** + * Class constructor. */ public function __construct() { diff --git a/CRM/Report/Form/Contact/Relationship.php b/CRM/Report/Form/Contact/Relationship.php index 61f86667a0a..a7918726966 100644 --- a/CRM/Report/Form/Contact/Relationship.php +++ b/CRM/Report/Form/Contact/Relationship.php @@ -44,6 +44,19 @@ class CRM_Report_Form_Contact_Relationship extends CRM_Report_Form { ); public $_drilldownReport = array('contact/detail' => 'Link to Detail Report'); + /** + * 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; + /** * This will be a_b or b_a. * diff --git a/CRM/Report/Form/Contact/Summary.php b/CRM/Report/Form/Contact/Summary.php index 40b44d581b3..b3759306ae0 100644 --- a/CRM/Report/Form/Contact/Summary.php +++ b/CRM/Report/Form/Contact/Summary.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Contact_Summary extends CRM_Report_Form { @@ -50,6 +48,20 @@ class CRM_Report_Form_Contact_Summary extends CRM_Report_Form { public $_drilldownReport = array('contact/detail' => 'Link to Detail Report'); /** + * 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->_autoIncludeIndexedFieldsAsOrderBys = 1; diff --git a/CRM/Report/Form/Contribute/Bookkeeping.php b/CRM/Report/Form/Contribute/Bookkeeping.php index 7dc7fcb79a9..34bdb94f23e 100644 --- a/CRM/Report/Form/Contribute/Bookkeeping.php +++ b/CRM/Report/Form/Contribute/Bookkeeping.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Contribute_Bookkeeping extends CRM_Report_Form { protected $_addressField = FALSE; @@ -47,6 +45,20 @@ class CRM_Report_Form_Contribute_Bookkeeping 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; + + /** + * Class constructor. */ public function __construct() { $this->_autoIncludeIndexedFieldsAsOrderBys = 1; diff --git a/CRM/Report/Form/Contribute/Detail.php b/CRM/Report/Form/Contribute/Detail.php index 1683005a42c..f5f4509087e 100644 --- a/CRM/Report/Form/Contribute/Detail.php +++ b/CRM/Report/Form/Contribute/Detail.php @@ -46,6 +46,16 @@ class CRM_Report_Form_Contribute_Detail extends CRM_Report_Form { ); /** + * This report has been optimised for group filtering. + * + * CRM-19170 + * + * @var bool + */ + protected $groupFilterNotOptimised = FALSE; + + /** + * Class constructor. */ public function __construct() { $this->_autoIncludeIndexedFieldsAsOrderBys = 1; @@ -378,13 +388,14 @@ public function orderBy() { } /** - * @param bool $softcredit + * Set the FROM clause for the report. */ - public function from($softcredit = FALSE) { - $this->_from = " - FROM civicrm_contact {$this->_aliases['civicrm_contact']} {$this->_aclFrom} - INNER JOIN civicrm_contribution {$this->_aliases['civicrm_contribution']} - ON {$this->_aliases['civicrm_contact']}.id = {$this->_aliases['civicrm_contribution']}.contact_id AND {$this->_aliases['civicrm_contribution']}.is_test = 0"; + public function from() { + $this->setFromBase('civicrm_contact'); + $this->_from .= " + INNER JOIN civicrm_contribution {$this->_aliases['civicrm_contribution']} + ON {$this->_aliases['civicrm_contact']}.id = {$this->_aliases['civicrm_contribution']}.contact_id + AND {$this->_aliases['civicrm_contribution']}.is_test = 0"; if (CRM_Utils_Array::value('contribution_or_soft_value', $this->_params) == 'both' @@ -398,64 +409,7 @@ public function from($softcredit = FALSE) { $this->_from .= "\n INNER JOIN civicrm_contribution_soft contribution_soft_civireport ON contribution_soft_civireport.contribution_id = {$this->_aliases['civicrm_contribution']}.id"; } - - if ($softcredit) { - $this->_from = " - FROM civireport_contribution_detail_temp1 temp1_civireport - INNER JOIN civicrm_contribution {$this->_aliases['civicrm_contribution']} - ON temp1_civireport.civicrm_contribution_contribution_id = {$this->_aliases['civicrm_contribution']}.id - INNER JOIN civicrm_contribution_soft contribution_soft_civireport - ON contribution_soft_civireport.contribution_id = {$this->_aliases['civicrm_contribution']}.id - INNER JOIN civicrm_contact {$this->_aliases['civicrm_contact']} - ON {$this->_aliases['civicrm_contact']}.id = contribution_soft_civireport.contact_id - {$this->_aclFrom}"; - } - - if (!empty($this->_params['ordinality_value'])) { - $this->_from .= " - INNER JOIN (SELECT c.id, IF(COUNT(oc.id) = 0, 0, 1) AS ordinality FROM civicrm_contribution c LEFT JOIN civicrm_contribution oc ON c.contact_id = oc.contact_id AND oc.receive_date < c.receive_date GROUP BY c.id) {$this->_aliases['civicrm_contribution_ordinality']} - ON {$this->_aliases['civicrm_contribution_ordinality']}.id = {$this->_aliases['civicrm_contribution']}.id"; - } - - $this->addPhoneFromClause(); - - if ($this->_addressField OR - (!empty($this->_params['state_province_id_value']) OR - !empty($this->_params['country_id_value'])) - ) { - $this->_from .= " - LEFT JOIN civicrm_address {$this->_aliases['civicrm_address']} - ON {$this->_aliases['civicrm_contact']}.id = {$this->_aliases['civicrm_address']}.contact_id AND - {$this->_aliases['civicrm_address']}.is_primary = 1\n"; - } - - if ($this->_emailField) { - $this->_from .= " - LEFT JOIN civicrm_email {$this->_aliases['civicrm_email']} - ON {$this->_aliases['civicrm_contact']}.id = {$this->_aliases['civicrm_email']}.contact_id AND - {$this->_aliases['civicrm_email']}.is_primary = 1\n"; - } - // include contribution note - if (!empty($this->_params['fields']['contribution_note']) || - !empty($this->_params['note_value']) - ) { - $this->_from .= " - LEFT JOIN civicrm_note {$this->_aliases['civicrm_note']} - ON ( {$this->_aliases['civicrm_note']}.entity_table = 'civicrm_contribution' AND - {$this->_aliases['civicrm_contribution']}.id = {$this->_aliases['civicrm_note']}.entity_id )"; - } - //for contribution batches - if (!empty($this->_params['fields']['batch_id']) || - !empty($this->_params['bid_value']) - ) { - $this->_from .= " - LEFT JOIN civicrm_entity_financial_trxn eft - ON eft.entity_id = {$this->_aliases['civicrm_contribution']}.id AND - eft.entity_table = 'civicrm_contribution' - LEFT JOIN civicrm_entity_batch {$this->_aliases['civicrm_batch']} - ON ({$this->_aliases['civicrm_batch']}.entity_id = eft.financial_trxn_id - AND {$this->_aliases['civicrm_batch']}.entity_table = 'civicrm_financial_trxn')"; - } + $this->appendAdditionalFromJoins(); } public function groupBy() { @@ -560,6 +514,19 @@ public function statistics(&$rows) { return $statistics; } + /** + * This function appears to have been overrriden for the purposes of facilitating soft credits in the report. + * + * The report appears to have 2 different functions: + * 1) contribution report + * 2) soft credit report - showing a row per 'payment engagement' (payment or soft credit). There is a separate + * soft credit report as well. + * + * Somewhat confusingly this report returns multiple rows per contribution when soft credits are included. It feels + * like there is a case to split it into 2 separate reports. + * + * Soft credit functionality is not currently unit tested for this report. + */ public function postProcess() { // get the acl clauses built before we assemble the query $this->buildACLClause($this->_aliases['civicrm_contact']); @@ -589,14 +556,15 @@ public function postProcess() { $this->setPager(); // 2. customize main contribution query for soft credit, and build temp table 2 with soft credit contributions only - $this->from(TRUE); + $this->softCreditFrom(); // also include custom group from if included // since this might be included in select $this->customDataFrom(); $select = str_ireplace('contribution_civireport.total_amount', 'contribution_soft_civireport.amount', $this->_select); $select = str_ireplace("'Contribution' as", "'Soft Credit' as", $select); - if (!empty($this->_groupBy)) { + // We really don't want to join soft credit in if not required. + if (!empty($this->_groupBy) && !$this->noDisplayContributionOrSoftColumn) { $this->_groupBy .= ', contribution_soft_civireport.amount'; } // we inner join with temp1 to restrict soft contributions to those in temp1 table @@ -967,4 +935,65 @@ public function sectionTotals() { } } + /** + * Generate the from clause as it relates to the soft credits. + */ + public function softCreditFrom() { + + $this->_from = " + FROM civireport_contribution_detail_temp1 temp1_civireport + INNER JOIN civicrm_contribution {$this->_aliases['civicrm_contribution']} + ON temp1_civireport.civicrm_contribution_contribution_id = {$this->_aliases['civicrm_contribution']}.id + INNER JOIN civicrm_contribution_soft contribution_soft_civireport + ON contribution_soft_civireport.contribution_id = {$this->_aliases['civicrm_contribution']}.id + INNER JOIN civicrm_contact {$this->_aliases['civicrm_contact']} + ON {$this->_aliases['civicrm_contact']}.id = contribution_soft_civireport.contact_id + {$this->_aclFrom} + "; + + $this->appendAdditionalFromJoins(); + } + + /** + * Append the joins that are required regardless of context. + */ + public function appendAdditionalFromJoins() { + if (!empty($this->_params['ordinality_value'])) { + $this->_from .= " + INNER JOIN (SELECT c.id, IF(COUNT(oc.id) = 0, 0, 1) AS ordinality FROM civicrm_contribution c LEFT JOIN civicrm_contribution oc ON c.contact_id = oc.contact_id AND oc.receive_date < c.receive_date GROUP BY c.id) {$this->_aliases['civicrm_contribution_ordinality']} + ON {$this->_aliases['civicrm_contribution_ordinality']}.id = {$this->_aliases['civicrm_contribution']}.id"; + } + $this->addPhoneFromClause(); + + $this->addAddressFromClause(); + + if ($this->_emailField) { + $this->_from .= " + LEFT JOIN civicrm_email {$this->_aliases['civicrm_email']} + ON {$this->_aliases['civicrm_contact']}.id = {$this->_aliases['civicrm_email']}.contact_id AND + {$this->_aliases['civicrm_email']}.is_primary = 1\n"; + } + // include contribution note + if (!empty($this->_params['fields']['contribution_note']) || + !empty($this->_params['note_value']) + ) { + $this->_from .= " + LEFT JOIN civicrm_note {$this->_aliases['civicrm_note']} + ON ( {$this->_aliases['civicrm_note']}.entity_table = 'civicrm_contribution' AND + {$this->_aliases['civicrm_contribution']}.id = {$this->_aliases['civicrm_note']}.entity_id )"; + } + //for contribution batches + if (!empty($this->_params['fields']['batch_id']) || + !empty($this->_params['bid_value']) + ) { + $this->_from .= " + LEFT JOIN civicrm_entity_financial_trxn eft + ON eft.entity_id = {$this->_aliases['civicrm_contribution']}.id AND + eft.entity_table = 'civicrm_contribution' + LEFT JOIN civicrm_entity_batch {$this->_aliases['civicrm_batch']} + ON ({$this->_aliases['civicrm_batch']}.entity_id = eft.financial_trxn_id + AND {$this->_aliases['civicrm_batch']}.entity_table = 'civicrm_financial_trxn')"; + } + } + } diff --git a/CRM/Report/Form/Contribute/Lybunt.php b/CRM/Report/Form/Contribute/Lybunt.php index f599dea959a..ce1a06c7c09 100644 --- a/CRM/Report/Form/Contribute/Lybunt.php +++ b/CRM/Report/Form/Contribute/Lybunt.php @@ -65,11 +65,13 @@ class CRM_Report_Form_Contribute_Lybunt extends CRM_Report_Form { protected $contactTempTable = ''; /** - * Table containing list of contact IDs. + * This report has been optimised for group filtering. * - * @var string + * CRM-19170 + * + * @var bool */ - protected $groupTempTable = ''; + protected $groupFilterNotOptimised = FALSE; /** * Class constructor. @@ -338,12 +340,7 @@ public function from() { $this->addAddressFromClause(); } else { - if ($this->groupTempTable) { - $this->_from .= "FROM $this->groupTempTable {$this->_aliases['civicrm_contact']}"; - } - else { - $this->_from .= "FROM civicrm_contact {$this->_aliases['civicrm_contact']}"; - } + $this->setFromBase('civicrm_contact'); $this->_from .= " INNER JOIN civicrm_contribution {$this->_aliases['civicrm_contribution']} "; if (!$this->groupTempTable) { @@ -391,75 +388,16 @@ public function whereClause(&$field, $op, $value, $min, $max) { $this->_whereClauses[] = "cont_exclude.id IS NULL"; } } + // Group filtering is already done so skip. + elseif (!empty($field['group']) && $this->contactTempTable) { + return 1; + } else { $clause = parent::whereClause($field, $op, $value, $min, $max); } return $clause; } - /** - * Build where clause for groups. - * - * This has been overridden in order to: - * 1) only build the group clause when filtering - * 2) render the id field as id rather than contact_id in - * order to allow us to join on hte created temp table as if it - * were the contact table. - * - * Further refactoring could break down the parent function so it can be selectively - * leveraged. - * - * @param string $field - * @param mixed $value - * @param string $op - * - * @return string - */ - public function whereGroupClause($field, $value, $op) { - if ($op == 'notin') { - // We do not have an optimisation for this scenario at this stage. Use - // parent. - return parent::whereGroupClause($field, $value, $op); - } - - if (empty($this->groupTempTable)) { - $group = new CRM_Contact_DAO_Group(); - $group->is_active = 1; - $group->find(); - $smartGroups = array(); - while ($group->fetch()) { - if (in_array($group->id, $this->_params['gid_value']) && - $group->saved_search_id - ) { - $smartGroups[] = $group->id; - } - } - - CRM_Contact_BAO_GroupContactCache::check($smartGroups); - - $smartGroupQuery = ''; - if (!empty($smartGroups)) { - $smartGroups = implode(',', $smartGroups); - $smartGroupQuery = " UNION DISTINCT - SELECT DISTINCT smartgroup_contact.contact_id as id - FROM civicrm_group_contact_cache smartgroup_contact - WHERE smartgroup_contact.group_id IN ({$smartGroups}) "; - } - - $sqlOp = $this->getSQLOperator($op); - if (!is_array($value)) { - $value = array($value); - } - $clause = "{$field['dbAlias']} IN (" . implode(', ', $value) . ")"; - - $query = "SELECT DISTINCT {$this->_aliases['civicrm_group']}.contact_id as id - FROM civicrm_group_contact {$this->_aliases['civicrm_group']} - WHERE {$clause} AND {$this->_aliases['civicrm_group']}.status = 'Added' - {$smartGroupQuery} "; - $this->buildGroupTempTable($query); - } - return "1"; - } /** * Generate where clause for last calendar year or fiscal year. * @@ -621,8 +559,7 @@ public function beginPostProcessCommon() { CREATE TEMPORARY TABLE $this->contactTempTable SELECT SQL_CALC_FOUND_ROWS {$this->_aliases['civicrm_contact']}.id as cid {$this->_from} {$this->_where} GROUP BY {$this->_aliases['civicrm_contact']}.id"; - $this->addToDeveloperTab($getContacts); - CRM_Core_DAO::executeQuery($getContacts); + $this->executeReportQuery($getContacts); if (empty($this->_params['charts'])) { $this->setPager(); } @@ -631,27 +568,6 @@ public function beginPostProcessCommon() { $this->_whereClauses = array(); } - /** - * This function is called by both the api (tests) and the UI. - * - * @todo consider moving this to the parent class & reusing the filter then render logic. - * - * (this approach is taken to it's extreme in the extended reports extension with it's 'preconstrain' - * concept). - * - * @param string $clause - */ - public function buildGroupTempTable($clause) { - if (empty($this->groupTempTable)) { - $this->groupTempTable = 'civicrm_report_temp_lybunt_g_' . date('Ymd_') . uniqid(); - CRM_Core_DAO::executeQuery(" - CREATE TEMPORARY TABLE $this->groupTempTable - $clause - "); - CRM_Core_DAO::executeQuery("ALTER TABLE $this->groupTempTable ADD INDEX i_id(id)"); - } - } - /** * Build the report query. * @@ -664,7 +580,7 @@ public function buildGroupTempTable($clause) { * @return string */ public function buildQuery($applyLimit = TRUE) { - + $this->buildGroupTempTable(); // Calling where & select before FROM allows us to build temp tables to use in from. $this->where(); $this->select(); diff --git a/CRM/Report/Form/Contribute/PCP.php b/CRM/Report/Form/Contribute/PCP.php index 52074d09e17..17a1ea5e075 100644 --- a/CRM/Report/Form/Contribute/PCP.php +++ b/CRM/Report/Form/Contribute/PCP.php @@ -33,9 +33,9 @@ * */ class CRM_Report_Form_Contribute_PCP extends CRM_Report_Form { + /** - */ - /** + * Class constructor. */ public function __construct() { $this->_columns = array( diff --git a/CRM/Report/Form/Contribute/Recur.php b/CRM/Report/Form/Contribute/Recur.php index bd71fee2cc7..f4a27bea0e5 100644 --- a/CRM/Report/Form/Contribute/Recur.php +++ b/CRM/Report/Form/Contribute/Recur.php @@ -34,6 +34,19 @@ */ class CRM_Report_Form_Contribute_Recur 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; + /** * Class constructor. */ diff --git a/CRM/Report/Form/Contribute/Repeat.php b/CRM/Report/Form/Contribute/Repeat.php index 30eeeb80f20..e90ba421a99 100644 --- a/CRM/Report/Form/Contribute/Repeat.php +++ b/CRM/Report/Form/Contribute/Repeat.php @@ -82,6 +82,15 @@ class CRM_Report_Form_Contribute_Repeat extends CRM_Report_Form { */ protected $contributionJoinTableColumn; + /** + * This report has been optimised for group filtering. + * + * CRM-19170 + * + * @var bool + */ + protected $groupFilterNotOptimised = FALSE; + /** * Class constructor. */ @@ -399,10 +408,11 @@ public function from() { * @return mixed|string */ public function fromContribution($replaceAliasWith = 'contribution1') { - $from = " FROM civicrm_contribution {$replaceAliasWith} "; + $this->setFromBase('civicrm_contribution', 'contact_id', $replaceAliasWith); + $temp = $this->_aliases['civicrm_contribution']; $this->_aliases['civicrm_contribution'] = $replaceAliasWith; - $this->_from = $from; + $from = $this->_from; $from .= (string) $this->getPermissionedFTQuery($this, 'civicrm_line_item_report', TRUE); $this->_aliases['civicrm_contribution'] = $temp; $this->_where = ''; diff --git a/CRM/Report/Form/Contribute/SoftCredit.php b/CRM/Report/Form/Contribute/SoftCredit.php index dba761272c1..3ef83d16dde 100644 --- a/CRM/Report/Form/Contribute/SoftCredit.php +++ b/CRM/Report/Form/Contribute/SoftCredit.php @@ -52,6 +52,19 @@ class CRM_Report_Form_Contribute_SoftCredit extends CRM_Report_Form { public $_drilldownReport = array('contribute/detail' => 'Link to Detail Report'); + /** + * 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; + /** */ public function __construct() { diff --git a/CRM/Report/Form/Contribute/Summary.php b/CRM/Report/Form/Contribute/Summary.php index 1f3d4e977c1..b70a3f4f53d 100644 --- a/CRM/Report/Form/Contribute/Summary.php +++ b/CRM/Report/Form/Contribute/Summary.php @@ -57,6 +57,15 @@ class CRM_Report_Form_Contribute_Summary extends CRM_Report_Form { 'FISCALYEAR' => 'Fiscal Year', ); + /** + * This report has been optimised for group filtering. + * + * CRM-19170 + * + * @var bool + */ + protected $groupFilterNotOptimised = FALSE; + /** * Class constructor. */ @@ -443,6 +452,9 @@ public static function formRule($fields, $files, $self) { * Set from clause. * * @param string $entity + * + * @todo fix function signature to match parent. Remove hacky passing of $entity + * to acheive unclear results. */ public function from($entity = NULL) { $softCreditJoinType = "LEFT"; @@ -459,8 +471,9 @@ public function from($entity = NULL) { $softCreditJoin .= " AND {$this->_aliases['civicrm_contribution_soft']}.id = (SELECT MIN(id) FROM civicrm_contribution_soft cs WHERE cs.contribution_id = {$this->_aliases['civicrm_contribution']}.id) "; } - $this->_from = " - FROM civicrm_contact {$this->_aliases['civicrm_contact']} {$this->_aclFrom} + $this->setFromBase('civicrm_contact'); + + $this->_from .= " INNER JOIN civicrm_contribution {$this->_aliases['civicrm_contribution']} ON {$this->_aliases['civicrm_contact']}.id = {$this->_aliases['civicrm_contribution']}.contact_id AND {$this->_aliases['civicrm_contribution']}.is_test = 0 @@ -475,13 +488,7 @@ public function from($entity = NULL) { ON ({$this->_aliases['civicrm_contact']}.id = {$this->_aliases['civicrm_phone']}.contact_id AND {$this->_aliases['civicrm_phone']}.is_primary = 1)"; - if ($this->_addressField) { - $this->_from .= " - LEFT JOIN civicrm_address {$this->_aliases['civicrm_address']} - ON {$this->_aliases['civicrm_contact']}.id = - {$this->_aliases['civicrm_address']}.contact_id AND - {$this->_aliases['civicrm_address']}.is_primary = 1\n"; - } + $this->addAddressFromClause(); if (!empty($this->_params['batch_id_value'])) { $this->_from .= " LEFT JOIN civicrm_entity_financial_trxn eft diff --git a/CRM/Report/Form/Contribute/Sybunt.php b/CRM/Report/Form/Contribute/Sybunt.php index 5d5b30a8f40..aa13ae9ff7c 100644 --- a/CRM/Report/Form/Contribute/Sybunt.php +++ b/CRM/Report/Form/Contribute/Sybunt.php @@ -47,6 +47,16 @@ class CRM_Report_Form_Contribute_Sybunt extends CRM_Report_Form { public $_drilldownReport = array('contribute/detail' => 'Link to Detail Report'); /** + * This report has been optimised for group filtering. + * + * CRM-19170 + * + * @var bool + */ + protected $groupFilterNotOptimised = FALSE; + + /** + * Class constructor. */ public function __construct() { $this->_rollup = 'WITH ROLLUP'; @@ -108,7 +118,6 @@ public function __construct() { 'title' => ts('Contact Subtype'), ), ), - 'grouping' => 'contact-fields', 'order_bys' => array( 'sort_name' => array( 'title' => ts('Last Name, First Name'), @@ -317,9 +326,8 @@ public function select() { } public function from() { - - $this->_from = " - FROM civicrm_contribution {$this->_aliases['civicrm_contribution']} + $this->setFromBase('civicrm_contribution', 'contact_id'); + $this->_from .= " INNER JOIN civicrm_contact {$this->_aliases['civicrm_contact']} ON {$this->_aliases['civicrm_contact']}.id = {$this->_aliases['civicrm_contribution']}.contact_id {$this->_aclFrom}"; diff --git a/CRM/Report/Form/Contribute/TopDonor.php b/CRM/Report/Form/Contribute/TopDonor.php index 2481484ea63..de4043f00d1 100644 --- a/CRM/Report/Form/Contribute/TopDonor.php +++ b/CRM/Report/Form/Contribute/TopDonor.php @@ -41,6 +41,19 @@ class CRM_Report_Form_Contribute_TopDonor extends CRM_Report_Form { 'Contribution', ); + /** + * 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; + public $_drilldownReport = array('contribute/detail' => 'Link to Detail Report'); protected $_charts = array( @@ -277,7 +290,7 @@ public function select() { } $this->_selectClauses = $select; - $this->_select = " SELECT * FROM ( SELECT " . implode(', ', $select) . " "; + $this->_select = " SELECT " . implode(', ', $select) . " "; } /** @@ -391,7 +404,7 @@ public function postProcess() { $setVariable = " SET @rows:=0, @rank=0 "; CRM_Core_DAO::singleValueQuery($setVariable); - $sql = " {$this->_select} {$this->_from} {$this->_where} {$this->_groupBy} + $sql = "SELECT * FROM ( {$this->_select} {$this->_from} {$this->_where} {$this->_groupBy} ORDER BY civicrm_contribution_total_amount_sum DESC ) as abc {$this->_outerCluase} $this->_limit "; diff --git a/CRM/Report/Form/Event/IncomeCountSummary.php b/CRM/Report/Form/Event/IncomeCountSummary.php index 93c9793e982..db3577a22e4 100644 --- a/CRM/Report/Form/Event/IncomeCountSummary.php +++ b/CRM/Report/Form/Event/IncomeCountSummary.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Event_IncomeCountSummary extends CRM_Report_Form_Event { @@ -51,8 +49,7 @@ class CRM_Report_Form_Event_IncomeCountSummary extends CRM_Report_Form_Event { public $_drilldownReport = array('event/participantlist' => 'Link to Detail Report'); /** - */ - /** + * Class constructor. */ public function __construct() { diff --git a/CRM/Report/Form/Event/ParticipantListCount.php b/CRM/Report/Form/Event/ParticipantListCount.php index a472ed982dd..0a01456817f 100644 --- a/CRM/Report/Form/Event/ParticipantListCount.php +++ b/CRM/Report/Form/Event/ParticipantListCount.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Event_ParticipantListCount extends CRM_Report_Form_Event { @@ -41,10 +39,23 @@ class CRM_Report_Form_Event_ParticipantListCount extends CRM_Report_Form_Event { 'Participant', 'Event', ); + /** + * 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; public $_drilldownReport = array('event/income' => 'Link to Detail Report'); /** + * Class constructor. */ public function __construct() { $this->_columns = array( diff --git a/CRM/Report/Form/Event/ParticipantListing.php b/CRM/Report/Form/Event/ParticipantListing.php index 9a5bd5160c2..4917c7f27da 100644 --- a/CRM/Report/Form/Event/ParticipantListing.php +++ b/CRM/Report/Form/Event/ParticipantListing.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Event_ParticipantListing extends CRM_Report_Form_Event { @@ -52,8 +50,7 @@ class CRM_Report_Form_Event_ParticipantListing extends CRM_Report_Form_Event { public $_drilldownReport = array('event/income' => 'Link to Detail Report'); /** - */ - /** + * Class constructor. */ public function __construct() { $this->_autoIncludeIndexedFieldsAsOrderBys = 1; diff --git a/CRM/Report/Form/Event/Summary.php b/CRM/Report/Form/Event/Summary.php index 9d1e2234446..42858c57494 100644 --- a/CRM/Report/Form/Event/Summary.php +++ b/CRM/Report/Form/Event/Summary.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Event_Summary extends CRM_Report_Form_Event { @@ -50,8 +48,7 @@ class CRM_Report_Form_Event_Summary extends CRM_Report_Form_Event { public $_drilldownReport = array('event/income' => 'Link to Detail Report'); /** - */ - /** + * Class constructor. */ public function __construct() { diff --git a/CRM/Report/Form/Grant/Detail.php b/CRM/Report/Form/Grant/Detail.php index 461b7f29b6e..91c7f36a427 100644 --- a/CRM/Report/Form/Grant/Detail.php +++ b/CRM/Report/Form/Grant/Detail.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Grant_Detail extends CRM_Report_Form { @@ -41,8 +39,7 @@ class CRM_Report_Form_Grant_Detail extends CRM_Report_Form { ); /** - */ - /** + * Class constructor. */ public function __construct() { $this->_columns = array( diff --git a/CRM/Report/Form/Grant/Statistics.php b/CRM/Report/Form/Grant/Statistics.php index 421a8781ef1..67896422d61 100644 --- a/CRM/Report/Form/Grant/Statistics.php +++ b/CRM/Report/Form/Grant/Statistics.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Grant_Statistics extends CRM_Report_Form { @@ -41,8 +39,7 @@ class CRM_Report_Form_Grant_Statistics extends CRM_Report_Form { protected $_add2groupSupported = FALSE; /** - */ - /** + * Class constructor. */ public function __construct() { $this->_columns = array( diff --git a/CRM/Report/Form/Mailing/Bounce.php b/CRM/Report/Form/Mailing/Bounce.php index a351a96e7c2..f2e220b7cae 100644 --- a/CRM/Report/Form/Mailing/Bounce.php +++ b/CRM/Report/Form/Mailing/Bounce.php @@ -52,6 +52,20 @@ class CRM_Report_Form_Mailing_Bounce 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; + + /** + * Class constructor. */ public function __construct() { $this->_columns = array(); diff --git a/CRM/Report/Form/Mailing/Clicks.php b/CRM/Report/Form/Mailing/Clicks.php index de9b1e20724..2199da9fc6a 100644 --- a/CRM/Report/Form/Mailing/Clicks.php +++ b/CRM/Report/Form/Mailing/Clicks.php @@ -54,8 +54,20 @@ class CRM_Report_Form_Mailing_Clicks 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; + /** + * Class constructor. */ public function __construct() { $this->_columns = array(); diff --git a/CRM/Report/Form/Mailing/Detail.php b/CRM/Report/Form/Mailing/Detail.php index ccbddd44e03..b27ce99a859 100644 --- a/CRM/Report/Form/Mailing/Detail.php +++ b/CRM/Report/Form/Mailing/Detail.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Mailing_Detail extends CRM_Report_Form { @@ -44,6 +42,20 @@ class CRM_Report_Form_Mailing_Detail extends CRM_Report_Form { protected $_exposeContactID = 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(); diff --git a/CRM/Report/Form/Mailing/Opened.php b/CRM/Report/Form/Mailing/Opened.php index 3d6649dd459..e0774aeaedb 100644 --- a/CRM/Report/Form/Mailing/Opened.php +++ b/CRM/Report/Form/Mailing/Opened.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Mailing_Opened extends CRM_Report_Form { @@ -54,8 +52,20 @@ class CRM_Report_Form_Mailing_Opened 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; + /** + * Class constructor. */ public function __construct() { $this->_columns = array(); diff --git a/CRM/Report/Form/Mailing/Summary.php b/CRM/Report/Form/Mailing/Summary.php index d32ef515cae..facc20790af 100644 --- a/CRM/Report/Form/Mailing/Summary.php +++ b/CRM/Report/Form/Mailing/Summary.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Mailing_Summary extends CRM_Report_Form { @@ -50,8 +48,7 @@ class CRM_Report_Form_Mailing_Summary extends CRM_Report_Form { public $campaignEnabled = FALSE; /** - */ - /** + * Class constructor. */ public function __construct() { $this->_columns = array(); diff --git a/CRM/Report/Form/Member/ContributionDetail.php b/CRM/Report/Form/Member/ContributionDetail.php index a66b96dfe55..66d1278a88b 100644 --- a/CRM/Report/Form/Member/ContributionDetail.php +++ b/CRM/Report/Form/Member/ContributionDetail.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Member_ContributionDetail extends CRM_Report_Form { protected $_addressField = FALSE; @@ -45,8 +43,20 @@ class CRM_Report_Form_Member_ContributionDetail 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; + /** + * Class constructor. */ public function __construct() { $config = CRM_Core_Config::singleton(); diff --git a/CRM/Report/Form/Member/Detail.php b/CRM/Report/Form/Member/Detail.php index 4cc84f6ab2f..e29893828f5 100644 --- a/CRM/Report/Form/Member/Detail.php +++ b/CRM/Report/Form/Member/Detail.php @@ -47,6 +47,19 @@ class CRM_Report_Form_Member_Detail extends CRM_Report_Form { protected $_customGroupExtends = array('Membership', 'Contribution'); protected $_customGroupGroupBy = 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. */ diff --git a/CRM/Report/Form/Member/Lapse.php b/CRM/Report/Form/Member/Lapse.php index 7a6cd9a6f80..640f04a3853 100644 --- a/CRM/Report/Form/Member/Lapse.php +++ b/CRM/Report/Form/Member/Lapse.php @@ -45,8 +45,20 @@ class CRM_Report_Form_Member_Lapse extends CRM_Report_Form { public $_drilldownReport = array('member/detail' => 'Link to Detail Report'); /** + * 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() { @@ -78,10 +90,6 @@ public function __construct() { 'title' => ts('First Name'), 'no_repeat' => TRUE, ), - 'id' => array( - 'no_display' => TRUE, - 'required' => TRUE, - ), 'last_name' => array( 'title' => ts('Last Name'), 'no_repeat' => TRUE, diff --git a/CRM/Report/Form/Member/Summary.php b/CRM/Report/Form/Member/Summary.php index d9d7c7371a2..efd78052e57 100644 --- a/CRM/Report/Form/Member/Summary.php +++ b/CRM/Report/Form/Member/Summary.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Member_Summary extends CRM_Report_Form { @@ -48,8 +46,20 @@ class CRM_Report_Form_Member_Summary extends CRM_Report_Form { public $_drilldownReport = array('member/detail' => 'Link to Detail Report'); /** + * 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() { diff --git a/CRM/Report/Form/Pledge/Detail.php b/CRM/Report/Form/Pledge/Detail.php index af3a6a2c473..5c72e55badd 100644 --- a/CRM/Report/Form/Pledge/Detail.php +++ b/CRM/Report/Form/Pledge/Detail.php @@ -40,8 +40,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Pledge_Detail extends CRM_Report_Form { @@ -53,6 +51,19 @@ class CRM_Report_Form_Pledge_Detail extends CRM_Report_Form { 'Individual', ); + /** + * 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. */ diff --git a/CRM/Report/Form/Pledge/Pbnp.php b/CRM/Report/Form/Pledge/Pbnp.php index 2d349bb69cb..1f1a60d11bb 100644 --- a/CRM/Report/Form/Pledge/Pbnp.php +++ b/CRM/Report/Form/Pledge/Pbnp.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Pledge_Pbnp extends CRM_Report_Form { protected $_charts = array( @@ -45,8 +43,7 @@ class CRM_Report_Form_Pledge_Pbnp extends CRM_Report_Form { ); /** - */ - /** + * Class constructor. */ public function __construct() { diff --git a/CRM/Report/Form/Pledge/Summary.php b/CRM/Report/Form/Pledge/Summary.php index 3abdbca7067..b8429ecc42a 100644 --- a/CRM/Report/Form/Pledge/Summary.php +++ b/CRM/Report/Form/Pledge/Summary.php @@ -42,8 +42,20 @@ class CRM_Report_Form_Pledge_Summary extends CRM_Report_Form { protected $_emailField = 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( diff --git a/CRM/Report/Form/Walklist/Walklist.php b/CRM/Report/Form/Walklist/Walklist.php index 361492aa226..b0121139783 100644 --- a/CRM/Report/Form/Walklist/Walklist.php +++ b/CRM/Report/Form/Walklist/Walklist.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Walklist_Walklist extends CRM_Report_Form { protected $_addressField = FALSE; @@ -51,8 +49,7 @@ class CRM_Report_Form_Walklist_Walklist extends CRM_Report_Form { ); /** - */ - /** + * Class constructor. */ public function __construct() { $this->_columns = array( diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php index ed3e3af0a98..f20a713b518 100644 --- a/tests/phpunit/api/v3/ReportTemplateTest.php +++ b/tests/phpunit/api/v3/ReportTemplateTest.php @@ -35,9 +35,15 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase { protected $_apiversion = 3; - public function setUp() { - parent::setUp(); - $this->useTransaction(TRUE); + /** + * Our group reports use an alter so transaction cleanup won't work. + * + * @throws \Exception + */ + public function tearDown() { + $this->quickCleanUpFinancialEntities(); + $this->quickCleanup(array('civicrm_group', 'civicrm_saved_search', 'civicrm_group_contact')); + parent::tearDown(); } public function testReportTemplate() { @@ -203,6 +209,15 @@ public static function getReportTemplates() { return $reportTemplates; } + /** + * Get contribution templates that work with basic filter tests. + * + * These templates require minimal data config. + */ + public static function getContributionReportTemplates() { + return array(array('contribute/summary'), array('contribute/detail'), array('contribute/repeat'), array('contribute/topDonor')); + } + /** * Test Lybunt report to check basic inclusion of a contact who gave in the year before the chosen year. */ @@ -271,4 +286,250 @@ public function testLybuntReportWithFYDataOrderByLastYearAmount() { $this->assertEquals(2, $rows['count'], "Report failed - the sql used to generate the results was " . print_r($rows['metadata']['sql'], TRUE)); } + /** + * Test the group filter works on the contribution summary (with a smart group). + */ + public function testContributionSummaryWithSmartGroupFilter() { + $groupID = $this->setUpPopulatedSmartGroup(); + $rows = $this->callAPISuccess('report_template', 'getrows', array( + 'report_id' => 'contribute/summary', + 'gid_value' => $groupID, + 'gid_op' => 'in', + 'options' => array('metadata' => array('sql')), + )); + $this->assertEquals(3, $rows['values'][0]['civicrm_contribution_total_amount_count']); + + } + + /** + * Test the group filter works on the contribution summary (with a smart group). + */ + public function testContributionSummaryWithNotINSmartGroupFilter() { + $groupID = $this->setUpPopulatedSmartGroup(); + $rows = $this->callAPISuccess('report_template', 'getrows', array( + 'report_id' => 'contribute/summary', + 'gid_value' => $groupID, + 'gid_op' => 'notin', + 'options' => array('metadata' => array('sql')), + )); + $this->assertEquals(2, $rows['values'][0]['civicrm_contribution_total_amount_count']); + + } + + /** + * Test the group filter works on the contribution summary (with a smart group). + * + * @dataProvider getContributionReportTemplates + * + * @param string $template + * Report template unique identifier. + */ + public function testContributionSummaryWithNonSmartGroupFilter($template) { + $groupID = $this->setUpPopulatedGroup(); + $rows = $this->callAPISuccess('report_template', 'getrows', array( + 'report_id' => $template, + 'gid_value' => array($groupID), + 'gid_op' => 'in', + 'options' => array('metadata' => array('sql')), + )); + $this->assertNumberOfContactsInResult(1, $rows, $template); + } + + /** + * Assert the included results match the expected. + * + * There may or may not be a group by in play so the assertion varies a little. + * + * @param int $numberExpected + * @param array $rows + * Rows returned from the report. + * @param string $template + */ + protected function assertNumberOfContactsInResult($numberExpected, $rows, $template) { + if (isset($rows['values'][0]['civicrm_contribution_total_amount_count'])) { + $this->assertEquals($numberExpected, $rows['values'][0]['civicrm_contribution_total_amount_count'], 'wrong row count in ' . $template); + } + else { + $this->assertEquals($numberExpected, count($rows['values']), 'wrong row count in ' . $template); + } + } + + /** + * Test the group filter works on the contribution summary when 2 groups are involved. + */ + public function testContributionSummaryWithTwoGroups() { + $groupID = $this->setUpPopulatedGroup(); + $groupID2 = $this->setUpPopulatedSmartGroup(); + $rows = $this->callAPISuccess('report_template', 'getrows', array( + 'report_id' => 'contribute/summary', + 'gid_value' => array($groupID, $groupID2), + 'gid_op' => 'in', + 'options' => array('metadata' => array('sql')), + )); + $this->assertEquals(4, $rows['values'][0]['civicrm_contribution_total_amount_count']); + } + + /** + * Test the group filter works on the contribution summary when 2 groups are involved. + */ + public function testContributionSummaryWithTwoGroupsWithIntersection() { + $groups = $this->setUpIntersectingGroups(); + + $rows = $this->callAPISuccess('report_template', 'getrows', array( + 'report_id' => 'contribute/summary', + 'gid_value' => $groups, + 'gid_op' => 'in', + 'options' => array('metadata' => array('sql')), + )); + $this->assertEquals(7, $rows['values'][0]['civicrm_contribution_total_amount_count']); + } + + /** + * Set up a smart group for testing. + * + * The smart group includes all Households by filter. In addition an individual + * is created and hard-added and an individual is created that is not added. + * + * One household is hard-added as well as being in the filter. + * + * This gives us a range of scenarios for testing contacts are included only once + * whenever they are hard-added or in the criteria. + * + * @return int + */ + public function setUpPopulatedSmartGroup() { + $household1ID = $this->householdCreate(); + $individual1ID = $this->individualCreate(); + $householdID = $this->householdCreate(); + $individualID = $this->individualCreate(); + $individualIDRemoved = $this->individualCreate(); + $groupID = $this->smartGroupCreate(array(), array('name' => uniqid(), 'title' => uniqid())); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID, + 'contact_id' => $individualIDRemoved, + 'status' => 'Removed', + )); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID, + 'contact_id' => $individualID, + 'status' => 'Added', + )); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID, + 'contact_id' => $householdID, + 'status' => 'Added', + )); + foreach (array($household1ID, $individual1ID, $householdID, $individualID, $individualIDRemoved) as $contactID) { + $this->contributionCreate(array('contact_id' => $contactID, 'invoice_id' => '', 'trxn_id' => '')); + } + + // Refresh the cache for test purposes. It would be better to alter to alter the GroupContact add function to add contacts to the cache. + CRM_Contact_BAO_GroupContactCache::remove($groupID, FALSE); + return $groupID; + } + + /** + * Set up a smart group for testing. + * + * The smart group includes all Households by filter. In addition an individual + * is created and hard-added and an individual is created that is not added. + * + * One household is hard-added as well as being in the filter. + * + * This gives us a range of scenarios for testing contacts are included only once + * whenever they are hard-added or in the criteria. + * + * @return int + */ + public function setUpPopulatedGroup() { + $individual1ID = $this->individualCreate(); + $individualID = $this->individualCreate(); + $individualIDRemoved = $this->individualCreate(); + $groupID = $this->groupCreate(array('name' => uniqid(), 'title' => uniqid())); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID, + 'contact_id' => $individualIDRemoved, + 'status' => 'Removed', + )); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID, + 'contact_id' => $individualID, + 'status' => 'Added', + )); + + foreach (array($individual1ID, $individualID, $individualIDRemoved) as $contactID) { + $this->contributionCreate(array('contact_id' => $contactID, 'invoice_id' => '', 'trxn_id' => '')); + } + + // Refresh the cache for test purposes. It would be better to alter to alter the GroupContact add function to add contacts to the cache. + CRM_Contact_BAO_GroupContactCache::remove($groupID, FALSE); + return $groupID; + } + + /** + * @return array + */ + public function setUpIntersectingGroups() { + $groupID = $this->setUpPopulatedGroup(); + $groupID2 = $this->setUpPopulatedSmartGroup(); + $addedToBothIndividualID = $this->individualCreate(); + $removedFromBothIndividualID = $this->individualCreate(); + $addedToSmartGroupRemovedFromOtherIndividualID = $this->individualCreate(); + $removedFromSmartGroupAddedToOtherIndividualID = $this->individualCreate(); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID, + 'contact_id' => $addedToBothIndividualID, + 'status' => 'Added', + )); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID2, + 'contact_id' => $addedToBothIndividualID, + 'status' => 'Added', + )); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID, + 'contact_id' => $removedFromBothIndividualID, + 'status' => 'Removed', + )); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID2, + 'contact_id' => $removedFromBothIndividualID, + 'status' => 'Removed', + )); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID2, + 'contact_id' => $addedToSmartGroupRemovedFromOtherIndividualID, + 'status' => 'Added', + )); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID, + 'contact_id' => $addedToSmartGroupRemovedFromOtherIndividualID, + 'status' => 'Removed', + )); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID, + 'contact_id' => $removedFromSmartGroupAddedToOtherIndividualID, + 'status' => 'Added', + )); + $this->callAPISuccess('GroupContact', 'create', array( + 'group_id' => $groupID2, + 'contact_id' => $removedFromSmartGroupAddedToOtherIndividualID, + 'status' => 'Removed', + )); + + foreach (array( + $addedToBothIndividualID, + $removedFromBothIndividualID, + $addedToSmartGroupRemovedFromOtherIndividualID, + $removedFromSmartGroupAddedToOtherIndividualID, + ) as $contactID) { + $this->contributionCreate(array( + 'contact_id' => $contactID, + 'invoice_id' => '', + 'trxn_id' => '', + )); + } + return array($groupID, $groupID2); + } + }