From 4f5911bbb76a9cafd45f953f5b40668f21c97cf9 Mon Sep 17 00:00:00 2001 From: KarinG Date: Sun, 22 Jan 2017 22:03:38 -0700 Subject: [PATCH 1/2] Fundamental Fixes for TaxMath Calculations. --- CRM/Contribute/BAO/Contribution/Utils.php | 3 ++- CRM/Price/BAO/LineItem.php | 32 +++++++---------------- CRM/Price/BAO/PriceSet.php | 5 ++-- templates/CRM/Price/Page/LineItem.tpl | 2 +- 4 files changed, 15 insertions(+), 27 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution/Utils.php b/CRM/Contribute/BAO/Contribution/Utils.php index 6ea7c5b271fd..5b650a10a09c 100644 --- a/CRM/Contribute/BAO/Contribution/Utils.php +++ b/CRM/Contribute/BAO/Contribution/Utils.php @@ -475,7 +475,8 @@ public static function getFirstLastDetails($contactID) { */ public static function calculateTaxAmount($amount, $taxRate) { $taxAmount = array(); - $taxAmount['tax_amount'] = round(($taxRate / 100) * CRM_Utils_Rule::cleanMoney($amount), 2); + // There can not be any rounding at this stage - as this is prior to quantity multiplication + $taxAmount['tax_amount'] = ($taxRate / 100) * CRM_Utils_Rule::cleanMoney($amount); return $taxAmount; } diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php index 4767d5e7ff80..56694bd71cf6 100644 --- a/CRM/Price/BAO/LineItem.php +++ b/CRM/Price/BAO/LineItem.php @@ -298,7 +298,15 @@ public static function getLineItems($entityId, $entity = 'participant', $isQuick 'tax_amount' => $dao->tax_amount, 'price_set_id' => $dao->price_set_id, ); - $lineItems[$dao->id]['tax_rate'] = CRM_Price_BAO_LineItem::calculateTaxRate($lineItems[$dao->id]); + $taxRates = CRM_Core_PseudoConstant::getTaxRates(); + if (isset($lineItems[$dao->id]['financial_type_id']) && array_key_exists($lineItems[$dao->id]['financial_type_id'], $taxRates)) { + // We are close to output/display here - so apply some rounding at output/display level - to not show Tax Rate in all 8 decimals + $lineItems[$dao->id]['tax_rate'] = round($taxRates[$lineItems[$dao->id]['financial_type_id']], 3); + } + else { + // There is no Tax Rate associated with this Financial Type + $lineItems[$dao->id]['tax_rate'] = FALSE; + } $lineItems[$dao->id]['subTotal'] = $lineItems[$dao->id]['qty'] * $lineItems[$dao->id]['unit_price']; if ($lineItems[$dao->id]['tax_amount'] != '') { $getTaxDetails = TRUE; @@ -597,26 +605,4 @@ public static function getLineItemArray(&$params, $entityId = NULL, $entityTable } } - /** - * Calculate tax rate in percentage. - * - * @param array $lineItemId - * An assoc array of lineItem. - * - * @return int|void - * tax rate - */ - public static function calculateTaxRate($lineItemId) { - if ($lineItemId['unit_price'] == 0 || $lineItemId['qty'] == 0) { - return FALSE; - } - if ($lineItemId['html_type'] == 'Text') { - $tax = round($lineItemId['tax_amount'] / ($lineItemId['unit_price'] * $lineItemId['qty']) * 100, 2); - } - else { - $tax = round(($lineItemId['tax_amount'] / $lineItemId['unit_price']) * 100, 2); - } - return $tax; - } - } diff --git a/CRM/Price/BAO/PriceSet.php b/CRM/Price/BAO/PriceSet.php index 39fa2d567cf8..6f2ca783524b 100644 --- a/CRM/Price/BAO/PriceSet.php +++ b/CRM/Price/BAO/PriceSet.php @@ -1666,11 +1666,12 @@ public static function copyPriceSet($baoName, $id, $newId) { * @return array */ public static function setLineItem($field, $lineItem, $optionValueId, &$totalTax) { + // Here we round - i.e. after multiplying by quantity if ($field['html_type'] == 'Text') { - $taxAmount = $field['options'][$optionValueId]['tax_amount'] * $lineItem[$optionValueId]['qty']; + $taxAmount = round($field['options'][$optionValueId]['tax_amount'] * $lineItem[$optionValueId]['qty'], 2); } else { - $taxAmount = $field['options'][$optionValueId]['tax_amount']; + $taxAmount = round($field['options'][$optionValueId]['tax_amount'], 2); } $taxRate = $field['options'][$optionValueId]['tax_rate']; $lineItem[$optionValueId]['tax_amount'] = $taxAmount; diff --git a/templates/CRM/Price/Page/LineItem.tpl b/templates/CRM/Price/Page/LineItem.tpl index 9c0c0f365cbf..d0ab3de21c18 100644 --- a/templates/CRM/Price/Page/LineItem.tpl +++ b/templates/CRM/Price/Page/LineItem.tpl @@ -78,7 +78,7 @@ {if $getTaxDetails} {$line.line_total|crmMoney} {if $line.tax_rate != "" || $line.tax_amount != ""} - {$taxTerm} ({$line.tax_rate|string_format:"%.2f"}%) + {$taxTerm} ({$line.tax_rate|string_format:"%.3f"}%) {$line.tax_amount|crmMoney} {else} From 57d395bb4020aa48fce3164a48f2414cdfcbf057 Mon Sep 17 00:00:00 2001 From: KarinG Date: Wed, 25 Jan 2017 09:29:03 -0700 Subject: [PATCH 2/2] Add PHPUnit Test - testSubmitContributionPageWithPriceSetQuantity. --- tests/phpunit/api/v3/ContributionPageTest.php | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/phpunit/api/v3/ContributionPageTest.php b/tests/phpunit/api/v3/ContributionPageTest.php index d652063920d3..fe5ccd57ec6d 100644 --- a/tests/phpunit/api/v3/ContributionPageTest.php +++ b/tests/phpunit/api/v3/ContributionPageTest.php @@ -1416,4 +1416,66 @@ public function addPriceFields(&$params) { ); } + /** + * Test Tax Amount is calculated properly when using PriceSet with Field Type = Text/Numeric Quantity + */ + public function testSubmitContributionPageWithPriceSetQuantity() { + $this->_priceSetParams['is_quick_config'] = 0; + $this->enableTaxAndInvoicing(); + $financialType = $this->createFinancialType(); + $financialTypeId = $financialType['id']; + // This function sets the Tax Rate at 10% - it currently has no way to pass Tax Rate into it - so let's work with 10% + $financialAccount = $this->relationForFinancialTypeWithFinancialAccount($financialType['id'], 5); + + $this->setUpContributionPage(); + $submitParams = array( + 'price_' . $this->_ids['price_field'][0] => reset($this->_ids['price_field_value']), + 'id' => (int) $this->_ids['contribution_page'], + 'first_name' => 'J', + 'last_name' => 'T', + 'email' => 'JT@ohcanada.ca', + 'is_pay_later' => TRUE, + ); + + // Create PriceSet/PriceField + $priceSetID = reset($this->_ids['price_set']); + $priceField = $this->callAPISuccess('price_field', 'create', array( + 'price_set_id' => $priceSetID, + 'label' => 'Printing Rights', + 'html_type' => 'Text', + )); + $priceFieldValue = $this->callAPISuccess('price_field_value', 'create', array( + 'price_set_id' => $priceSetID, + 'price_field_id' => $priceField['id'], + 'label' => 'Printing Rights', + 'financial_type_id' => $financialTypeId, + 'amount' => '16.95', + )); + $priceFieldId = $priceField['id']; + + // Set quantity for our test + $submitParams['price_' . $priceFieldId] = 180; + + // contribution_page submit requires amount and tax_amount - and that's ok we're not testing that - we're testing at the LineItem level + $submitParams['amount'] = 180 * 16.95; + // This is the correct Tax Amount - use it later to compare to what the CiviCRM Core came up with at the LineItem level + $submitParams['tax_amount'] = 180 * 16.95 * 0.10; + + $this->callAPISuccess('contribution_page', 'submit', $submitParams); + $contribution = $this->callAPISuccessGetSingle('contribution', array( + 'contribution_page_id' => $this->_ids['contribution_page'], + )); + + // Retrieve the lineItem that belongs to the Printing Rights and check the tax_amount CiviCRM Core calculated for it + $lineItem = $this->callAPISuccess('LineItem', 'get', array( + 'contribution_id' => $contribution['id'], + 'label' => 'Printing Rights', + )); + $lineItemId = $lineItem['id']; + $lineItem_TaxAmount = round($lineItem['values'][$lineItemId]['tax_amount'], 2); + + // Compare this to what it should be! + $this->assertEquals($lineItem_TaxAmount, round($submitParams['tax_amount'], 2), 'Wrong Sales Tax Amount is calculated and stored.'); + } + }