Skip to content
Snippets Groups Projects
Unverified Commit 06eaefe4 authored by Eileen McNaughton's avatar Eileen McNaughton Committed by GitHub
Browse files

Merge pull request #15664 from eileenmcnaughton/odd_payment2 

financial#94 fix eroneous payment creation when editing line items
parents 045ada4d fab405c8
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
CRM/Price/BAO/LineItem.php
+ 1
− 41
View file @ 06eaefe4
......@@ -669,6 +669,7 @@ WHERE li.contribution_id = %1";
* @param $feeBlock
* @param array $lineItems
*
* @throws \CiviCRM_API3_Exception
*/
public static function changeFeeSelections(
$params,
......@@ -1046,7 +1047,6 @@ WHERE li.contribution_id = %1";
'entity_id' => $entityID,
'contribution_id' => $contributionID,
]);
$financialTypeChangeTrxnID = $this->addFinancialItemsOnLineItemChange($trxnID, $lineParams, $updatedContribution);
$lineObj = CRM_Price_BAO_LineItem::retrieve($lineParams);
// insert financial items
// ensure entity_financial_trxn table has a linking of it.
......@@ -1248,46 +1248,6 @@ WHERE li.contribution_id = %1";
return $adjustedTrxn;
}
/**
* Add financial items to reflect line item change.
*
* @param bool $isCreateAdditionalFinancialTrxn
* @param array $lineParams
* @param \CRM_Contribute_BAO_Contribution $updatedContribution
*/
protected function addFinancialItemsOnLineItemChange($isCreateAdditionalFinancialTrxn, $lineParams, $updatedContribution) {
$tempFinancialTrxnID = NULL;
// don't add financial item for cancelled line item
if ($lineParams['qty'] == 0) {
return NULL;
}
elseif ($isCreateAdditionalFinancialTrxn) {
// This routine & the return below is super uncomfortable.
// I have refactored to here and it is hit from
// testSubmitUnpaidPriceChangeWhileStillPending
// but I'm still skeptical it's not covered elsewhere.
// original comment : add financial item if ONLY financial type is changed
if ($lineParams['financial_type_id'] != $updatedContribution->financial_type_id) {
$changedFinancialTypeID = (int) $lineParams['financial_type_id'];
$adjustedTrxnValues = [
'from_financial_account_id' => NULL,
'to_financial_account_id' => CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($updatedContribution->payment_instrument_id),
'total_amount' => $lineParams['line_total'],
'net_amount' => $lineParams['line_total'],
'status_id' => $updatedContribution->contribution_status_id,
'payment_instrument_id' => $updatedContribution->payment_instrument_id,
'contribution_id' => $updatedContribution->id,
'is_payment' => TRUE,
// since balance is 0, which means contribution is completed
'trxn_date' => date('YmdHis'),
'currency' => $updatedContribution->currency,
];
$adjustedTrxn = CRM_Core_BAO_FinancialTrxn::create($adjustedTrxnValues);
return $adjustedTrxn->id;
}
}
}
/**
* Get Financial items, culling out any that have already been reversed.
*
......
tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php
+ 50
− 24
View file @ 06eaefe4
......@@ -59,6 +59,9 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
/**
* Set up for test.
*
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
*/
public function setUp() {
parent::setUp();
......@@ -86,11 +89,11 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
* @param string $type
*
* @return int
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
* @todo resolve this with parent function.
*/
protected function priceSetCreate($type = 'Radio') {
$feeTotal = 55;
$minAmt = 0;
$paramsSet['title'] = 'Two Options' . substr(sha1(rand()), 0, 4);
$paramsSet['name'] = CRM_Utils_String::titleToVar('Two Options') . substr(sha1(rand()), 0, 4);
$paramsSet['is_active'] = FALSE;
......@@ -98,7 +101,7 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
$priceSet = CRM_Price_BAO_PriceSet::create($paramsSet);
if ($type == 'Text') {
if ($type === 'Text') {
$paramsField = [
'label' => 'Text Price Field',
'name' => CRM_Utils_String::titleToVar('text_price_field'),
......@@ -118,14 +121,15 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
];
}
else {
/** @noinspection UnNecessaryDoubleQuotesInspection */
$paramsField = [
'label' => 'Price Field',
'name' => CRM_Utils_String::titleToVar('Two Options'),
'html_type' => 'Radio',
//'price' => $feeTotal,
'option_label' => ['1' => 'Expensive Room', '2' => "Cheap Room", '3' => 'Very Expensive', '4' => 'No Fee'],
'option_label' => ['1' => 'Expensive Room', '2' => 'Cheap Room', '3' => 'Very Expensive', '4' => 'No Fee'],
'option_value' => ['1' => 'E', '2' => 'C', '3' => 'V', '4' => 'N'],
'option_name' => ['1' => 'Expensive', '2' => "Cheap", "3" => "Very Expensive", '3' => 'No Fee'],
'option_name' => ['1' => 'Expensive', '2' => 'Cheap', '3' => 'Very Expensive'],
'option_weight' => ['1' => 1, '2' => 2, '3' => 3, '4' => 4],
'option_amount' => ['1' => $this->_expensiveFee, '2' => $this->_cheapFee, '3' => $this->_veryExpensive, '4' => $this->_noFee],
'option_count' => [1 => 1, 2 => 1, 3 => 1, 4 => 1],
......@@ -182,7 +186,7 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
WHERE contribution_id = {$contributionId}";
$dao = CRM_Core_DAO::executeQuery($query);
$this->assertTrue($dao->fetch(), "Succeeded retrieving invoicetotal");
$this->assertTrue($dao->fetch(), 'Succeeded retrieving invoice total');
return $dao->total;
}
......@@ -202,7 +206,7 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
";
$dao = CRM_Core_DAO::executeQuery($query);
$this->assertTrue($dao->fetch(), "Succeeded retrieving total Income");
$this->assertTrue($dao->fetch(), 'Succeeded retrieving total Income');
return $dao->total;
}
......@@ -218,6 +222,10 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
/**
* Prepare records for editing.
*
* @param null $actualPaidAmt
*
* @throws \CRM_Core_Exception
*/
public function registerParticipantAndPay($actualPaidAmt = NULL) {
$params = [
......@@ -225,7 +233,7 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
'is_test' => 0,
'is_pay_later' => 0,
'event_id' => $this->_eventId,
'register_date' => date('Y-m-d') . " 00:00:00",
'register_date' => date('Y-m-d') . ' 00:00:00',
'role_id' => 1,
'status_id' => 1,
'source' => 'Event_' . $this->_eventId,
......@@ -235,18 +243,18 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
$participant = $this->callAPISuccess('Participant', 'create', $params);
$this->_participantId = $participant['id'];
$actualPaidAmt = $actualPaidAmt ? $actualPaidAmt : $this->_expensiveFee;
$actualPaidAmt = $actualPaidAmt ?: $this->_expensiveFee;
$contributionParams = [
'total_amount' => $actualPaidAmt,
'source' => 'Testset with information',
'source' => 'Test set with information',
'currency' => 'USD',
'receipt_date' => date('Y-m-d') . " 00:00:00",
'receipt_date' => date('Y-m-d') . ' 00:00:00',
'contact_id' => $this->_contactId,
'financial_type_id' => 4,
'payment_instrument_id' => 4,
'contribution_status_id' => 1,
'receive_date' => date('Y-m-d') . " 00:00:00",
'receive_date' => date('Y-m-d') . ' 00:00:00',
'skipLineItem' => 1,
'partial_payment_total' => $this->_expensiveFee,
'partial_amount_to_pay' => $actualPaidAmt,
......@@ -271,43 +279,50 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
}
/**
* @throws \CRM_Core_Exception
*/
public function testCRM19273() {
$this->registerParticipantAndPay();
$priceSetParams['price_' . $this->priceSetFieldID] = $this->cheapFeeValueID;
$lineItem = CRM_Price_BAO_LineItem::getLineItems($this->_participantId, 'participant');
CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $this->_participantId, 'participant', $this->_contributionId, $this->_feeBlock, $lineItem, $this->_expensiveFee);
CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $this->_participantId, 'participant', $this->_contributionId, $this->_feeBlock, $lineItem);
$this->balanceCheck($this->_cheapFee);
$priceSetParams['price_' . $this->priceSetFieldID] = $this->expensiveFeeValueID;
$lineItem = CRM_Price_BAO_LineItem::getLineItems($this->_participantId, 'participant');
CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $this->_participantId, 'participant', $this->_contributionId, $this->_feeBlock, $lineItem, $this->_expensiveFee);
CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $this->_participantId, 'participant', $this->_contributionId, $this->_feeBlock, $lineItem);
$this->balanceCheck($this->_expensiveFee);
$priceSetParams['price_' . $this->priceSetFieldID] = $this->veryExpensiveFeeValueID;
$lineItem = CRM_Price_BAO_LineItem::getLineItems($this->_participantId, 'participant');
CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $this->_participantId, 'participant', $this->_contributionId, $this->_feeBlock, $lineItem, $this->_expensiveFee);
CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $this->_participantId, 'participant', $this->_contributionId, $this->_feeBlock, $lineItem);
$this->balanceCheck($this->_veryExpensive);
}
/**
* CRM-21245: Test that Contribution status doesn't changed to 'Pending Refund' from 'Partially Paid' if the partially paid amount is lower then newly selected fee amount
*
* @throws \CRM_Core_Exception
*/
public function testCRM21245() {
$this->registerParticipantAndPay(50);
$partiallyPaidContribuitonStatus = CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Partially paid');
$this->assertEquals($this->callAPISuccessGetValue('Contribution', ['id' => $this->_contributionId, 'return' => 'contribution_status_id']), $partiallyPaidContribuitonStatus);
$partiallyPaidContributionStatus = CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Partially paid');
$this->assertEquals($this->callAPISuccessGetValue('Contribution', ['id' => $this->_contributionId, 'return' => 'contribution_status_id']), $partiallyPaidContributionStatus);
$priceSetParams['price_' . $this->priceSetFieldID] = $this->veryExpensiveFeeValueID;
$lineItem = CRM_Price_BAO_LineItem::getLineItems($this->_participantId, 'participant');
CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $this->_participantId, 'participant', $this->_contributionId, $this->_feeBlock, $lineItem);
$this->assertEquals($this->callAPISuccessGetValue('Contribution', ['id' => $this->_contributionId, 'return' => 'contribution_status_id']), $partiallyPaidContribuitonStatus);
$this->assertEquals($this->callAPISuccessGetValue('Contribution', ['id' => $this->_contributionId, 'return' => 'contribution_status_id']), $partiallyPaidContributionStatus);
}
/**
* Test that proper financial items are recorded for cancelled line items
*
* @throws \CRM_Core_Exception
*/
public function testCRM20611() {
$this->registerParticipantAndPay();
......@@ -360,6 +375,9 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
/**
* Test to ensure that correct financial records are entered on text price field fee change on event registration
*
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
*/
public function testCRM21513() {
$this->_priceSetID = $this->priceSetCreate('Text');
......@@ -373,7 +391,7 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
'is_test' => 0,
'is_pay_later' => 0,
'event_id' => $this->_eventId,
'register_date' => date('Y-m-d') . " 00:00:00",
'register_date' => date('Y-m-d') . ' 00:00:00',
'role_id' => 1,
'status_id' => 1,
'source' => 'Event_' . $this->_eventId,
......@@ -383,14 +401,14 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
$this->_participantId = $participant['id'];
$contributionParams = [
'total_amount' => 10,
'source' => 'Testset with information',
'source' => 'Test set with information',
'currency' => 'USD',
'receipt_date' => date('Y-m-d') . " 00:00:00",
'receipt_date' => date('Y-m-d') . ' 00:00:00',
'contact_id' => $this->_contactId,
'financial_type_id' => 4,
'payment_instrument_id' => 4,
'contribution_status_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_DAO_Contribution', 'contribution_status_id', 'Pending'),
'receive_date' => date('Y-m-d') . " 00:00:00",
'receive_date' => date('Y-m-d') . ' 00:00:00',
'skipLineItem' => 1,
];
......@@ -412,12 +430,12 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
// CASE 2: Choose text price qty 3 (x$10 = $30 amount)
$priceSetParams['price_' . $this->priceSetFieldID] = 3;
$lineItem = CRM_Price_BAO_LineItem::getLineItems($participant['id'], 'participant');
CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $participant['id'], 'participant', $this->_contributionId, $this->_feeBlock, $lineItem, 0);
CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $participant['id'], 'participant', $this->_contributionId, $this->_feeBlock, $lineItem);
// CASE 3: Choose text price qty 2 (x$10 = $20 amount)
$priceSetParams['price_' . $this->priceSetFieldID] = 2;
$lineItem = CRM_Price_BAO_LineItem::getLineItems($participant['id'], 'participant');
CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $participant['id'], 'participant', $this->_contributionId, $this->_feeBlock, $lineItem, 0);
CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $participant['id'], 'participant', $this->_contributionId, $this->_feeBlock, $lineItem);
$financialItems = $this->callAPISuccess('FinancialItem', 'Get', [
'entity_table' => 'civicrm_line_item',
......@@ -462,6 +480,8 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
/**
* CRM-17151: Test that Contribution status change to 'Completed' if balance is zero.
*
* @throws \CRM_Core_Exception
*/
public function testCRM17151() {
$this->registerParticipantAndPay();
......@@ -487,6 +507,8 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
/**
* Test that recording a refund when fee selection is 0 works
*
* @throws \CRM_Core_Exception
*/
public function testRefundWithFeeAmount0() {
$this->registerParticipantAndPay();
......@@ -540,6 +562,8 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
/**
* dev-financial-40: Test that partial payment entries in entity-financial-trxn table to ensure that reverse transaction is entered
*
* @throws \CRM_Core_Exception
*/
public function testPartialPaymentEntries() {
$this->registerParticipantAndPay($this->_expensiveFee);
......@@ -579,6 +603,8 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase {
/**
* dev-financial-40: Test that refund payment entries in entity-financial-trxn table to ensure that reverse transaction is entered on fee change to lesser amount
*
* @throws \CRM_Core_Exception
*/
public function testRefundPaymentEntries() {
$this->registerParticipantAndPay($this->_expensiveFee);
......
tests/phpunit/CRM/Event/Form/ParticipantTest.php
+ 4
− 0
View file @ 06eaefe4
......@@ -102,6 +102,10 @@ class CRM_Event_Form_ParticipantTest extends CiviUnitTestCase {
$this->assertEquals(55, $sum);
CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $participants['id'], 'participant', $contribution['id'], $this->eventFeeBlock, $lineItem);
// Check that no payment records have been created.
// In https://lab.civicrm.org/dev/financial/issues/94 we had an issue where payments were created when none happend.
$payments = $this->callAPISuccess('Payment', 'get', [])['values'];
$this->assertCount(0, $payments);
$lineItem = CRM_Price_BAO_LineItem::getLineItems($participants['id'], 'participant');
// Participants is updated to 0 but line remains.
$this->assertEquals(0, $lineItem[1]['subTotal']);
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment