[REF] Fix financial item allocation of negative payments against completed payments

[seamuslee001]

Overview

This the allocation of negative payments (created by Payment.create) such that pro-rata line items are created for the financial items

Before

Negative payment can be made against contribution but no financial item is created to allocate it against line items

After

Negative payments made using payment.create API create pro-rata line items

Technical Details

This fixes financial item allocation for a negative payment where the current contribution balance is fully paid (via the payment.create API)

ping @mattwire @eileenmcnaughton @monishdeb

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[mattwire]

@seamuslee001 I saw the discussion on mattermost around this. Can we get a bit more description about what it currently does and what we are expecting to see in terms of financial transactions/items?

[seamuslee001]

@mattwire I was working on the moneris plugin for JMA working on integrating full refund via moneris JMAConsulting/ca.civicrm.moneris@aa7e0a2#diff-61e08438be5781d5849a1554cae3f01eR401 I was finding that when i was calling payment.create on a completed payment with total_amount = - the current total amount and contribution_id = and this only had 1 payment. I was finding that the correct financial trxn was being recorded but no financial item was getting recorded because the allocation for the line items was getting set to 0

[eileenmcnaughton]

test this please

[eileenmcnaughton]

@seamuslee001 that test failed without this fix didn't it? This looks good to me. Tests were failing but will likely pass on re-run

[eileenmcnaughton]

@seamuslee001 so the remaining edge case is if you refund more than was actually paid - that overpayment would not have items.

[eileenmcnaughton]

@seamuslee001 test fail relates

[eileenmcnaughton]

Note failure is

Stacktrace
CRM_Event_Form_ParticipantTest::testPaymentAllocationOnMultiLineItemEvent
Failed asserting that actual size 5 matches expected size 4.

/home/jenkins/bknix-dfl/build/core-17810-4zcyi/web/sites/all/modules/civicrm/tests/phpunit/CRM/Event/Form/ParticipantTest.php:187
/home/jenkins/bknix-dfl/build/core-17810-4zcyi/web/sites/all/modules/civicrm/tests/phpunit/CiviTest/CiviUnitTestCase.php:209
/home/jenkins/bknix-dfl/extern/phpunit7/phpunit7.phar:615

[eileenmcnaughton]

@seamuslee001 this still has test failures

[seamuslee001]

ok I think I have fixed the failing test here

[eileenmcnaughton]

@seamuslee001 what if CRM_Core_BAO_FinancialTrxn::getTotalPayments() = 0 or is less than 0

I suspect in the former case you would get a divided by zero error.

I'm trying to get my head around the latter case but negative contributions are somewhat common whereas adding a refund to an already refunded contribution must be getting quite obscure but still possible I think

[seamuslee001]

@eileenmcnaughton I have now added in 2 unit tests one for adding an negative payment to an already refunded contribution and another adding another negative payment onto a negative contribution. In Neither case i encountered the fact that this was 0 because the $outstandingBalance was always non 0. Let me know what you think of the tests

[eileenmcnaughton]

Thanks @seamuslee001 - I think it's covered then