Skip to content

Commit

Permalink
Stop adding a refund payment from creating extraneous financial items
Browse files Browse the repository at this point in the history
Per
#15143 (comment)

and https://github.com/civicrm/civicrm-core/pull/13114/files#diff-8967f6e57d6209aa277ded6512413608L326
it seems agreed the financial items should not be created on refund.

This removes them & updates the test
eileenmcnaughton committed Sep 3, 2019

Verified

This commit was signed with the committer’s verified signature. The key has expired.
receyuki Rhys Yang
1 parent e0b4778 commit 36057c8
Showing 2 changed files with 34 additions and 29 deletions.
25 changes: 0 additions & 25 deletions CRM/Financial/BAO/Payment.php
Original file line number Diff line number Diff line change
@@ -385,31 +385,6 @@ protected static function recordRefundPayment($contributionId, $trxnData, $updat
if ($updateStatus) {
CRM_Core_DAO::setFieldValue('CRM_Contribute_BAO_Contribution', $contributionId, 'contribution_status_id', $completedStatusId);
}
// add financial item entry
$lineItems = CRM_Price_BAO_LineItem::getLineItemsByContributionID($contributionDAO->id);
if (!empty($lineItems)) {
foreach ($lineItems as $lineItemId => $lineItemValue) {
// don't record financial item for cancelled line-item
if ($lineItemValue['qty'] == 0) {
continue;
}
$paid = $financialTrxn->total_amount;
if (!empty(floatval($contributionDAO->total_amount))) {
$paid = $lineItemValue['line_total'] * ($financialTrxn->total_amount / $contributionDAO->total_amount);
}
$addFinancialEntry = [
'transaction_date' => $financialTrxn->trxn_date,
'contact_id' => $contributionDAO->contact_id,
'amount' => round($paid, 2),
'currency' => $contributionDAO->currency,
'status_id' => $paidStatus,
'entity_id' => $lineItemId,
'entity_table' => 'civicrm_line_item',
];
$trxnIds = ['id' => $financialTrxn->id];
CRM_Financial_BAO_FinancialItem::create($addFinancialEntry, NULL, $trxnIds);
}
}
return $financialTrxn;
}

38 changes: 34 additions & 4 deletions tests/phpunit/api/v3/PaymentTest.php
Original file line number Diff line number Diff line change
@@ -489,7 +489,28 @@ public function testDeletePayment() {
}

/**
* Test update payment api
* Test update payment api.
*
* 1) create a contribution for $300 with a partial payment of $150
* - this results in 2 financial transactions. The accounts receivable transaction is linked
* via entity_financial_trxns to the 2 line items. The $150 payment is not linked to the line items
* so the line items are fully allocated even though they are only half paid.
*
* 2) add a payment of $50 -
* This payment transaction IS linked to the line items so $350 of the $300 in line items is allocated
* but $200 is paid
*
* 3) update that payment to be $100
* This results in a negative and a positive payment ($50 & $100) - the negative payment results in
* financial_items but the positive payment does not.
*
* The final result is we have
* - 1 partly paid contribution of $300
* - payment financial_trxns totalling $250
* - 1 Accounts receivable financial_trxn totalling $300
* - 2 financial items totalling $300 linked to the Accounts receivable financial_trxn
* - 6 entries in the civicrm_entity_financial_trxn linked to line items - totalling $450.
* - 5 entries in the civicrm_entity_financial_trxn linked to contributions - totalling $550.
*/
public function testUpdatePayment() {
CRM_Core_Config::singleton()->userPermissionClass->permissions = ['administer CiviCRM', 'access CiviContribute', 'edit contributions'];
@@ -522,7 +543,6 @@ public function testUpdatePayment() {
foreach ($eft['values'] as $value) {
$this->assertEquals($value['amount'], array_pop($amounts));
}
CRM_Core_Config::singleton()->userPermissionClass->permissions = ['administer CiviCRM', 'access CiviContribute'];

// update the amount for payment
$params = [
@@ -531,9 +551,11 @@ public function testUpdatePayment() {
'id' => $payment['id'],
'check_permissions' => TRUE,
];
$payment = $this->callAPIFailure('payment', 'create', $params, 'API permission check failed for Payment/create call; insufficient permission: require access CiviCRM and access CiviContribute and edit contributions');
// @todo - move this permissions test to it's own test - it just confuses here.
CRM_Core_Config::singleton()->userPermissionClass->permissions = ['administer CiviCRM', 'access CiviContribute'];
$this->callAPIFailure('payment', 'create', $params, 'API permission check failed for Payment/create call; insufficient permission: require access CiviCRM and access CiviContribute and edit contributions');

array_push(CRM_Core_Config::singleton()->userPermissionClass->permissions, 'access CiviCRM', 'edit contributions');
CRM_Core_Config::singleton()->userPermissionClass->permissions = ['administer CiviCRM', 'access CiviContribute', 'access CiviCRM', 'edit contributions'];
$payment = $this->callAPIAndDocument('payment', 'create', $params, __FUNCTION__, __FILE__, 'Update Payment', 'UpdatePayment');

// Check for proportional cancelled payment against lineitems.
@@ -559,6 +581,14 @@ public function testUpdatePayment() {
foreach ($eft['values'] as $value) {
$this->assertEquals($value['amount'], array_pop($amounts));
}
$items = $this->callAPISuccess('FinancialItem', 'get', [])['values'];
$this->assertCount(2, $items);
$itemSum = 0;
foreach ($items as $item) {
$this->assertEquals('civicrm_line_item', $item['entity_table']);
$itemSum += $item['amount'];
}
$this->assertEquals(300, $itemSum);

$params = [
'contribution_id' => $contribution['id'],

0 comments on commit 36057c8

Please sign in to comment.