From 88a20030a42b4bdc5592894bb37624fd876f3739 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 21 Jun 2019 17:50:40 -0400 Subject: [PATCH] Decommission recordPartialPayment function While I would normally say extracting is good sometimes if feels like things are being broken down in the wrong places. In this case I think the function name is misleading and having this in the body of Payment.create makes the path to cleaning up that function clearer --- CRM/Contribute/BAO/Contribution.php | 4 ++-- CRM/Financial/BAO/Payment.php | 23 ++++++++++++++++++- .../CRM/Core/BAO/FinancialTrxnTest.php | 8 +++---- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 66695a72c4..12adfb8f00 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -942,7 +942,7 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution { * * @return int */ - protected static function getToFinancialAccount($contribution, $params) { + public static function getToFinancialAccount($contribution, $params) { $contributionStatuses = CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name'); CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name'); $pendingStatus = [ @@ -4839,7 +4839,7 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac * @return CRM_Financial_DAO_FinancialTrxn */ public static function recordPartialPayment($contribution, $params) { - + CRM_Core_Error::deprecatedFunctionWarning('use payment create api'); $balanceTrxnParams['to_financial_account_id'] = self::getToFinancialAccount($contribution, $params); $balanceTrxnParams['from_financial_account_id'] = CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship($contribution['financial_type_id'], 'Accounts Receivable Account is'); $balanceTrxnParams['total_amount'] = $params['total_amount']; diff --git a/CRM/Financial/BAO/Payment.php b/CRM/Financial/BAO/Payment.php index c1608c805e..7217637710 100644 --- a/CRM/Financial/BAO/Payment.php +++ b/CRM/Financial/BAO/Payment.php @@ -65,8 +65,29 @@ class CRM_Financial_BAO_Payment { $isSkipRecordingPaymentHereForLegacyHandlingReasons = ($contributionStatus == 'Pending' && $isPaymentCompletesContribution); if (!$isSkipRecordingPaymentHereForLegacyHandlingReasons && $params['total_amount'] > 0) { - $trxn = CRM_Contribute_BAO_Contribution::recordPartialPayment($contribution, $params); + $balanceTrxnParams['to_financial_account_id'] = CRM_Contribute_BAO_Contribution::getToFinancialAccount($contribution, $params); + $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'))); + $balanceTrxnParams['fee_amount'] = CRM_Utils_Array::value('fee_amount', $params); + $balanceTrxnParams['net_amount'] = CRM_Utils_Array::value('total_amount', $params); + $balanceTrxnParams['currency'] = $contribution['currency']; + $balanceTrxnParams['trxn_id'] = CRM_Utils_Array::value('contribution_trxn_id', $params, NULL); + $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); + $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 getToFinancialAccount + // also anticipates it. + CRM_Core_Error::deprecatedFunctionWarning('passing payment_processor is deprecated - use payment_processor_id'); + $balanceTrxnParams['payment_processor_id'] = $params['payment_processor']; + } + $trxn = CRM_Core_BAO_FinancialTrxn::create($balanceTrxnParams); + // @todo - this is just weird & historical & inconsistent - why 2 tracks? if (CRM_Utils_Array::value('line_item', $params) && !empty($trxn)) { foreach ($params['line_item'] as $values) { foreach ($values as $id => $amount) { diff --git a/tests/phpunit/CRM/Core/BAO/FinancialTrxnTest.php b/tests/phpunit/CRM/Core/BAO/FinancialTrxnTest.php index 388dcea22b..6173bc3476 100644 --- a/tests/phpunit/CRM/Core/BAO/FinancialTrxnTest.php +++ b/tests/phpunit/CRM/Core/BAO/FinancialTrxnTest.php @@ -113,11 +113,11 @@ class CRM_Core_BAO_FinancialTrxnTest extends CiviUnitTestCase { $contributionTest = new CRM_Contribute_BAO_ContributionTest(); list($lineItems, $contribution) = $contributionTest->addParticipantWithContribution(); $contribution = (array) $contribution; - $params = array( + $params = [ 'contribution_id' => $contribution['id'], 'total_amount' => 100.00, - ); - $trxn = CRM_Contribute_BAO_Contribution::recordPartialPayment($contribution, $params); + ]; + $this->callAPISuccess('Payment', 'create', $params); $paid = CRM_Core_BAO_FinancialTrxn::getTotalPayments($params['contribution_id']); $total = CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $params['contribution_id'], 'total_amount'); $cmp = bccomp($total, $paid, 5); @@ -126,8 +126,6 @@ class CRM_Core_BAO_FinancialTrxnTest extends CiviUnitTestCase { civicrm_api3('Contribution', 'completetransaction', array('id' => $contribution['id'])); } - $this->assertEquals('100.00', $trxn->total_amount, 'Amount does not match.'); - $totalPaymentAmount = CRM_Core_BAO_FinancialTrxn::getTotalPayments($contribution['id']); $this->assertEquals('250.00', $totalPaymentAmount, 'Amount does not match.'); } -- 2.25.1