CRM-19126 load previous values selectively for tax

This includes moving the setting of tax values from the api to the BAO.

Unfortunately an awful lot of the tax work was done in the form layer & it would
be a big job to figure out which parts can be left to the BAO

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 6360c6a2f788d904d16151e35386fbe719fbd5b1..1eada55ae143633e2f3beee8c8dd20e37ca742b5 100644 (file)
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -183,12 +183,17 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
       }
     }
     if ($contributionID && $setPrevContribution) {
-      $params['prevContribution'] = self::getValues(array('id' => $contributionID), CRM_Core_DAO::$_nullArray, CRM_Core_DAO::$_nullArray);
+      $params['prevContribution'] = self::getOriginalContribution($contributionID);
     }
 
     // CRM-16189
     CRM_Financial_BAO_FinancialAccount::checkFinancialTypeHasDeferred($params, $contributionID);
 
+    if (!isset($params['tax_amount']) && $setPrevContribution && (isset($params['total_amount']) || isset
+      ($params['financial_type_id']))) {
+      CRM_Contribute_BAO_Contribution::checkTaxAmount($params);
+    }
+
     if ($contributionID) {
       CRM_Utils_Hook::pre('edit', 'Contribution', $contributionID, $params);
     }
@@ -4079,12 +4084,12 @@ WHERE eft.financial_trxn_id IN ({$trxnId}, {$baseTrxnId['financialTrxnId']})
   }
 
   /**
-   * Get financial account id has 'Sales Tax Account is'
-   * account relationship with financial type
+   * Get financial account id has 'Sales Tax Account is' account relationship with financial type.
    *
    * @param int $financialTypeId
    *
-   * @return FinancialAccountId
+   * @return int
+   *   Financial Account Id
    */
   public static function getFinancialAccountId($financialTypeId) {
     $accountRel = key(CRM_Core_PseudoConstant::accountOptionValues('account_relationship', NULL, " AND v.name LIKE 'Sales Tax Account is' "));
@@ -4100,61 +4105,50 @@ WHERE eft.financial_trxn_id IN ({$trxnId}, {$baseTrxnId['financialTrxnId']})
   }
 
   /**
-   * Check tax amount.
+   * Get the tax amount (misnamed function).
    *
    * @param array $params
    * @param bool $isLineItem
    *
-   * @return mixed
+   * @return array
    */
   public static function checkTaxAmount($params, $isLineItem = FALSE) {
     $taxRates = CRM_Core_PseudoConstant::getTaxRates();
 
     // Update contribution.
     if (!empty($params['id'])) {
-
-      $id = $params['id'];
-      $values = $ids = array();
-      $contrbutionParams = array('id' => $id);
-      $prevContributionValue = CRM_Contribute_BAO_Contribution::getValues($contrbutionParams, $values, $ids);
-
-      // CRM-19126 and CRM-19152, civicrm_line_item.tax_amount incorrectly set when using online payment processor.
-      // It looks like this method is meant to be called in multiple contexts: new & update
-      // When creating, would expect total_amount to be set?  Prior to 4.7, when creating online membership
-      // via contribution page, call scenario was different.
-      // This would not never get called, end result, taxes were correct.
-      // Since 4.7 it does.  Not sure this fix is ideal, but it does do the trick.
-      // Conceptually, if we're "updating", and total_amount is unknown.  We're dealing with an incomplete
-      // view, why are we nulling out all line item taxes, or messing with any other tax amounts?
-      if (!isset($params['total_amount'])) {
-        if (!empty($prevContributionValue->tax_amount)) {
-          $params['total_amount'] = $prevContributionValue->total_amount - $prevContributionValue->tax_amount;
-          if (isset($params['fee_amount'])) {
-            $params['net_amount'] = $params['total_amount'] - $params['fee_amount'];
-          }
-          else {
-            $params['net_amount'] = $params['total_amount'] - $prevContributionValue->fee_amount;
-            $params['fee_amount'] = $prevContributionValue->fee_amount;
-          }
-        }
-        else {
-          if (isset($params['fee_amount'])) {
-            $params['net_amount'] = $prevContributionValue->total_amount - $params['fee_amount'];
-            $params['total_amount'] = $prevContributionValue->total_amount;
+      // CRM-19126 and CRM-19152 If neither total or financial_type_id are set on an update
+      // there are no tax implications - early return.
+      if (!isset($params['total_amount']) && !isset($params['financial_type_id'])) {
+        return $params;
+      }
+      if (empty($params['prevContribution'])) {
+        $params['prevContribution'] = self::getOriginalContribution($params['id']);
+      }
+      $isRequireTaxCalculation = FALSE;
+      foreach (array('total_amount', 'financial_type_id', 'fee_amount', 'tax_amount') as $field) {
+        if (!isset($params[$field])) {
+          if ($field == 'total_amount' && $params['prevContribution']->tax_amount) {
+            // Tax amount gets added back on later....
+            $params['total_amount'] = $params['prevContribution']->total_amount -
+              $params['prevContribution']->tax_amount;
+            $isRequireTaxCalculation = TRUE;
           }
           else {
-            $params['total_amount'] = $prevContributionValue->total_amount;
-            $params['fee_amount'] = $prevContributionValue->fee_amount;
+            $params[$field] = $params['prevContribution']->$field;
+            if ($params[$field] != $params['prevContribution']->$field) {
+              $isRequireTaxCalculation = TRUE;
+            }
           }
         }
       }
-      // To assign pervious finantial type on update of contribution
-      if (!isset($params['financial_type_id'])) {
-        $params['financial_type_id'] = $prevContributionValue->financial_type_id;
+      if (!$isRequireTaxCalculation) {
+        return $params;
       }
-      elseif (isset($params['financial_type_id']) && !array_key_exists($params['financial_type_id'], $taxRates)) {
-        // Assisn tax Amount on update of contrbution
-        if (!empty($prevContributionValue->tax_amount)) {
+      self::calculateMissingAmountParams($params, $params['id']);
+      if (!array_key_exists($params['financial_type_id'], $taxRates)) {
+        // Assign tax Amount on update of contribution
+        if (!empty($params['prevContribution']->tax_amount)) {
           $params['tax_amount'] = 'null';
           CRM_Price_BAO_LineItem::getLineItemArray($params, array($params['id']));
           foreach ($params['line_item'] as $setID => $priceField) {
@@ -4166,7 +4160,7 @@ WHERE eft.financial_trxn_id IN ({$trxnId}, {$baseTrxnId['financialTrxnId']})
       }
     }
 
-    // New Contrbution and update of contribution with tax rate financial type
+    // New Contribution and update of contribution with tax rate financial type
     if (isset($params['financial_type_id']) && array_key_exists($params['financial_type_id'], $taxRates) &&
       empty($params['skipLineItem']) && !$isLineItem
     ) {
@@ -5219,6 +5213,17 @@ LEFT JOIN  civicrm_contribution on (civicrm_contribution.contact_id = civicrm_co
     return $statusMsg;
   }
 
+  /**
+   * Get the contribution as it is in the database before being updated.
+   *
+   * @param int $contributionID
+   *
+   * @return array
+   */
+  private static function getOriginalContribution($contributionID) {
+    return self::getValues(array('id' => $contributionID), CRM_Core_DAO::$_nullArray, CRM_Core_DAO::$_nullArray);
+  }
+
   /**
    * Assign Test Value.
    *

diff --git a/CRM/Core/PseudoConstant.php b/CRM/Core/PseudoConstant.php
index 5d887bb5abeb40ac612c7a3da37c2c232a18c0d6..26513dffc0f70648b771a37e57d3c325253e38b3 100644 (file)
--- a/CRM/Core/PseudoConstant.php
+++ b/CRM/Core/PseudoConstant.php
@@ -1828,6 +1828,8 @@ WHERE  id = %1
   public static function getTaxRates() {
     if (!isset(Civi::$statics[__CLASS__]['taxRates'])) {
       Civi::$statics[__CLASS__]['taxRates'] = array();
+      // Never do a copy & paste of this as the join on the option value is not indexed.
+      // @todo fix to resolve option values first.
       $sql = "
         SELECT fa.tax_rate, efa.entity_id
         FROM civicrm_entity_financial_account efa

diff --git a/api/v3/Contribution.php b/api/v3/Contribution.php
index 5d6bae177ee21eaeccc9d3395ccdd2b2e2f1bcec..be9656c681cb7c215f526a3a933697a2e63b16ca 100644 (file)
--- a/api/v3/Contribution.php
+++ b/api/v3/Contribution.php
@@ -80,10 +80,6 @@ function civicrm_api3_contribution_create(&$params) {
   }
   _civicrm_api3_contribution_create_legacy_support_45($params);
 
-  // Make sure tax calculation is handled via api.
-  // @todo this belongs in the BAO NOT the api.
-  $params = CRM_Contribute_BAO_Contribution::checkTaxAmount($params);
-
   return _civicrm_api3_basic_create(_civicrm_api3_get_BAO(__FUNCTION__), $params, 'Contribution');
 }