Fix bug in Annual summary query (on contact dashboard) whereby a left…

… join gives the wrong results

    As demonstrated in the test the left join does not do any filtering but it DOES join in as many line items as match the
    permitted types - resulting in too many rows & the sum calculated on them being incorrect.

    As this doesn't work it should be removed - I would aniticpate that sites with the financialreportacls extensions
    would get this adequately added through CRM_Financial_BAO_FinancialType::addACLClausesToWhereClauses
    but regardless this should go as it causes a bug without achieving it's intended result and adding a test prevents
    a later fix from re-breaking

CRM/Contribute/BAO/Contribution.php

@@ -5581,14 +5581,7 @@ 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'),
@@ -5609,7 +5602,6 @@ public static function getAnnualQuery($contactIDs) {
              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
       ";

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);
+  }
+
+}