From d96598581654e954b696ca8467efc8a2c7618111 Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <eileen@fuzion.co.nz>
Date: Mon, 15 Sep 2014 11:56:02 +1200
Subject: [PATCH] CRM-15296

---
 CRM/Contribute/BAO/Contribution/Utils.php | 24 +++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/CRM/Contribute/BAO/Contribution/Utils.php b/CRM/Contribute/BAO/Contribution/Utils.php
index 458108c9c9..fdb830594a 100644
--- a/CRM/Contribute/BAO/Contribution/Utils.php
+++ b/CRM/Contribute/BAO/Contribution/Utils.php
@@ -65,9 +65,11 @@ class CRM_Contribute_BAO_Contribution_Utils {
     $isTest = ($form->_mode == 'test') ? 1 : 0;
     $lineItems = $form->_lineItem;
 
-    //do overrides that should have been done in the calling function - ie the calling function should pass the correct
-    //variable for $contributionTypeId but we need to ensure it is doing so.
-    // before we remove this cruft
+    //do overrides that should have been done in the calling function -
+    // @todo the calling function should pass the correct
+    // variable for $contributionTypeId as it's impossible to debug what is happening in each flow here
+    // but we need to ensure it is doing so before we remove this cruft
+
     if (isset($paymentParams['financial_type'])) {
       $contributionTypeId = $paymentParams['financial_type'];
     }
@@ -80,16 +82,17 @@ class CRM_Contribute_BAO_Contribution_Utils {
 
     $contributionType = new CRM_Financial_DAO_FinancialType();
     $contributionType->id = $contributionTypeId;
-
     if (!$contributionType->find(TRUE)) {
+      //@todo - surely this check was part of the 4.3 upgrade & can go now?
       CRM_Core_Error::fatal('Could not find a system table');
     }
 
     // add some financial type details to the params list
     // if folks need to use it
-    $paymentParams['contributionType_name'] = $form->_params['contributionType_name'] = $contributionType->name;
+    //CRM-15297 - contributionType is obsolete - pass financial type as well so people can deprecate it
+    $paymentParams['financialType_name'] = $paymentParams['contributionType_name'] = $form->_params['contributionType_name'] = $contributionType->name;
     //CRM-11456
-    $paymentParams['contributionType_accounting_code'] = $form->_params['contributionType_accounting_code'] = CRM_Financial_BAO_FinancialAccount::getAccountingCode($contributionTypeId);
+    $paymentParams['financialType_accounting_code'] =  $paymentParams['contributionType_accounting_code'] = $form->_params['contributionType_accounting_code'] = CRM_Financial_BAO_FinancialAccount::getAccountingCode($contributionTypeId);
     $paymentParams['contributionPageID'] = $form->_params['contributionPageID'] = $form->_values['id'];
 
 
@@ -148,7 +151,7 @@ class CRM_Contribute_BAO_Contribution_Utils {
         else {
           if (!$form->_params['is_pay_later']) {
             if (is_object($payment)) {
-              // call postprocess hook before leaving
+              // call postProcess hook before leaving
               $form->postProcessHook();
               // this does not return
               $result = &$payment->doTransferCheckout($form->_params, 'contribute');
@@ -249,7 +252,8 @@ class CRM_Contribute_BAO_Contribution_Utils {
         }
 
         $paymentParams['contributionID'] = $contribution->id;
-        $paymentParams['contributionTypeID'] = $contribution->financial_type_id;
+        //CRM-15297 deprecate contributionTypeID
+        $paymentParams['financialTypeID'] = $paymentParams['contributionTypeID'] = $contribution->financial_type_id;
         $paymentParams['contributionPageID'] = $contribution->contribution_page_id;
 
         if ($form->_values['is_recur'] && $contribution->contribution_recur_id) {
@@ -270,6 +274,8 @@ class CRM_Contribute_BAO_Contribution_Utils {
 
     if (is_a($result, 'CRM_Core_Error')) {
       //make sure to cleanup db for recurring case.
+      //@todo this clean up has always been controversial as many orgs prefer to see failed transactions.
+      // most recent discussion has been that they should be retained and this could be altered
       if (!empty($paymentParams['contributionID'])) {
         CRM_Contribute_BAO_Contribution::deleteContribution($paymentParams['contributionID']);
       }
@@ -298,6 +304,8 @@ class CRM_Contribute_BAO_Contribution_Utils {
 
       // result has all the stuff we need
       // lets archive it to a financial transaction
+      //@todo - this is done in 2 places - can't we just do it once straight after retrieving contribution type -
+      // when would this be a bad thing?
       if ($contributionType->is_deductible) {
         $form->assign('is_deductible', TRUE);
         $form->set('is_deductible', TRUE);
-- 
2.25.1