make contribution filters work

CRM/Report/Form/Contribute/History.php

@@ -390,7 +390,7 @@ class CRM_Report_Form_Contribute_History extends CRM_Report_Form {
   public function where() {
     $whereClauses = $havingClauses = $relationshipWhere = [];
     $this->_relationshipWhere = '';
-    $this->_statusClause = '';
+    $this->_contributionClauses = [];
 
     foreach ($this->_columns as $tableName => $table) {
       if (array_key_exists('filters', $table)) {
@@ -425,8 +425,11 @@ class CRM_Report_Form_Contribute_History extends CRM_Report_Form {
               continue;
             }
 
-            if ($fieldName == 'contribution_status_id') {
-              $this->_statusClause = " AND " . $clause;
+            // Make contribution filters work.
+            // Note total_sum is already accounted for in the main buildRows
+            // and this_year and other_year skip the loop above.
+            if ($tableName == 'civicrm_contribution' && $fieldName != 'total_sum') {
+              $this->_contributionClauses[$fieldName] = $clause;
             }
 
             if (!empty($field['having'])) {
@@ -681,9 +684,14 @@ class CRM_Report_Form_Contribute_History extends CRM_Report_Form {
       return $rows;
     }
 
+    $contributionClauses = '';
+    if (!empty($this->_contributionClauses)) {
+      $contributionClauses = ' AND ' . implode(' AND ', $this->_contributionClauses);
+    }
+
     $sqlContribution = "{$this->_select} {$this->_from} WHERE {$this->_aliases['civicrm_contact']}.id IN (" .
       implode(',', $contactIds) .
-      ") AND {$this->_aliases['civicrm_contribution']}.is_test = 0 AND {$this->_aliases['civicrm_contribution']}.is_template = 0 {$this->_statusClause} {$this->_groupBy} ";
+      ") AND {$this->_aliases['civicrm_contribution']}.is_test = 0 AND {$this->_aliases['civicrm_contribution']}.is_template = 0 {$contributionClauses} {$this->_groupBy} ";
 
     $dao = CRM_Core_DAO::executeQuery($sqlContribution);
     $contributionSum = 0;
@@ -814,8 +822,14 @@ class CRM_Report_Form_Contribute_History extends CRM_Report_Form {
           }
 
           if ($last_primary && ($rowNum == "{$last_primary}_total")) {
-            $value = CRM_Utils_Money::formatLocaleNumericRoundedForDefaultCurrency($value);
+            // Passing non-numeric is deprecated, but this isn't a perfect fix
+            // since it will still format things like postal code 90210 as
+            // "90,210.00", but that predates the deprecation. See dev/core#2819
+            if (is_numeric($value)) {
+              $value = CRM_Utils_Money::formatLocaleNumericRoundedForDefaultCurrency($value);
+            }
           }
+          // TODO: It later tries to format this as money which then gives a warning. One option is to instead set something like $row[$key]['classes'] and then use that in the template, but I don't think the stock template supports something like that.
           $row[$key] = '<strong>' . $value . '</strong>';
         }
         $rows[$rowNum] = $row;

tests/phpunit/api/v3/ReportTemplateTest.php

@@ -1622,6 +1622,33 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
     ]);
   }
 
+  /**
+   * Test that the contribution aggregate by relationship report filters
+   * by financial type.
+   */
+  public function testContributionAggregateByRelationship() {
+    $contact = $this->individualCreate();
+    // Two contributions with different financial types.
+    // We don't really care which types, just different.
+    $this->contributionCreate(['contact_id' => $contact, 'receive_date' => (date('Y') - 1) . '-07-01', 'financial_type_id' => 1, 'total_amount' => '10']);
+    $this->contributionCreate(['contact_id' => $contact, 'receive_date' => (date('Y') - 1) . '-08-01', 'financial_type_id' => 2, 'total_amount' => '20']);
+    $rows = $this->callAPISuccess('report_template', 'getrows', [
+      'report_id' => 'contribute/history',
+      'financial_type_id_op' => 'in',
+      'financial_type_id_value' => [1],
+      'options' => ['metadata' => ['sql']],
+      'fields' => [
+        'relationship_type_id' => 1,
+        'total_amount' => 1,
+      ],
+    ]);
+
+    // Hmm it has styling in it before being sent to the template. If that gets fixed then will need to update this.
+    $this->assertEquals('<strong>10.00</strong>', $rows['values'][$contact]['civicrm_contribution_total_amount'], 'should only include the $10 contribution');
+
+    $this->callAPISuccess('Contact', 'delete', ['id' => $contact]);
+  }
+
   /**
    * Convoluted test of the convoluted logging detail report.
    *