Further deconstruction of updateFinancialAccounts

updateFinancialAccounts is only called from one place with the context of 'changedStatus'. When called this
way the function updateFinancialAccountsOnContributionStatusChange is called & that 'tells'
the calling function whether to call the rest of this function.

Since this handling is specific to one place from which the function is called we can move that logic
back to the place that calls it & further simplify the updateFinancialAccounts function
which has been a bit of a haven of chaos

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index b4176178edfe34b4ddcf7699391cceb99a323d0d..8aafcee8eea5de98c5d5148e556ba3060c7381a4 100644 (file)
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -3691,7 +3691,10 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
           $params['prevContribution']->contribution_status_id != $params['contribution']->contribution_status_id
         ) {
           //Update Financial Records
-          self::updateFinancialAccounts($params, 'changedStatus');
+          $callUpdateFinancialAccounts = self::updateFinancialAccountsOnContributionStatusChange($params);
+          if ($callUpdateFinancialAccounts) {
+            self::updateFinancialAccounts($params, 'changedStatus');
+          }
           $updated = TRUE;
         }
 
@@ -3808,14 +3811,6 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
     $inputParams = $params;
     $isARefund = self::isContributionUpdateARefund($params['prevContribution']->contribution_status_id, $params['contribution']->contribution_status_id);
 
-    if ($context == 'changedStatus') {
-      $continue = self::updateFinancialAccountsOnContributionStatusChange($params);
-      // @todo - it may be that this is always false & the parent function is just a confusing wrapper for the child fn.
-      if (!$continue) {
-        return;
-      }
-    }
-
     if ($context == 'changedAmount' || $context == 'changeFinancialType') {
       // @todo we should stop passing $params by reference - splitting this out would be a step towards that.
       $params['trxnParams']['total_amount'] = $params['trxnParams']['net_amount'] = ($params['total_amount'] - $params['prevContribution']->total_amount);