From 53c8b1bebcbb6b0d974f57ba17615d3adf364ab5 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Mon, 24 May 2021 14:29:04 +1200 Subject: [PATCH] Add call to validateAllContributions & fix getTotalAmount I've updated 2 tests for readability and extended one to call validateAllContributions. The call failed and it turned out to be the check was wrong. Fixing that caused a test to fail. I have fixed the test by fixing getTotalAmount on the internal helper BAO_Order class to include the tax_amount for each line in the total. I think this would most affect tests - but it might affect the Membership_Form due to changes made recently to that form so I'm on the fence about going for the rc with this one --- CRM/Financial/BAO/Order.php | 2 +- tests/phpunit/CiviTest/CiviUnitTestCase.php | 2 +- tests/phpunit/api/v3/ContributionPageTest.php | 20 ++++++--- tests/phpunit/api/v3/ContributionTest.php | 41 +++++++++++++++---- 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/CRM/Financial/BAO/Order.php b/CRM/Financial/BAO/Order.php index d4916e7e4e..4ef6de2f5b 100644 --- a/CRM/Financial/BAO/Order.php +++ b/CRM/Financial/BAO/Order.php @@ -447,7 +447,7 @@ class CRM_Financial_BAO_Order { public function getTotalAmount() :float { $amount = 0.0; foreach ($this->getLineItems() as $lineItem) { - $amount += $lineItem['line_total'] ?? 0.0; + $amount += ($lineItem['line_total'] ?? 0.0) + ($lineItem['tax_amount'] ?? 0.0); } return $amount; } diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index e97f52d011..e2ca637ae0 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -3643,7 +3643,7 @@ VALUES $taxTotal += (float) ($lineItem['tax_amount'] ?? 0); } $this->assertEquals($taxTotal, (float) ($contribution['tax_amount'] ?? 0)); - $this->assertEquals($total, $contribution['total_amount']); + $this->assertEquals($total + $taxTotal, $contribution['total_amount']); } } diff --git a/tests/phpunit/api/v3/ContributionPageTest.php b/tests/phpunit/api/v3/ContributionPageTest.php index f64673fe11..3217464543 100644 --- a/tests/phpunit/api/v3/ContributionPageTest.php +++ b/tests/phpunit/api/v3/ContributionPageTest.php @@ -826,7 +826,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase { $contribution_ids = []; $found_membership_amount = $found_contribution_amount = FALSE; foreach ($result['values'] as $value) { - list($junk, $json) = explode("$test_uniq:", $value['message']); + [$junk, $json] = explode("$test_uniq:", $value['message']); $logged_contribution = json_decode($json, TRUE); $contribution_ids[] = $logged_contribution['contributionID']; if (!empty($logged_contribution['total_amount'])) { @@ -1947,13 +1947,23 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase { /** * Test Tax Amount is calculated properly when using PriceSet with Field Type = Text/Numeric Quantity * + * The created contribution has 3 line items + * + * |qty | unit_price| line_total| tax |total including tax| + * | 1 | 10 | 10 | 0 | 10 | + * | 180 | 16.95 | 3051 |305.1 | 3356.1| + * | 110 | 2.95 | 324.5 | 32.45 | 356.95| + * + * Contribution total = 3723.05 + * made up of tax 337.55 + * non tax 3385.5 * @param string $thousandSeparator * punctuation used to refer to thousands. * * @dataProvider getThousandSeparators * @throws \CRM_Core_Exception */ - public function testSubmitContributionPageWithPriceSetQuantity($thousandSeparator) { + public function testSubmitContributionPageWithPriceSetQuantity(string $thousandSeparator): void { $this->setCurrencySeparators($thousandSeparator); $this->_priceSetParams['is_quick_config'] = 0; $this->enableTaxAndInvoicing(); @@ -1984,7 +1994,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase { 'html_type' => 'Text', ]); - $this->callAPISuccess('price_field_value', 'create', [ + $this->callAPISuccess('PriceFieldValue', 'create', [ 'price_set_id' => $priceSetID, 'price_field_id' => $priceField['id'], 'label' => 'Printing Rights', @@ -1996,7 +2006,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase { // Set quantity for our test $submitParams['price_' . $priceFieldId] = 180; - $priceField = $this->callAPISuccess('price_field', 'create', [ + $priceField = $this->callAPISuccess('PriceField', 'create', [ 'price_set_id' => $priceSetID, 'label' => 'Another Line Item', 'html_type' => 'Text', @@ -2020,7 +2030,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase { $this->callAPISuccess('ContributionPage', 'submit', $submitParams); $this->validateAllContributions(); - $contribution = $this->callAPISuccessGetSingle('contribution', [ + $contribution = $this->callAPISuccessGetSingle('Contribution', [ 'contribution_page_id' => $this->_ids['contribution_page'], ]); diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index 4f7e183fb2..04f94744c9 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -2248,6 +2248,23 @@ class api_v3_ContributionTest extends CiviUnitTestCase { * CRM-19126 Add test to verify when complete transaction is called tax * amount is not changed. * + * We start of with a pending contribution. + * - total_amount (input) = 100 + * - total_amount post save (based on tax being added) = 105 + * - net_amount = 95 + * - fee_amount = 5 + * - non_deductible_amount = 10 + * - tax rate = 5% + * - tax_amount = 5 + * - sum of (calculated) line items = 105 + * + * Note the fee_amount should really be set when the payment is received + * and whatever the non_deductible amount is, it is ignored. + * + * The fee amount when the payment comes in is 6 rather than 5. The net_amount + * and fee_amount should change, but not the total_amount or + * the line items. + * * @param string $thousandSeparator * punctuation used to refer to thousands. * @@ -2259,18 +2276,24 @@ class api_v3_ContributionTest extends CiviUnitTestCase { $this->createFinancialTypeWithSalesTax(); $financialTypeId = $this->ids['FinancialType']['taxable']; - $params = array_merge($this->_params, ['contribution_status_id' => 2, 'financial_type_id' => $financialTypeId]); - $contribution = $this->callAPISuccess('contribution', 'create', $params); - $contribution1 = $this->callAPISuccess('contribution', 'get', ['id' => $contribution['id'], 'return' => 'tax_amount', 'sequential' => 1]); - $this->callAPISuccess('contribution', 'completetransaction', [ - 'id' => $contribution['id'], + $contributionID = $this->callAPISuccess('Order', 'create', + array_merge($this->_params, ['financial_type_id' => $financialTypeId]) + )['id']; + $contributionPrePayment = $this->callAPISuccessGetSingle('Contribution', ['id' => $contributionID, 'return' => ['tax_amount', 'total_amount']]); + $this->validateAllContributions(); + $this->callAPISuccess('Contribution', 'completetransaction', [ + 'id' => $contributionID, 'trxn_id' => '777788888', 'fee_amount' => '6.00', + 'sequential' => 1, ]); - $contribution2 = $this->callAPISuccess('contribution', 'get', ['id' => $contribution['id'], 'return' => ['tax_amount', 'fee_amount', 'net_amount'], 'sequential' => 1]); - $this->assertEquals($contribution1['values'][0]['tax_amount'], $contribution2['values'][0]['tax_amount']); - $this->assertEquals('6.00', $contribution2['values'][0]['fee_amount']); - $this->assertEquals('99.00', $contribution2['values'][0]['net_amount']); + $contributionPostPayment = $this->callAPISuccessGetSingle('Contribution', ['id' => $contributionID, 'return' => ['tax_amount', 'fee_amount', 'net_amount']]); + $this->assertEquals(5, $contributionPrePayment['tax_amount']); + $this->assertEquals(5, $contributionPostPayment['tax_amount']); + $this->assertEquals('6.00', $contributionPostPayment['fee_amount']); + $this->assertEquals('99.00', $contributionPostPayment['net_amount']); + $this->validateAllContributions(); + $this->validateAllPayments(); } /** -- 2.25.1