diff --git a/CRM/Report/Form/Contribute/History.php b/CRM/Report/Form/Contribute/History.php index b2b168828da9f76914a317cba45f255087f24bf6..ba2dd4212cbb0451f89823656cb6063ba0846d10 100644 --- a/CRM/Report/Form/Contribute/History.php +++ b/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; diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php index 6e173c7d621d4c6f0d62e86c16aa41d8da7c231c..0b6d23f0562dce392816178e2291b1abd54a5665 100644 --- a/tests/phpunit/api/v3/ReportTemplateTest.php +++ b/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. *