From eaba6ea1929700d1792f02363e423f32cb9615be Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 9 Nov 2019 00:42:40 +1300 Subject: [PATCH] Remove 'partially paid' as a contribution status option for 'record payment' Fixes a bug where it is possible to select contribution statuses that do not result in valid financial transactions. Specifically the 'Partially Paid' option creates no payment transaction and any subsequent financial_trxns get the wrong line item allocations as a result. --- CRM/Contribute/BAO/Contribution/Utils.php | 16 +++++---- .../CRM/Member/Form/MembershipTest.php | 34 +++++++++---------- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution/Utils.php b/CRM/Contribute/BAO/Contribution/Utils.php index 8c38e23d66..4eb6aa620f 100644 --- a/CRM/Contribute/BAO/Contribution/Utils.php +++ b/CRM/Contribute/BAO/Contribution/Utils.php @@ -538,7 +538,7 @@ LIMIT 1 * Array of contribution statuses in array('status id' => 'label') format */ public static function getContributionStatuses($usedFor = 'contribution', $id = NULL) { - if ($usedFor == 'pledge') { + if ($usedFor === 'pledge') { $statusNames = CRM_Pledge_BAO_Pledge::buildOptions('status_id', 'validate'); } else { @@ -562,25 +562,29 @@ LIMIT 1 'Pending refund', ]); - // Event registration and New Membership backoffice form support partially paid payment, - // so exclude this status only for 'New Contribution' form - if ($usedFor == 'contribution') { + if ($usedFor === 'contribution') { $statusNamesToUnset = array_merge($statusNamesToUnset, [ 'In Progress', 'Overdue', 'Partially paid', ]); } - elseif ($usedFor == 'participant') { + elseif ($usedFor === 'participant') { $statusNamesToUnset = array_merge($statusNamesToUnset, [ 'Cancelled', 'Failed', + 'In Progress', + 'Overdue', + 'Partially paid', ]); } - elseif ($usedFor == 'membership') { + elseif ($usedFor === 'membership') { $statusNamesToUnset = array_merge($statusNamesToUnset, [ 'In Progress', + 'Cancelled', + 'Failed', 'Overdue', + 'Partially paid', ]); } } diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php index 3108cfb0d8..2f647ef51e 100644 --- a/tests/phpunit/CRM/Member/Form/MembershipTest.php +++ b/tests/phpunit/CRM/Member/Form/MembershipTest.php @@ -657,7 +657,7 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { $this->createLoggedInUser(); $priceSet = $this->callAPISuccess('PriceSet', 'Get', ["extends" => "CiviMember"]); $form->set('priceSetId', $priceSet['id']); - $partiallyPaidAmount = 25; + CRM_Price_BAO_PriceSet::buildPriceSet($form); $params = [ 'cid' => $this->_individualId, @@ -668,9 +668,9 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { 'membership_type_id' => [23, $this->membershipTypeAnnualFixedID], 'receive_date' => date('Y-m-d', time()) . ' 20:36:00', 'record_contribution' => 1, - 'total_amount' => $this->formatMoneyInput($partiallyPaidAmount), - 'payment_instrument_id' => array_search('Check', $this->paymentInstruments), - 'contribution_status_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Partially paid'), + 'total_amount' => $this->formatMoneyInput(50), + 'payment_instrument_id' => array_search('Check', $this->paymentInstruments, TRUE), + 'contribution_status_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Pending'), //Member dues, see data.xml 'financial_type_id' => '2', 'payment_processor_id' => $this->_paymentProcessorID, @@ -679,13 +679,11 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { $form->testSubmit($params); $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]); // check the membership status after partial payment, if its Pending - $this->assertEquals(array_search('Pending', CRM_Member_PseudoConstant::membershipStatus()), $membership['status_id']); - $contribution = $this->callAPISuccessGetSingle('Contribution', [ - 'contact_id' => $this->_individualId, - ]); + $this->assertEquals(array_search('Pending', CRM_Member_PseudoConstant::membershipStatus(), TRUE), $membership['status_id']); + $contribution = $this->callAPISuccessGetSingle('Contribution', ['contact_id' => $this->_individualId]); + $this->callAPISuccess('Payment', 'create', ['contribution_id' => $contribution['id'], 'total_amount' => 25, 'payment_instrument_id' => 'Cash']); + $contribution = $this->callAPISuccessGetSingle('Contribution', ['id' => $contribution['id']]); $this->assertEquals('Partially paid', $contribution['contribution_status']); - // $this->assertEquals(50.00, $contribution['total_amount']); - // $this->assertEquals(25.00, $contribution['net_amount']); // Step 2: submit the other half of the partial payment // via AdditionalPayment form to complete the related contribution @@ -693,27 +691,27 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { $submitParams = [ 'contribution_id' => $contribution['contribution_id'], 'contact_id' => $this->_individualId, - 'total_amount' => $this->formatMoneyInput($partiallyPaidAmount), + 'total_amount' => $this->formatMoneyInput(25), 'currency' => 'USD', 'financial_type_id' => 2, 'receive_date' => '2015-04-21 23:27:00', 'trxn_date' => '2017-04-11 13:05:11', 'payment_processor_id' => 0, - 'payment_instrument_id' => array_search('Check', $this->paymentInstruments), + 'payment_instrument_id' => array_search('Check', $this->paymentInstruments, TRUE), 'check_number' => 'check-12345', ]; $form->cid = $this->_individualId; $form->testSubmit($submitParams); $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]); // check the membership status after additional payment, if its changed to 'New' - $this->assertEquals(array_search('New', CRM_Member_PseudoConstant::membershipStatus()), $membership['status_id']); + $this->assertEquals(array_search('New', CRM_Member_PseudoConstant::membershipStatus(), TRUE), $membership['status_id']); // check the contribution status and net amount after additional payment $contribution = $this->callAPISuccessGetSingle('Contribution', [ 'contact_id' => $this->_individualId, ]); $this->assertEquals('Completed', $contribution['contribution_status']); - // $this->assertEquals(50.00, $contribution['net_amount']); + $this->validateAllPayments(); } /** @@ -724,14 +722,14 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase { public function testSubmitRecur() { CRM_Core_Session::singleton()->getStatus(TRUE); $pendingVal = $this->callAPISuccessGetValue('OptionValue', [ - 'return' => "id", - 'option_group_id' => "contribution_status", - 'label' => "Pending Label**", + 'return' => 'id', + 'option_group_id' => 'contribution_status', + 'label' => 'Pending Label**', ]); //Update label for Pending contribution status. $this->callAPISuccess('OptionValue', 'create', [ 'id' => $pendingVal, - 'label' => "PendingEdited", + 'label' => 'PendingEdited', ]); $form = $this->getForm(); -- 2.25.1