From 6c434a1dfe2b9dcf2b34dd59600806e34bfca6e5 Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 27 Apr 2019 02:38:48 +1200 Subject: [PATCH] Remove call to CRM_Contribute_BAO_Contribution::recordAdditionalPayment in favour of payment create --- CRM/Contribute/BAO/Contribution.php | 8 +++++-- CRM/Financial/BAO/Payment.php | 24 +++++++++++++++---- .../CRM/Event/BAO/AdditionalPaymentTest.php | 17 +++++++------ .../CRM/Event/BAO/ChangeFeeSelectionTest.php | 10 ++++---- 4 files changed, 40 insertions(+), 19 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index d5f7a0b76e..8f1fd8db46 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -3591,8 +3591,12 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac ) { return; } - if ((($previousContributionStatus == 'Partially paid' - && $currentContributionStatus == 'Completed') + // The 'right' way to add payments or refunds is through the Payment.create api. That api + // then updates the contribution but this process shoud not also record another financial trxn. + if ((($previousContributionStatus == 'Partially paid' && $currentContributionStatus == 'Completed') + || ($previousContributionStatus == 'Pending refund' && $currentContributionStatus == 'Completed') + // This concept of pay_later as different to any other sort of pending is deprecated & it's unclear + // why it is here or where it is handled instead. || ($previousContributionStatus == 'Pending' && $params['prevContribution']->is_pay_later == TRUE && $currentContributionStatus == 'Partially paid')) && $context == 'changedStatus' diff --git a/CRM/Financial/BAO/Payment.php b/CRM/Financial/BAO/Payment.php index e743d2d796..664c193e0c 100644 --- a/CRM/Financial/BAO/Payment.php +++ b/CRM/Financial/BAO/Payment.php @@ -51,6 +51,7 @@ class CRM_Financial_BAO_Payment { * * @throws \API_Exception * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception */ public static function create($params) { $contribution = civicrm_api3('Contribution', 'getsingle', ['id' => $params['contribution_id']]); @@ -104,11 +105,24 @@ class CRM_Financial_BAO_Payment { } if ($isPaymentCompletesContribution) { - 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); + if ($contributionStatus == 'Pending refund') { + // Ideally we could still call completetransaction as non-payment related actions should + // be outside this class. However, for now we just update the contribution here. + // Unit test cover in CRM_Event_BAO_AdditionalPaymentTest::testTransactionInfo. + civicrm_api3('Contribution', 'create', + [ + 'id' => $contribution['id'], + 'contribution_status_id' => 'Completed', + ] + ); + } + else { + 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); + } } elseif ($contributionStatus === 'Pending') { civicrm_api3('Contribution', 'create', diff --git a/tests/phpunit/CRM/Event/BAO/AdditionalPaymentTest.php b/tests/phpunit/CRM/Event/BAO/AdditionalPaymentTest.php index aa4cd9e431..9e70d946bf 100644 --- a/tests/phpunit/CRM/Event/BAO/AdditionalPaymentTest.php +++ b/tests/phpunit/CRM/Event/BAO/AdditionalPaymentTest.php @@ -214,20 +214,23 @@ class CRM_Event_BAO_AdditionalPaymentTest extends CiviUnitTestCase { $result = $this->addParticipantWithPayment($feeAmt, $amtPaid); $contributionID = $result['contribution']['id']; - //Complete the partial payment. - $submittedValues = array( + $this->callAPISuccess('Payment', 'create', [ + 'contribution_id' => $contributionID, 'total_amount' => 20, 'payment_instrument_id' => 3, - ); - CRM_Contribute_BAO_Contribution::recordAdditionalPayment($contributionID, $submittedValues, 'owed', $result['participant']['id']); + 'participant_id' => $result['participant']['id'], + ]); //Change selection to a lower amount. $params['price_2'] = 50; CRM_Price_BAO_LineItem::changeFeeSelections($params, $result['participant']['id'], 'participant', $contributionID, $result['feeBlock'], $result['lineItem']); - //Record a refund of the remaining amount. - $submittedValues['total_amount'] = 50; - CRM_Contribute_BAO_Contribution::recordAdditionalPayment($contributionID, $submittedValues, 'refund', $result['participant']['id']); + $this->callAPISuccess('Payment', 'create', [ + 'total_amount' => -50, + 'contribution_id' => $contributionID, + 'participant_id' => $result['participant']['id'], + 'payment_instrument_id' => 3, + ]); $paymentInfo = CRM_Contribute_BAO_Contribution::getPaymentInfo($result['participant']['id'], 'event', TRUE); $transaction = $paymentInfo['transaction']; diff --git a/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php b/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php index a2fc34fbbc..047fcb652d 100644 --- a/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php +++ b/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php @@ -308,12 +308,12 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase { $contributionBalance = ($this->_cheapFee - $actualPaidAmount); $this->assertEquals($contributionBalance, CRM_Contribute_BAO_Contribution::getContributionBalance($this->_contributionId)); - //Complete the refund payment. - $submittedValues = array( - 'total_amount' => 120, + $this->callAPISuccess('Payment', 'create', [ + 'contribution_id' => $this->_contributionId, + 'total_amount' => -120, 'payment_instrument_id' => 3, - ); - CRM_Contribute_BAO_Contribution::recordAdditionalPayment($this->_contributionId, $submittedValues, 'refund', $this->_participantId); + 'participant_id' => $this->_participantId, + ]); $contributionBalance += 120; $this->assertEquals($contributionBalance, CRM_Contribute_BAO_Contribution::getContributionBalance($this->_contributionId)); -- 2.25.1