Merge pull request #13512 from eileenmcnaughton/cont_annual_speed

Speed up loading of contribution tab on contacts with large number of contributions in a large database

CRM/Contribute/BAO/Contribution.php

@@ -5568,20 +5568,13 @@ public static function getAnnualQuery($contactIDs) {
     }
     $startDate = "$year$monthDay";
     $endDate = "$nextYear$monthDay";
-    $financialTypes = [];
-    CRM_Financial_BAO_FinancialType::getAvailableFinancialTypes($financialTypes);
-    // this is a clumsy way of saying never return anything
-    // @todo improve!
-    $liWhere = " AND i.financial_type_id IN (0)";
-    if (!empty($financialTypes)) {
-      $liWhere = " AND i.financial_type_id NOT IN (" . implode(',', array_keys($financialTypes)) . ")";
-    }
+
     $whereClauses = [
       'contact_id' => 'IN (' . $contactIDs . ')',
-      'contribution_status_id' => '= ' . (int) CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed'),
       'is_test' => ' = 0',
       'receive_date' => ['>=' . $startDate, '<  ' . $endDate],
     ];
+    $havingClause = 'contribution_status_id = ' . (int) CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed');
     CRM_Financial_BAO_FinancialType::addACLClausesToWhereClauses($whereClauses);
 
     $clauses = [];
@@ -5590,15 +5583,17 @@ public static function getAnnualQuery($contactIDs) {
     }
     $whereClauseString = implode(' AND ', $clauses);
 
+    // See https://github.com/civicrm/civicrm-core/pull/13512 for discussion of how
+    // this group by + having on contribution_status_id improves performance
     $query = "
       SELECT COUNT(*) as count,
              SUM(total_amount) as amount,
              AVG(total_amount) as average,
              currency
       FROM civicrm_contribution b
-      LEFT JOIN civicrm_line_item i ON i.contribution_id = b.id AND i.entity_table = 'civicrm_contribution' $liWhere
       WHERE " . $whereClauseString . "
-      GROUP BY currency
+      GROUP BY currency, contribution_status_id
+      HAVING $havingClause
       ";
     return $query;
   }

tests/phpunit/CRM/Contribute/BAO/ContributionTest.php

@@ -32,6 +32,7 @@
 class CRM_Contribute_BAO_ContributionTest extends CiviUnitTestCase {
 
   use CRMTraits_Financial_FinancialACLTrait;
+  use CRMTraits_Financial_PriceSetTrait;
 
   /**
    * Clean up after tests.
@@ -324,6 +325,23 @@ public function testAnnualQueryWithFinancialACLsEnabled() {
     $this->disableFinancialACLs();
   }
 
+  /**
+   * Test the annual query returns a correct result when multiple line items are present.
+   */
+  public function testAnnualWithMultipleLineItems() {
+    $contactID = $this->createLoggedInUserWithFinancialACL();
+    $this->createContributionWithTwoLineItemsAgainstPriceSet([
+      'contact_id' => $contactID]
+    );
+    $this->enableFinancialACLs();
+    $sql = CRM_Contribute_BAO_Contribution::getAnnualQuery([$contactID]);
+    $result = CRM_Core_DAO::executeQuery($sql);
+    $result->fetch();
+    $this->assertEquals(300, $result->amount);
+    $this->assertEquals(1, $result->count);
+    $this->disableFinancialACLs();
+  }
+
   /**
    * Test that financial type data is not added to the annual query if acls not enabled.
    */

tests/phpunit/CRMTraits/Financial/PriceSetTrait.php

@@ -0,0 +1,61 @@
+<?php
+/*
+ +--------------------------------------------------------------------+
+ | CiviCRM version 5                                                  |
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC (c) 2004-2019                                |
+ +--------------------------------------------------------------------+
+ | This file is a part of CiviCRM.                                    |
+ |                                                                    |
+ | CiviCRM is free software; you can copy, modify, and distribute it  |
+ | under the terms of the GNU Affero General Public License           |
+ | Version 3, 19 November 2007 and the CiviCRM Licensing Exception.   |
+ |                                                                    |
+ | CiviCRM is distributed in the hope that it will be useful, but     |
+ | WITHOUT ANY WARRANTY; without even the implied warranty of         |
+ | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.               |
+ | See the GNU Affero General Public License for more details.        |
+ |                                                                    |
+ | You should have received a copy of the GNU Affero General Public   |
+ | License and the CiviCRM Licensing Exception along                  |
+ | with this program; if not, contact CiviCRM LLC                     |
+ | at info[AT]civicrm[DOT]org. If you have questions about the        |
+ | GNU Affero General Public License or the licensing of CiviCRM,     |
+ | see the CiviCRM license FAQ at http://civicrm.org/licensing        |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ * Trait PriceSetTrait
+ *
+ * Trait for working with Price Sets in tests
+ */
+trait CRMTraits_Financial_PriceSetTrait {
+
+  /**
+   * Create a contribution with 2 line items.
+   *
+   * This also involves creating t
+   *
+   * @param $params
+   */
+  protected function createContributionWithTwoLineItemsAgainstPriceSet($params) {
+    $params = array_merge(['total_amount' => 300, 'financial_type_id' => 'Donation'], $params);
+    $priceFields = $this->createPriceSet('contribution');
+    foreach ($priceFields['values'] as $key => $priceField) {
+      $params['line_items'][]['line_item'][$key] = [
+        'price_field_id' => $priceField['price_field_id'],
+        'price_field_value_id' => $priceField['id'],
+        'label' => $priceField['label'],
+        'field_title' => $priceField['label'],
+        'qty' => 1,
+        'unit_price' => $priceField['amount'],
+        'line_total' => $priceField['amount'],
+        'financial_type_id' => $priceField['financial_type_id'],
+        'entity_table' => 'civicrm_contribution',
+      ];
+    }
+    $this->callAPISuccess('order', 'create', $params);
+  }
+
+}