[REF] small cleanups on payment.create flow.

In the interests of keeping some momentum I added this small cleanup which does 3 things

1) uses the preferred function to get 'from_financial_account_id', 2 moves a line of code to get participantID
& 3 removes the conditionals around is_payment. This probably needs the most double checking but
I believe recordPartialPaymet used to be called from places other than Payment.create so
it may not have always been handling a payment - however it I can't see a case for it not be
is_payment now

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index b89d304cee7155749cbf2d2d365a46611f968476..b9fd75d7324e9d4a5c8656e033384b7b2f848164 100644 (file)
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -4716,8 +4716,7 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
   public static function recordPartialPayment($contribution, $params) {
 
     $balanceTrxnParams['to_financial_account_id'] = self::getToFinancialAccount($contribution, $params);
-    $fromFinancialAccountId = CRM_Contribute_PseudoConstant::getRelationalFinancialAccount($contribution['financial_type_id'], 'Accounts Receivable Account is');
-    $balanceTrxnParams['from_financial_account_id'] = $fromFinancialAccountId;
+    $balanceTrxnParams['from_financial_account_id'] = CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship($contribution['financial_type_id'], 'Accounts Receivable Account is');
     $balanceTrxnParams['total_amount'] = $params['total_amount'];
     $balanceTrxnParams['contribution_id'] = $params['contribution_id'];
     $balanceTrxnParams['trxn_date'] = CRM_Utils_Array::value('trxn_date', $params, CRM_Utils_Array::value('contribution_receive_date', $params, date('YmdHis')));
@@ -4728,18 +4727,12 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
     $balanceTrxnParams['status_id'] = CRM_Core_PseudoConstant::getKey('CRM_Core_BAO_FinancialTrxn', 'status_id', 'Completed');
     $balanceTrxnParams['payment_instrument_id'] = CRM_Utils_Array::value('payment_instrument_id', $params, $contribution['payment_instrument_id']);
     $balanceTrxnParams['check_number'] = CRM_Utils_Array::value('check_number', $params);
-
-    // @todo the logic of this section seems very wrong. This code is ONLY reached from the Payment.create
-    // routine so is_payment should ALWAYS be true
-    $contributionStatuses = CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name');
-    $statusId = array_search('Completed', $contributionStatuses);
-    if ($fromFinancialAccountId != NULL &&
-      ($statusId == array_search('Completed', $contributionStatuses) || $statusId == array_search('Partially paid', $contributionStatuses))
-    ) {
-      $balanceTrxnParams['is_payment'] = 1;
-    }
+    $balanceTrxnParams['is_payment'] = 1;
 
     if (!empty($params['payment_processor'])) {
+      // I can't find evidence this is passed in - I was gonna just remove it but decided to deprecate  as I see self::getToFinancialAccount
+      // also anticipates it.
+      CRM_Core_Error::deprecatedFunctionWarning('passing payment_processor is deprecated - use payment_processor_id');
       $balanceTrxnParams['payment_processor_id'] = $params['payment_processor'];
     }
     return CRM_Core_BAO_FinancialTrxn::create($balanceTrxnParams);

diff --git a/CRM/Financial/BAO/Payment.php b/CRM/Financial/BAO/Payment.php
index e7d3aff4df66d6d066c7f6ffce80e7e6a607d970..84e01390b827b005382355e212eb09283432a18e 100644 (file)
--- a/CRM/Financial/BAO/Payment.php
+++ b/CRM/Financial/BAO/Payment.php
@@ -358,10 +358,6 @@ class CRM_Financial_BAO_Payment {
   public static function recordPayment($contributionId, $trxnData, $participantId) {
     list($contributionDAO, $params) = self::getContributionAndParamsInFormatForRecordFinancialTransaction($contributionId);
 
-    if (!$participantId) {
-      $participantId = CRM_Core_DAO::getFieldValue('CRM_Event_DAO_ParticipantPayment', $contributionId, 'participant_id', 'contribution_id');
-    }
-
     $trxnData['trxn_date'] = !empty($trxnData['trxn_date']) ? $trxnData['trxn_date'] : date('YmdHis');
     $params['payment_instrument_id'] = CRM_Utils_Array::value('payment_instrument_id', $trxnData, CRM_Utils_Array::value('payment_instrument_id', $params));
 
@@ -428,6 +424,9 @@ WHERE eft.entity_table = 'civicrm_contribution'
       // which in 'Partial Paid' => 'Completed' is not useful, instead specific financial record updates
       // are coded below i.e. just updating financial_item status to 'Paid'
 
+      if (!$participantId) {
+        $participantId = CRM_Core_DAO::getFieldValue('CRM_Event_DAO_ParticipantPayment', $contributionId, 'participant_id', 'contribution_id');
+      }
       if ($participantId) {
         // update participant status
         $participantStatuses = CRM_Event_PseudoConstant::participantStatus();