From 2daaaca0013b8f1076cb6d3d1b8c979858cbb415 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 4 Aug 2016 16:03:38 +1200 Subject: [PATCH 01/10] CRM-19175 fix add to group when viewing contribution detail without soft credits. This also includes some tidy up of the code, getting rid of the overriding of the signature --- CRM/Report/Form/Contribute/Detail.php | 140 ++++++++++++++----------- CRM/Report/Form/Contribute/Summary.php | 8 +- 2 files changed, 79 insertions(+), 69 deletions(-) diff --git a/CRM/Report/Form/Contribute/Detail.php b/CRM/Report/Form/Contribute/Detail.php index 7ebd1cdcb19d..6875b04b091c 100644 --- a/CRM/Report/Form/Contribute/Detail.php +++ b/CRM/Report/Form/Contribute/Detail.php @@ -378,9 +378,9 @@ public function orderBy() { } /** - * @param bool $softcredit + * Set the FROM clause for the report. */ - public function from($softcredit = FALSE) { + public function from() { $this->_from = " FROM civicrm_contact {$this->_aliases['civicrm_contact']} {$this->_aclFrom} INNER JOIN civicrm_contribution {$this->_aliases['civicrm_contribution']} @@ -398,64 +398,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 +503,18 @@ public function statistics(&$rows) { return $statistics; } + /** + * This function appears to have been overrriden for the purposes of facilitating soft credits in the report. + * + * An alternative approach would have been to have had 2 reports. + * 1) contribution report with optional join extending the retrievable fields & filters with soft credit data + * 2) soft credit report - showing a row per 'payment engagement' (payment or soft credit). + * + * As it is many people are confused by the duplicate rows in 'soft credit mode' and this report is complex + * and slowed down by soft credit calculations regardless of whether that information is desired. + * + * 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 +544,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 +923,64 @@ 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/Summary.php b/CRM/Report/Form/Contribute/Summary.php index ee09282b3ddd..20a155de38f6 100644 --- a/CRM/Report/Form/Contribute/Summary.php +++ b/CRM/Report/Form/Contribute/Summary.php @@ -475,13 +475,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 From fccce871fb3948f163f2f42e54393cf7768c3070 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 4 Aug 2016 20:21:00 +1200 Subject: [PATCH 02/10] CRM-19170 preliminary tidy up. Remove unused return variable, fix comments, remove duplicate key --- CRM/Report/Form.php | 2 +- CRM/Report/Form/ActivitySummary.php | 3 +-- CRM/Report/Form/Case/Demographics.php | 2 -- CRM/Report/Form/Case/Summary.php | 5 +---- CRM/Report/Form/Case/TimeSpent.php | 6 ++---- CRM/Report/Form/Contact/CurrentEmployer.php | 3 +-- CRM/Report/Form/Contact/Detail.php | 2 -- CRM/Report/Form/Contact/Log.php | 5 +---- CRM/Report/Form/Contact/Summary.php | 2 -- CRM/Report/Form/Contribute/Bookkeeping.php | 2 -- CRM/Report/Form/Contribute/Detail.php | 11 ++++++----- CRM/Report/Form/Contribute/PCP.php | 4 ++-- CRM/Report/Form/Contribute/Summary.php | 3 +++ CRM/Report/Form/Event/IncomeCountSummary.php | 5 +---- CRM/Report/Form/Event/ParticipantListCount.php | 3 +-- CRM/Report/Form/Event/ParticipantListing.php | 5 +---- CRM/Report/Form/Event/Summary.php | 5 +---- CRM/Report/Form/Grant/Detail.php | 5 +---- CRM/Report/Form/Grant/Statistics.php | 5 +---- CRM/Report/Form/Mailing/Detail.php | 2 -- CRM/Report/Form/Mailing/Opened.php | 3 +-- CRM/Report/Form/Mailing/Summary.php | 5 +---- CRM/Report/Form/Member/ContributionDetail.php | 3 +-- CRM/Report/Form/Member/Lapse.php | 5 +---- CRM/Report/Form/Member/Summary.php | 3 +-- CRM/Report/Form/Pledge/Detail.php | 2 -- CRM/Report/Form/Pledge/Pbnp.php | 5 +---- CRM/Report/Form/Pledge/Summary.php | 1 + CRM/Report/Form/Walklist/Walklist.php | 5 +---- 29 files changed, 32 insertions(+), 80 deletions(-) diff --git a/CRM/Report/Form.php b/CRM/Report/Form.php index 04be6ad607de..7555efed3d8d 100644 --- a/CRM/Report/Form.php +++ b/CRM/Report/Form.php @@ -3171,7 +3171,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; diff --git a/CRM/Report/Form/ActivitySummary.php b/CRM/Report/Form/ActivitySummary.php index 43c2169719d6..6633d599f634 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 { @@ -42,6 +40,7 @@ class CRM_Report_Form_ActivitySummary extends CRM_Report_Form { /** */ /** + * 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 3badc118d50f..0ec779030fb9 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 { diff --git a/CRM/Report/Form/Case/Summary.php b/CRM/Report/Form/Case/Summary.php index ea39d69d3635..a5ef3c4f6a21 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 c90270576470..e8d7f62def7b 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 e5ea4b76d598..d8a9d127af21 100644 --- a/CRM/Report/Form/Contact/CurrentEmployer.php +++ b/CRM/Report/Form/Contact/CurrentEmployer.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2016 - * $Id$ - * */ class CRM_Report_Form_Contact_CurrentEmployer extends CRM_Report_Form { @@ -44,6 +42,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 417cf07e079a..dc1d46704467 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; diff --git a/CRM/Report/Form/Contact/Log.php b/CRM/Report/Form/Contact/Log.php index 3cdc9f06b241..5c8833c57da2 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/Summary.php b/CRM/Report/Form/Contact/Summary.php index 40b44d581b3b..3a496d599c53 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 { diff --git a/CRM/Report/Form/Contribute/Bookkeeping.php b/CRM/Report/Form/Contribute/Bookkeeping.php index 1f818f4f74e4..b9b921561847 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; diff --git a/CRM/Report/Form/Contribute/Detail.php b/CRM/Report/Form/Contribute/Detail.php index 6875b04b091c..c1c2cb656651 100644 --- a/CRM/Report/Form/Contribute/Detail.php +++ b/CRM/Report/Form/Contribute/Detail.php @@ -506,12 +506,13 @@ public function statistics(&$rows) { /** * This function appears to have been overrriden for the purposes of facilitating soft credits in the report. * - * An alternative approach would have been to have had 2 reports. - * 1) contribution report with optional join extending the retrievable fields & filters with soft credit data - * 2) soft credit report - showing a row per 'payment engagement' (payment or soft credit). + * 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. * - * As it is many people are confused by the duplicate rows in 'soft credit mode' and this report is complex - * and slowed down by soft credit calculations regardless of whether that information is desired. + * 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. */ diff --git a/CRM/Report/Form/Contribute/PCP.php b/CRM/Report/Form/Contribute/PCP.php index 52074d09e174..17a1ea5e0755 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/Summary.php b/CRM/Report/Form/Contribute/Summary.php index 20a155de38f6..af2383871611 100644 --- a/CRM/Report/Form/Contribute/Summary.php +++ b/CRM/Report/Form/Contribute/Summary.php @@ -443,6 +443,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"; diff --git a/CRM/Report/Form/Event/IncomeCountSummary.php b/CRM/Report/Form/Event/IncomeCountSummary.php index f502c2d0a4bc..9b3a516b34ec 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 bc95ee0b41aa..8130e4206b6a 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 { @@ -45,6 +43,7 @@ class CRM_Report_Form_Event_ParticipantListCount extends CRM_Report_Form_Event { 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 e30d3dae8484..c828efca2489 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 17ee01029e8f..e67430c59f2a 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 461b7f29b6e2..91c7f36a4274 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 421a8781ef1f..67896422d61c 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/Detail.php b/CRM/Report/Form/Mailing/Detail.php index ccbddd44e033..c0f31303b3ab 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 { diff --git a/CRM/Report/Form/Mailing/Opened.php b/CRM/Report/Form/Mailing/Opened.php index 3d6649dd4590..c41bc77d92ce 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 { @@ -56,6 +54,7 @@ class CRM_Report_Form_Mailing_Opened extends CRM_Report_Form { /** */ /** + * 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 2c2a0b798d13..740de3a06fbd 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 6a6639d4ad66..fc98850a8067 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; @@ -47,6 +45,7 @@ class CRM_Report_Form_Member_ContributionDetail extends CRM_Report_Form { /** */ /** + * Class constructor. */ public function __construct() { $config = CRM_Core_Config::singleton(); diff --git a/CRM/Report/Form/Member/Lapse.php b/CRM/Report/Form/Member/Lapse.php index d746013baa41..679eb2eb5aad 100644 --- a/CRM/Report/Form/Member/Lapse.php +++ b/CRM/Report/Form/Member/Lapse.php @@ -47,6 +47,7 @@ class CRM_Report_Form_Member_Lapse extends CRM_Report_Form { /** */ /** + * Class constructor. */ public function __construct() { @@ -78,10 +79,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 f7d2f77890d7..ff78ca29502a 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 { @@ -50,6 +48,7 @@ class CRM_Report_Form_Member_Summary extends CRM_Report_Form { /** */ /** + * Class constructor. */ public function __construct() { diff --git a/CRM/Report/Form/Pledge/Detail.php b/CRM/Report/Form/Pledge/Detail.php index b504d1905b03..0616b619038c 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 { diff --git a/CRM/Report/Form/Pledge/Pbnp.php b/CRM/Report/Form/Pledge/Pbnp.php index 5bbf9d13d6b9..8e1e5e3c8cb0 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 a5b4d022d91b..d07ce045799d 100644 --- a/CRM/Report/Form/Pledge/Summary.php +++ b/CRM/Report/Form/Pledge/Summary.php @@ -44,6 +44,7 @@ class CRM_Report_Form_Pledge_Summary extends CRM_Report_Form { /** */ /** + * 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 361492aa2260..b01211397832 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( From 75db18fb91cd151f58c1bbe92d8f5c5109a7863a Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 4 Aug 2016 20:27:17 +1200 Subject: [PATCH 03/10] CRM-19170 preliminary tidy up on top donor report for testability --- CRM/Report/Form/Contribute/TopDonor.php | 4 +- tests/phpunit/api/v3/ReportTemplateTest.php | 267 +++++++++++++++++++- 2 files changed, 266 insertions(+), 5 deletions(-) diff --git a/CRM/Report/Form/Contribute/TopDonor.php b/CRM/Report/Form/Contribute/TopDonor.php index 02362cd317f6..7b658fd3a9ce 100644 --- a/CRM/Report/Form/Contribute/TopDonor.php +++ b/CRM/Report/Form/Contribute/TopDonor.php @@ -277,7 +277,7 @@ public function select() { } $this->_selectClauses = $select; - $this->_select = " SELECT * FROM ( SELECT " . implode(', ', $select) . " "; + $this->_select = " SELECT " . implode(', ', $select) . " "; } /** @@ -391,7 +391,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/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php index ed3e3af0a987..2e26a2710d33 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' => 'not in', + '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); + } + } From 2460e251fa3d65137897ea7f007ae2a20be9bcfb Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 4 Aug 2016 20:32:07 +1200 Subject: [PATCH 04/10] CRM-19170 optimise group filter on contribution summary report. This reduced the time in testing from 190+ seconds (killed at that point) to ~1 second for a report with a small result set on a large DB --- CRM/Report/Form.php | 176 ++++++++++++++++++-- CRM/Report/Form/Contribute/Summary.php | 14 +- tests/phpunit/api/v3/ReportTemplateTest.php | 2 +- 3 files changed, 177 insertions(+), 15 deletions(-) diff --git a/CRM/Report/Form.php b/CRM/Report/Form.php index 7555efed3d8d..2755a74d9f7b 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) { @@ -2618,6 +2646,7 @@ public function beginPostProcessCommon() {} * @return string */ public function buildQuery($applyLimit = TRUE) { + $this->buildGroupTempTable(); $this->select(); $this->from(); $this->customDataFrom(); @@ -3381,7 +3410,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 @@ -3389,8 +3423,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(); @@ -3433,6 +3466,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. * @@ -4736,4 +4846,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/Contribute/Summary.php b/CRM/Report/Form/Contribute/Summary.php index af2383871611..934373334393 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. */ @@ -462,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 diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php index 2e26a2710d33..f20a713b518a 100644 --- a/tests/phpunit/api/v3/ReportTemplateTest.php +++ b/tests/phpunit/api/v3/ReportTemplateTest.php @@ -309,7 +309,7 @@ public function testContributionSummaryWithNotINSmartGroupFilter() { $rows = $this->callAPISuccess('report_template', 'getrows', array( 'report_id' => 'contribute/summary', 'gid_value' => $groupID, - 'gid_op' => 'not in', + 'gid_op' => 'notin', 'options' => array('metadata' => array('sql')), )); $this->assertEquals(2, $rows['values'][0]['civicrm_contribution_total_amount_count']); From b1683dfe3d7d2b076d41b48ba6f1126ee0f7cd34 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 4 Aug 2016 20:37:52 +1200 Subject: [PATCH 05/10] CRM-19170 explicitly mark the reports not taking advantage of efficient group queries. The process of switching them over is not hard but they need to be looked at one-be-one --- CRM/Report/Form/Activity.php | 13 +++++++++++++ CRM/Report/Form/ActivitySummary.php | 11 +++++++++++ CRM/Report/Form/Case/Demographics.php | 13 ++++++++++++- CRM/Report/Form/Contact/CurrentEmployer.php | 13 +++++++++++++ CRM/Report/Form/Contact/Detail.php | 13 +++++++++++++ CRM/Report/Form/Contact/Relationship.php | 13 +++++++++++++ CRM/Report/Form/Contact/Summary.php | 14 ++++++++++++++ CRM/Report/Form/Contribute/Bookkeeping.php | 14 ++++++++++++++ CRM/Report/Form/Contribute/Detail.php | 10 ++++++++++ CRM/Report/Form/Contribute/Recur.php | 13 +++++++++++++ CRM/Report/Form/Contribute/Repeat.php | 9 +++++++++ CRM/Report/Form/Contribute/SoftCredit.php | 13 +++++++++++++ CRM/Report/Form/Contribute/Sybunt.php | 13 +++++++++++++ CRM/Report/Form/Contribute/TopDonor.php | 13 +++++++++++++ CRM/Report/Form/Event/ParticipantListCount.php | 12 ++++++++++++ CRM/Report/Form/Mailing/Bounce.php | 14 ++++++++++++++ CRM/Report/Form/Mailing/Clicks.php | 12 ++++++++++++ CRM/Report/Form/Mailing/Detail.php | 14 ++++++++++++++ CRM/Report/Form/Mailing/Opened.php | 11 +++++++++++ CRM/Report/Form/Member/ContributionDetail.php | 11 +++++++++++ CRM/Report/Form/Member/Detail.php | 13 +++++++++++++ CRM/Report/Form/Member/Lapse.php | 11 +++++++++++ CRM/Report/Form/Member/Summary.php | 11 +++++++++++ CRM/Report/Form/Pledge/Detail.php | 13 +++++++++++++ CRM/Report/Form/Pledge/Summary.php | 11 +++++++++++ 25 files changed, 307 insertions(+), 1 deletion(-) diff --git a/CRM/Report/Form/Activity.php b/CRM/Report/Form/Activity.php index a78eda2db290..3e985449df7a 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 6633d599f634..a57a2f0df5e2 100644 --- a/CRM/Report/Form/ActivitySummary.php +++ b/CRM/Report/Form/ActivitySummary.php @@ -38,7 +38,18 @@ 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. */ diff --git a/CRM/Report/Form/Case/Demographics.php b/CRM/Report/Form/Case/Demographics.php index 0ec779030fb9..ece79d69b6cb 100644 --- a/CRM/Report/Form/Case/Demographics.php +++ b/CRM/Report/Form/Case/Demographics.php @@ -37,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/Contact/CurrentEmployer.php b/CRM/Report/Form/Contact/CurrentEmployer.php index d8a9d127af21..15858960874b 100644 --- a/CRM/Report/Form/Contact/CurrentEmployer.php +++ b/CRM/Report/Form/Contact/CurrentEmployer.php @@ -32,6 +32,19 @@ */ 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( diff --git a/CRM/Report/Form/Contact/Detail.php b/CRM/Report/Form/Contact/Detail.php index dc1d46704467..7345c5d1ac94 100644 --- a/CRM/Report/Form/Contact/Detail.php +++ b/CRM/Report/Form/Contact/Detail.php @@ -42,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/Relationship.php b/CRM/Report/Form/Contact/Relationship.php index 7d84f079a68e..de61cc4c228f 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 3a496d599c53..b3759306ae04 100644 --- a/CRM/Report/Form/Contact/Summary.php +++ b/CRM/Report/Form/Contact/Summary.php @@ -48,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 b9b921561847..71f56ea8eb5b 100644 --- a/CRM/Report/Form/Contribute/Bookkeeping.php +++ b/CRM/Report/Form/Contribute/Bookkeeping.php @@ -45,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 c1c2cb656651..0ff2ceacd85f 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; diff --git a/CRM/Report/Form/Contribute/Recur.php b/CRM/Report/Form/Contribute/Recur.php index 9e5ff3ccec5f..9d7d5a7c9b7b 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 ae608b46bbab..866b2e894605 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. */ diff --git a/CRM/Report/Form/Contribute/SoftCredit.php b/CRM/Report/Form/Contribute/SoftCredit.php index 70b99eb31349..4975042cc2f0 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/Sybunt.php b/CRM/Report/Form/Contribute/Sybunt.php index fae15cf5e6d6..73effdba25fb 100644 --- a/CRM/Report/Form/Contribute/Sybunt.php +++ b/CRM/Report/Form/Contribute/Sybunt.php @@ -46,6 +46,19 @@ class CRM_Report_Form_Contribute_Sybunt 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/TopDonor.php b/CRM/Report/Form/Contribute/TopDonor.php index 7b658fd3a9ce..bc8862cafb02 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( diff --git a/CRM/Report/Form/Event/ParticipantListCount.php b/CRM/Report/Form/Event/ParticipantListCount.php index 8130e4206b6a..d7796b8379fb 100644 --- a/CRM/Report/Form/Event/ParticipantListCount.php +++ b/CRM/Report/Form/Event/ParticipantListCount.php @@ -39,6 +39,18 @@ 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'); diff --git a/CRM/Report/Form/Mailing/Bounce.php b/CRM/Report/Form/Mailing/Bounce.php index c994faa1eeb4..5e753d5297da 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 de9b1e207241..2199da9fc6ac 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 c0f31303b3ab..b27ce99a8596 100644 --- a/CRM/Report/Form/Mailing/Detail.php +++ b/CRM/Report/Form/Mailing/Detail.php @@ -42,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 c41bc77d92ce..e0774aeaedb5 100644 --- a/CRM/Report/Form/Mailing/Opened.php +++ b/CRM/Report/Form/Mailing/Opened.php @@ -52,7 +52,18 @@ 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. */ diff --git a/CRM/Report/Form/Member/ContributionDetail.php b/CRM/Report/Form/Member/ContributionDetail.php index fc98850a8067..89377a64ebc9 100644 --- a/CRM/Report/Form/Member/ContributionDetail.php +++ b/CRM/Report/Form/Member/ContributionDetail.php @@ -43,7 +43,18 @@ 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. */ diff --git a/CRM/Report/Form/Member/Detail.php b/CRM/Report/Form/Member/Detail.php index fde2da1993de..c819c043b838 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 679eb2eb5aad..b12dceed93e5 100644 --- a/CRM/Report/Form/Member/Lapse.php +++ b/CRM/Report/Form/Member/Lapse.php @@ -45,7 +45,18 @@ 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. */ diff --git a/CRM/Report/Form/Member/Summary.php b/CRM/Report/Form/Member/Summary.php index ff78ca29502a..a7810d24907a 100644 --- a/CRM/Report/Form/Member/Summary.php +++ b/CRM/Report/Form/Member/Summary.php @@ -46,7 +46,18 @@ 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. */ diff --git a/CRM/Report/Form/Pledge/Detail.php b/CRM/Report/Form/Pledge/Detail.php index 0616b619038c..c3f1bb24d77f 100644 --- a/CRM/Report/Form/Pledge/Detail.php +++ b/CRM/Report/Form/Pledge/Detail.php @@ -51,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/Summary.php b/CRM/Report/Form/Pledge/Summary.php index d07ce045799d..b0a13a6a0f14 100644 --- a/CRM/Report/Form/Pledge/Summary.php +++ b/CRM/Report/Form/Pledge/Summary.php @@ -42,7 +42,18 @@ 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. */ From a8687d873e0ff289437c4accc29304178a3c3449 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 4 Aug 2016 20:42:10 +1200 Subject: [PATCH 06/10] CRM-19170 update Contribution Detail & Repeat reports to use optimised group filter --- CRM/Report/Form/Contribute/Detail.php | 28 ++++++++++++++------------- CRM/Report/Form/Contribute/Repeat.php | 5 +++-- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/CRM/Report/Form/Contribute/Detail.php b/CRM/Report/Form/Contribute/Detail.php index 0ff2ceacd85f..84e67a4c2465 100644 --- a/CRM/Report/Form/Contribute/Detail.php +++ b/CRM/Report/Form/Contribute/Detail.php @@ -391,10 +391,11 @@ public function orderBy() { * Set the FROM clause for the report. */ public function from() { - $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"; + $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' @@ -939,15 +940,16 @@ public function sectionTotals() { */ 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->_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(); } diff --git a/CRM/Report/Form/Contribute/Repeat.php b/CRM/Report/Form/Contribute/Repeat.php index 866b2e894605..46542ddbc88f 100644 --- a/CRM/Report/Form/Contribute/Repeat.php +++ b/CRM/Report/Form/Contribute/Repeat.php @@ -408,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 = ''; From 8edad8a325be651176440f81caa03ef9393a3d1d Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 4 Aug 2016 21:21:31 +1200 Subject: [PATCH 07/10] CRM-19170 update Lybunt to utilse shared code now it is moved --- CRM/Report/Form/Contribute/Lybunt.php | 108 +++----------------------- 1 file changed, 12 insertions(+), 96 deletions(-) diff --git a/CRM/Report/Form/Contribute/Lybunt.php b/CRM/Report/Form/Contribute/Lybunt.php index c17bc0b8160b..3bbbad292c77 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(); From bc5f6d91758099c799938031d160db67494d1c6c Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 4 Aug 2016 20:32:07 +1200 Subject: [PATCH 08/10] CRM-19170 optimise group filter on contribution summary report. This reduced the time in testing from 190+ seconds (killed at that point) to ~1 second for a report with a small result set on a large DB --- CRM/Report/Form.php | 1 + 1 file changed, 1 insertion(+) diff --git a/CRM/Report/Form.php b/CRM/Report/Form.php index 2755a74d9f7b..4b3a0265a554 100644 --- a/CRM/Report/Form.php +++ b/CRM/Report/Form.php @@ -1341,6 +1341,7 @@ protected function addToDeveloperTab($sql) { $this->sqlFormattedArray[] = $sql; $this->assign('sql', implode(';



', $this->sqlFormattedArray)); } + } /** From 3d9846dd4dd53edbe3ca3932c09c8e28636c50be Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 4 Aug 2016 23:01:18 +1200 Subject: [PATCH 09/10] CRM-19170 update sybunt to optimise group filter --- CRM/Report/Form/Contribute/Sybunt.php | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/CRM/Report/Form/Contribute/Sybunt.php b/CRM/Report/Form/Contribute/Sybunt.php index 73effdba25fb..1586d3487b65 100644 --- a/CRM/Report/Form/Contribute/Sybunt.php +++ b/CRM/Report/Form/Contribute/Sybunt.php @@ -47,19 +47,16 @@ class CRM_Report_Form_Contribute_Sybunt 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. + * This report has been optimised for group filtering. * * CRM-19170 * * @var bool */ - protected $groupFilterNotOptimised = TRUE; + protected $groupFilterNotOptimised = FALSE; /** + * Class constructor. */ public function __construct() { $this->_rollup = 'WITH ROLLUP'; @@ -121,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'), @@ -330,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}"; From 58f4703f8774e61eddaa1e4c19ab3583cf507577 Mon Sep 17 00:00:00 2001 From: robbrandt Date: Thu, 4 Aug 2016 23:05:33 +1200 Subject: [PATCH 10/10] CRM-19061 Update sybunt to show line items --- CRM/Report/Form/Contribute/Sybunt.php | 222 +++++++++++++++++++++----- 1 file changed, 179 insertions(+), 43 deletions(-) diff --git a/CRM/Report/Form/Contribute/Sybunt.php b/CRM/Report/Form/Contribute/Sybunt.php index 1586d3487b65..0b67a7e43cd8 100644 --- a/CRM/Report/Form/Contribute/Sybunt.php +++ b/CRM/Report/Form/Contribute/Sybunt.php @@ -174,9 +174,6 @@ public function __construct() { ), ), ), - 'civicrm_line_item' => array( - 'dao' => 'CRM_Price_DAO_LineItem', - ), 'civicrm_email' => array( 'dao' => 'CRM_Core_DAO_Email', 'grouping' => 'contact-field', @@ -200,6 +197,36 @@ public function __construct() { ); $this->_columns += $this->addAddressFields(); $this->_columns += array( + 'civicrm_line_item' => array( + 'dao' => 'CRM_Price_DAO_LineItem', + 'fields' => array( + 'line_item_financial_type_id' => array( + 'name' => 'financial_type_id', + 'title' => ts('Line Item Financial Type'), + 'default' => FALSE, + ), + 'line_total' => array( + 'title' => ts('Yearly Line Item Totals (Use only with Line Item Financial Type filters in order to avoid double counting)'), + 'type' => CRM_Utils_Type::T_MONEY, + 'default' => FALSE, + ), + ), + 'filters' => array( + 'line_item_financial_type_id' => array( + 'name' => 'financial_type_id', + 'title' => ts('Line Item Financial Type'), + 'type' => CRM_Utils_Type::T_INT, + 'operatorType' => CRM_Report_Form::OP_MULTISELECT, + 'options' => CRM_Financial_BAO_FinancialType::getAvailableFinancialTypes(), + ), + ), + 'group_bys' => array( + 'line_item_financial_type_id' => array( + 'name' => 'financial_type_id', + 'title' => ts('Line Item Financial Type'), + ), + ), + ), 'civicrm_contribution' => array( 'dao' => 'CRM_Contribute_DAO_Contribution', 'fields' => array( @@ -210,10 +237,8 @@ public function __construct() { 'no_repeat' => TRUE, ), 'total_amount' => array( - 'title' => ts('Total Amount'), - 'no_display' => TRUE, - 'required' => TRUE, - 'no_repeat' => TRUE, + 'title' => ts('Yearly Contribution Totals'), + 'default' => TRUE, ), 'receive_date' => array( 'title' => ts('Year'), @@ -221,6 +246,11 @@ public function __construct() { 'required' => TRUE, 'no_repeat' => TRUE, ), + 'financial_type_id' => array( + 'name' => 'financial_type_id', + 'title' => ts('Contribution Financial Type'), + 'default' => FALSE, + ), ), 'filters' => array( 'yid' => array( @@ -244,6 +274,12 @@ public function __construct() { 'default' => array('1'), ), ), + 'group_bys' => array( + 'financial_type_id' => array( + 'name' => 'financial_type_id', + 'title' => ts('Contribution Financial Type'), + ), + ), ), ); @@ -282,27 +318,34 @@ public function select() { foreach ($this->_columns as $tableName => $table) { if (array_key_exists('fields', $table)) { foreach ($table['fields'] as $fieldName => $field) { + // if field is required, or if specified, or, default to include total amount if line total isn't specified (so that it behaves like it did before line total was an option and total amount was required + if (!empty($field['required']) || !empty($this->_params['fields'][$fieldName]) || (empty($this->_params['fields']['line_total']) && $fieldName == 'total_amount')) { + if ($fieldName == 'total_amount' || $fieldName == 'line_total') { + if ($fieldName == 'line_total') { + $select[] = "SUM(line_item_civireport.line_total) as {$tableName}_{$fieldName}"; + $labelprefix = "Line Items"; + $columnprefix = "line_items"; + } + else {// if total_amount or default to total_amount + $select[] = "SUM({$field['dbAlias']}) as {$tableName}_{$fieldName}"; + $labelprefix = "Contributions"; + $columnprefix = "contributions"; + } - if (!empty($field['required']) || - !empty($this->_params['fields'][$fieldName]) - ) { - if ($fieldName == 'total_amount') { - $select[] = "SUM({$field['dbAlias']}) as {$tableName}_{$fieldName}"; - - $this->_columnHeaders["civicrm_upto_{$upTo_year}"]['type'] = $field['type']; - $this->_columnHeaders["civicrm_upto_{$upTo_year}"]['title'] = "Up To $upTo_year"; + $this->_columnHeaders["{$columnprefix}_civicrm_upto_{$upTo_year}"]['type'] = $field['type']; + $this->_columnHeaders["{$columnprefix}_civicrm_upto_{$upTo_year}"]['title'] = "$labelprefix Up To $upTo_year"; - $this->_columnHeaders["year_{$previous_ppyear}"]['type'] = $field['type']; - $this->_columnHeaders["year_{$previous_ppyear}"]['title'] = $previous_ppyear; + $this->_columnHeaders["{$columnprefix}_year_{$previous_ppyear}"]['type'] = $field['type']; + $this->_columnHeaders["{$columnprefix}_year_{$previous_ppyear}"]['title'] = "$labelprefix for $previous_ppyear"; - $this->_columnHeaders["year_{$previous_pyear}"]['type'] = $field['type']; - $this->_columnHeaders["year_{$previous_pyear}"]['title'] = $previous_pyear; + $this->_columnHeaders["{$columnprefix}_year_{$previous_pyear}"]['type'] = $field['type']; + $this->_columnHeaders["{$columnprefix}_year_{$previous_pyear}"]['title'] = "$labelprefix for $previous_pyear"; - $this->_columnHeaders["year_{$previous_year}"]['type'] = $field['type']; - $this->_columnHeaders["year_{$previous_year}"]['title'] = $previous_year; + $this->_columnHeaders["{$columnprefix}_year_{$previous_year}"]['type'] = $field['type']; + $this->_columnHeaders["{$columnprefix}_year_{$previous_year}"]['title'] = "$labelprefix for $previous_year"; - $this->_columnHeaders["civicrm_life_time_total"]['type'] = $field['type']; - $this->_columnHeaders["civicrm_life_time_total"]['title'] = 'LifeTime';; + $this->_columnHeaders["{$columnprefix}_life_time_total"]['type'] = $field['type']; + $this->_columnHeaders["{$columnprefix}_life_time_total"]['title'] = "$labelprefix for LifeTime"; } elseif ($fieldName == 'receive_date') { $select[] = self::fiscalYearOffset($field['dbAlias']) . @@ -331,7 +374,11 @@ public function from() { INNER JOIN civicrm_contact {$this->_aliases['civicrm_contact']} ON {$this->_aliases['civicrm_contact']}.id = {$this->_aliases['civicrm_contribution']}.contact_id {$this->_aclFrom}"; - + if ($this->isTableSelected('civicrm_line_item')) { + $this->_from .= " + LEFT JOIN civicrm_line_item {$this->_aliases['civicrm_line_item']} + ON {$this->_aliases['civicrm_contribution']}.id = {$this->_aliases['civicrm_line_item']}.contribution_id"; + } if ($this->isTableSelected('civicrm_email')) { $this->_from .= " LEFT JOIN civicrm_email {$this->_aliases['civicrm_email']} @@ -381,6 +428,7 @@ public function where() { CRM_Utils_Array::value("{$fieldName}_max", $this->_params) ); if (($fieldName == 'contribution_status_id' || + $fieldName == 'line_item_financial_type_id' || $fieldName == 'financial_type_id') && !empty($clause) ) { $this->_statusClause .= " AND " . $clause; @@ -404,10 +452,16 @@ public function where() { public function groupBy() { $this->assign('chartSupported', TRUE); - $fiscalYearOffset = self::fiscalYearOffset("{$this->_aliases['civicrm_contribution']}.receive_date"); - $this->_groupBy = "GROUP BY {$this->_aliases['civicrm_contribution']}.contact_id, {$fiscalYearOffset}"; - $this->appendSelect($this->_selectClauses, array("{$this->_aliases['civicrm_contribution']}.contact_id", $fiscalYearOffset)); - $this->_groupBy .= " {$this->_rollup}"; + $this->_groupBy = "Group BY {$this->_aliases['civicrm_contribution']}.contact_id, "; + if (isset($this->_params['group_bys']) && $this->_params['group_bys']['line_item_financial_type_id']) { + $this->_groupBy .= " {$this->_aliases['civicrm_line_item']}.financial_type_id, "; + } + if (isset($this->_params['group_bys']) && $this->_params['group_bys']['financial_type_id']) { + $this->_groupBy .= " {$this->_aliases['civicrm_contribution']}.financial_type_id, "; + } + $this->_groupBy .= self::fiscalYearOffset($this->_aliases['civicrm_contribution'] . + '.receive_date') . " " . " " . $this->_rollup; + } /** @@ -419,19 +473,36 @@ public function statistics(&$rows) { $statistics = parent::statistics($rows); if (!empty($rows)) { - $select = " + if (isset($this->_params['fields']['line_total']) && count($this->_params['fields']['line_total']) > 0) { + $select = " + SELECT + SUM({$this->_aliases['civicrm_line_item']}.line_total ) as lineamount "; + $sql = "{$select} {$this->_from} {$this->_where}"; + $dao = CRM_Core_DAO::executeQuery($sql); + if ($dao->fetch()) { + $statistics['counts']['lineamount'] = array( + 'value' => $dao->lineamount, + 'title' => 'Total Line Items LifeTime', + 'type' => CRM_Utils_Type::T_MONEY, + ); + } + } + if (isset($this->_params['fields']['total_amount']) && (count($this->_params['fields']['total_amount']) > 0) + || (isset($this->_params['fields']['line_total']) && count($this->_params['fields']['line_total']) == 0)) { + $select = " SELECT SUM({$this->_aliases['civicrm_contribution']}.total_amount ) as amount "; - - $sql = "{$select} {$this->_from} {$this->_where}"; - $dao = CRM_Core_DAO::executeQuery($sql); - if ($dao->fetch()) { - $statistics['counts']['amount'] = array( - 'value' => $dao->amount, - 'title' => 'Total LifeTime', - 'type' => CRM_Utils_Type::T_MONEY, - ); + $sql = "{$select} {$this->_from} {$this->_where}"; + $dao = CRM_Core_DAO::executeQuery($sql); + if ($dao->fetch()) { + $statistics['counts']['amount'] = array( + 'value' => $dao->amount, + 'title' => 'Total Contributions LifeTime', + 'type' => CRM_Utils_Type::T_MONEY, + ); + } } + } return $statistics; } @@ -474,34 +545,83 @@ public function postProcess() { $upTo_year = $current_year - 4; $rows = $row = array(); - $dao = CRM_Core_DAO::executeQuery($sql); + $dao = $this->executeReportQuery($sql); $contributionSum = 0; + $linetotalSum = 0; $yearcal = array(); + $count = 0; while ($dao->fetch()) { + $count++; if (!$dao->civicrm_contribution_contact_id) { continue; } + + // these conditions set row index values to keep groupings collated, and eliminate subtotals from the rollup results, yet preserving the last row which is the grand total + if (isset($this->_params['group_bys']) && !empty($this->_params['group_bys']['line_item_financial_type_id']) && $this->_params['group_bys']['financial_type_id']) { + if (is_null($dao->civicrm_contribution_financial_type_id) && $count != $dao->N - 1) { + continue; + } + $rowindex = "-l" . $dao->civicrm_line_item_line_item_financial_type_id . "-c" . $dao->civicrm_contribution_financial_type_id; + } + elseif (isset($this->_params['group_bys']) && $this->_params['group_bys']['line_item_financial_type_id']) { + if (is_null($dao->civicrm_line_item_line_item_financial_type_id) && $count != $dao->N - 1) { + continue; + } + $rowindex = "-l" . $dao->civicrm_line_item_line_item_financial_type_id; + } + elseif (isset($this->_params['group_bys']) && ($this->_params['group_bys']['financial_type_id'] || $this->_params['group_bys']['line_item_financial_type_id'])) { + if (is_null($dao->civicrm_contribution_financial_type_id) && $count != $dao->N - 1) { + continue; + } + $rowindex = "-c" . $dao->civicrm_contribution_financial_type_id; + } + else { + $rowindex = ""; + } $row = array(); foreach ($this->_columnHeaders as $key => $value) { if (property_exists($dao, $key)) { - $rows[$dao->civicrm_contribution_contact_id][$key] = $dao->$key; + $rows[$dao->civicrm_contribution_contact_id . $rowindex][$key] = $dao->$key; } } + + //if ($this->_params['group_bys']['line_item_financial_type_id']) { + if ($dao->civicrm_contribution_receive_date) { + if ($dao->civicrm_contribution_receive_date > $upTo_year && isset($dao->civicrm_line_item_line_total)) { + $linetotalSum += $dao->civicrm_line_item_line_total; + $rows[$dao->civicrm_contribution_contact_id . $rowindex]['line_items_year_' . $dao->civicrm_contribution_receive_date] = $dao->civicrm_line_item_line_total; + } + } + else { + if (isset($dao->civicrm_line_item_line_total)) { + $rows[$dao->civicrm_contribution_contact_id . $rowindex]['line_items_life_time_total'] = $dao->civicrm_line_item_line_total; + + if (($dao->civicrm_line_item_line_total - $linetotalSum) > 0 + ) { + $rows[$dao->civicrm_contribution_contact_id . $rowindex]["line_items_civicrm_upto_{$upTo_year}"] + = $dao->civicrm_line_item_line_total - $linetotalSum; + } + } + $linetotalSum = 0; + } + //} + //if ($this->_params['group_bys']['financial_type_id'] || $this->_params['group_bys']['line_item_financial_type_id']) { if ($dao->civicrm_contribution_receive_date) { if ($dao->civicrm_contribution_receive_date > $upTo_year) { $contributionSum += $dao->civicrm_contribution_total_amount; - $rows[$dao->civicrm_contribution_contact_id]['year_' . $dao->civicrm_contribution_receive_date] = $dao->civicrm_contribution_total_amount; + $rows[$dao->civicrm_contribution_contact_id . $rowindex]['contributions_year_' . $dao->civicrm_contribution_receive_date] = $dao->civicrm_contribution_total_amount; } } else { - $rows[$dao->civicrm_contribution_contact_id]['civicrm_life_time_total'] = $dao->civicrm_contribution_total_amount; + $rows[$dao->civicrm_contribution_contact_id . $rowindex]['contributions_life_time_total'] = $dao->civicrm_contribution_total_amount; if (($dao->civicrm_contribution_total_amount - $contributionSum) > 0 ) { - $rows[$dao->civicrm_contribution_contact_id]["civicrm_upto_{$upTo_year}"] + $rows[$dao->civicrm_contribution_contact_id . $rowindex]["contributions_civicrm_upto_{$upTo_year}"] = $dao->civicrm_contribution_total_amount - $contributionSum; } $contributionSum = 0; } + //} } $dao->free(); } @@ -572,6 +692,7 @@ public function buildChart(&$rows) { */ public function alterDisplay(&$rows) { $entryFound = FALSE; + $financialTypes = CRM_Contribute_PseudoConstant::financialType(); foreach ($rows as $rowNum => $row) { //Convert Display name into link @@ -615,6 +736,21 @@ public function alterDisplay(&$rows) { } $entryFound = TRUE; } + // change line item financial type ID to financial type name + if ((array_key_exists('civicrm_line_item_line_item_financial_type_id', $row))) { + if ($value = $row['civicrm_line_item_line_item_financial_type_id']) { + $rows[$rowNum]['civicrm_line_item_line_item_financial_type_id'] = $financialTypes[$value]; + } + $entryFound = TRUE; + } + + // change contribution financial type ID to financial type name + if ((array_key_exists('civicrm_contribution_financial_type_id', $row))) { + if ($value = $row['civicrm_contribution_financial_type_id']) { + $rows[$rowNum]['civicrm_contribution_financial_type_id'] = $financialTypes[$value]; + } + $entryFound = TRUE; + } // skip looking further in rows, if first row itself doesn't // have the column we need