Proposal - remove net_amount from contribution edit form - at least in edit mode.
When people edit the total_amount on a contribution they have to edit the net_amount to match - why? We can calculate it.
What happens now?
For new contributions with no fees it is enough for the user to enter total amount. However, if there is a fee they need to enter the fee AND calculate net_amount and enter them. If they edit the amount they need to update the net amount to be the same as the total amount less the fee amount.
The current behaviour dates back to svn days - however since 2015 the BAO has handled the possibility of it not being set.
What issues are there with this
- It causes pain for users
- In addition we have had code issues with the comparison around currency & float comparison issues and sales tax. In both cases the user cannot enter the correct data.
- It undermines our unit testing - in Mar 2017 the unit tests were edited to remove net_amount from the form submission values in our unit tests. New tests written since then have been based on these net_amountless tests.
[Proposed change Remove the field altogether from the form] (https://github.com/eileenmcnaughton/civicrm-core/commit/0bee3ce6fa18fa24d75c7bb9e86618ff0ac1f025)
Risks I would hope some people would commit to testing the rc - but as mentioned above our tests already expect it not to be set so we are already testing the proposed scenario.
there is a risk that some people really like the fact that when the edit the total_amount and not the fee_amount they get a validation error - IF that seems to be a real issue - not just me flailing around for reasons - then we could either move the fee_amount field or have some UI sugar mentioning the fee_amount next to total_amount.
Here is the BAO function that calculates it.
/**
* Calculate net_amount & fee_amount if they are not set.
*
* Net amount should be total - fee.
* This should only be called for new contributions.
*
* @param array $params
* Params for a new contribution before they are saved.
* @param int|null $contributionID
* Contribution ID if we are dealing with an update.
*
* @throws \CiviCRM_API3_Exception
*/
public static function calculateMissingAmountParams(&$params, $contributionID) {
if (!$contributionID && !isset($params['fee_amount'])) {
if (isset($params['total_amount']) && isset($params['net_amount'])) {
$params['fee_amount'] = $params['total_amount'] - $params['net_amount'];
}
else {
$params['fee_amount'] = 0;
}
}
if (!isset($params['net_amount'])) {
if (!$contributionID) {
$params['net_amount'] = $params['total_amount'] - $params['fee_amount'];
}
else {
if (isset($params['fee_amount']) || isset($params['total_amount'])) {
// We have an existing contribution and fee_amount or total_amount has been passed in but not net_amount.
// net_amount may need adjusting.
$contribution = civicrm_api3('Contribution', 'getsingle', array(
'id' => $contributionID,
'return' => array('total_amount', 'net_amount'),
));
$totalAmount = isset($params['total_amount']) ? $params['total_amount'] : CRM_Utils_Array::value('total_amount', $contribution);
$feeAmount = isset($params['fee_amount']) ? $params['fee_amount'] : CRM_Utils_Array::value('fee_amount', $contribution);
$params['net_amount'] = $totalAmount - $feeAmount;
}
}
}
}