From: eileen <emcnaughton@wikimedia.org>
Date: Sun, 13 Jan 2019 21:13:43 +0000 (+1300)
Subject: Remove inappropriate exception handling.
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=db77f7d32a57364c030ff6533a3767867e4ef54c;p=civicrm-core.git

Remove inappropriate exception handling.

Per discussion in https://github.com/civicrm/civicrm-core/commit/570243c292fa7bceea74ce233feceb3f69211e54#r31805899

the core logic has been changed to now accept a payment regardless of contribution status. This reflects the
fact that a payment could be received for a fully paid contribution and we still need to record it.

This exception is inappropriately narrow (& likely in the wrong place since it should probably be closer to
where the decision is made about adding a payment rather than when it is being processed
---

diff --git a/CRM/Financial/BAO/Payment.php b/CRM/Financial/BAO/Payment.php
index 826340335c..656f2a03e4 100644
--- a/CRM/Financial/BAO/Payment.php
+++ b/CRM/Financial/BAO/Payment.php
@@ -55,70 +55,65 @@ class CRM_Financial_BAO_Payment {
   public static function create($params) {
     $contribution = civicrm_api3('Contribution', 'getsingle', ['id' => $params['contribution_id']]);
     $contributionStatus = CRM_Contribute_PseudoConstant::contributionStatus($contribution['contribution_status_id'], 'name');
-    if ($contributionStatus != 'Partially paid'
-      && !($contributionStatus == 'Pending' && $contribution['is_pay_later'] == TRUE)
-    ) {
-      throw new API_Exception('Please select a contribution which has a partial or pending payment');
-    }
-    else {
-      // Check if pending contribution
-      $fullyPaidPayLater = FALSE;
-      if ($contributionStatus == 'Pending') {
-        $cmp = bccomp($contribution['total_amount'], $params['total_amount'], 5);
-        // Total payment amount is the whole amount paid against pending contribution
-        if ($cmp == 0 || $cmp == -1) {
-          civicrm_api3('Contribution', 'completetransaction', ['id' => $contribution['id']]);
-          // Get the trxn
-          $trxnId = CRM_Core_BAO_FinancialTrxn::getFinancialTrxnId($contribution['id'], 'DESC');
-          $ftParams = ['id' => $trxnId['financialTrxnId']];
-          $trxn = CRM_Core_BAO_FinancialTrxn::retrieve($ftParams, CRM_Core_DAO::$_nullArray);
-          $fullyPaidPayLater = TRUE;
-        }
-        else {
-          civicrm_api3('Contribution', 'create',
-            [
-              'id' => $contribution['id'],
-              'contribution_status_id' => 'Partially paid',
-            ]
-          );
-        }
+
+    // Check if pending contribution
+    $fullyPaidPayLater = FALSE;
+    if ($contributionStatus == 'Pending') {
+      $cmp = bccomp($contribution['total_amount'], $params['total_amount'], 5);
+      // Total payment amount is the whole amount paid against pending contribution
+      if ($cmp == 0 || $cmp == -1) {
+        civicrm_api3('Contribution', 'completetransaction', ['id' => $contribution['id']]);
+        // Get the trxn
+        $trxnId = CRM_Core_BAO_FinancialTrxn::getFinancialTrxnId($contribution['id'], 'DESC');
+        $ftParams = ['id' => $trxnId['financialTrxnId']];
+        $trxn = CRM_Core_BAO_FinancialTrxn::retrieve($ftParams, CRM_Core_DAO::$_nullArray);
+        $fullyPaidPayLater = TRUE;
+      }
+      else {
+        civicrm_api3('Contribution', 'create',
+          [
+            'id' => $contribution['id'],
+            'contribution_status_id' => 'Partially paid',
+          ]
+        );
       }
-      if (!$fullyPaidPayLater) {
-        $trxn = CRM_Core_BAO_FinancialTrxn::getPartialPaymentTrxn($contribution, $params);
-        if (CRM_Utils_Array::value('line_item', $params) && !empty($trxn)) {
-          foreach ($params['line_item'] as $values) {
-            foreach ($values as $id => $amount) {
-              $p = ['id' => $id];
-              $check = CRM_Price_BAO_LineItem::retrieve($p, $defaults);
-              if (empty($check)) {
-                throw new API_Exception('Please specify a valid Line Item.');
-              }
-              // get financial item
-              $sql = "SELECT fi.id
-              FROM civicrm_financial_item fi
-              INNER JOIN civicrm_line_item li ON li.id = fi.entity_id and fi.entity_table = 'civicrm_line_item'
-              WHERE li.contribution_id = %1 AND li.id = %2";
-              $sqlParams = [
-                1 => [$params['contribution_id'], 'Integer'],
-                2 => [$id, 'Integer'],
-              ];
-              $fid = CRM_Core_DAO::singleValueQuery($sql, $sqlParams);
-              // Record Entity Financial Trxn
-              $eftParams = [
-                'entity_table' => 'civicrm_financial_item',
-                'financial_trxn_id' => $trxn->id,
-                'amount' => $amount,
-                'entity_id' => $fid,
-              ];
-              civicrm_api3('EntityFinancialTrxn', 'create', $eftParams);
+    }
+    if (!$fullyPaidPayLater) {
+      $trxn = CRM_Core_BAO_FinancialTrxn::getPartialPaymentTrxn($contribution, $params);
+      if (CRM_Utils_Array::value('line_item', $params) && !empty($trxn)) {
+        foreach ($params['line_item'] as $values) {
+          foreach ($values as $id => $amount) {
+            $p = ['id' => $id];
+            $check = CRM_Price_BAO_LineItem::retrieve($p, $defaults);
+            if (empty($check)) {
+              throw new API_Exception('Please specify a valid Line Item.');
             }
+            // get financial item
+            $sql = "SELECT fi.id
+            FROM civicrm_financial_item fi
+            INNER JOIN civicrm_line_item li ON li.id = fi.entity_id and fi.entity_table = 'civicrm_line_item'
+            WHERE li.contribution_id = %1 AND li.id = %2";
+            $sqlParams = [
+              1 => [$params['contribution_id'], 'Integer'],
+              2 => [$id, 'Integer'],
+            ];
+            $fid = CRM_Core_DAO::singleValueQuery($sql, $sqlParams);
+            // Record Entity Financial Trxn
+            $eftParams = [
+              'entity_table' => 'civicrm_financial_item',
+              'financial_trxn_id' => $trxn->id,
+              'amount' => $amount,
+              'entity_id' => $fid,
+            ];
+            civicrm_api3('EntityFinancialTrxn', 'create', $eftParams);
           }
         }
-        elseif (!empty($trxn)) {
-          CRM_Contribute_BAO_Contribution::assignProportionalLineItems($params, $trxn->id, $contribution['total_amount']);
-        }
+      }
+      elseif (!empty($trxn)) {
+        CRM_Contribute_BAO_Contribution::assignProportionalLineItems($params, $trxn->id, $contribution['total_amount']);
       }
     }
+
     return $trxn;
   }