From 3ed03d9656c6ee97597ca51c03693fe36ecf85d0 Mon Sep 17 00:00:00 2001
From: KarinG <karin@semper-it.com>
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}
       <td class="right">{$line.line_total|crmMoney}</td>
       {if $line.tax_rate != "" || $line.tax_amount != ""}
-        <td class="right">{$taxTerm} ({$line.tax_rate|string_format:"%.2f"}%)</td>
+        <td class="right">{$taxTerm} ({$line.tax_rate|string_format:"%.3f"}%)</td>
         <td class="right">{$line.tax_amount|crmMoney}</td>
       {else}
         <td></td>

From fa46cc8eda564eeda4afe10bde3da082b60af6b7 Mon Sep 17 00:00:00 2001
From: KarinG <karin@semper-it.com>
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.');
+  }
+
 }