Fix Bug where Payment Balance is sometimes miscalculated

[eileenmcnaughton]

Overview

Fixes a bug where Add Refund is displayed instead of Add Payment on partially paid event contributions

Before

Notice that when viewing a partially paid transaction getPaymentInfo is called twice - the first time is with usingLineTotal = TRUE & results in the Record Payment button about half way up.

The second is at the bottom & the mystery param is not passed i - resulting in RecordRefund

After

Technical Details

I observed that when this function is called without the mystical the paymentBalance is miscalculated

The payment balance is calculated as the Contribution Total less the amount paid (for less queries the
total is passed into getContributionBalance & used if passed in). We were passing in the Balance
as the total causing the balance to be calculated as the Balance less any amount paid.

From what I can tell this function has been honed & cleaned up but because the parameter never made much sense
the impact of different variants was not really tested. This removes & fixes

Comments

@monishdeb @pradpnayak

[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).

[seamuslee001]

@eileenmcnaughton style warning need to remove https://github.com/civicrm/civicrm-core/pull/16546/files#diff-7a5b0e2d131dabc49178460fc63328c0R4020 as well

[seamuslee001]

This seems to be fine to me but would appreciate others to comment @monishdeb @mattwire

[mattwire]

@eileenmcnaughton It would be helpful to have a comment explaining why this variable is called oldCalculation as it's a mystery to me. Non-blocking to merging of this PR but if you get chance.

[eileenmcnaughton]

Oh - because I think it is weird & should go

[eileenmcnaughton]

@mattwire - I've expanded the code comments

[mattwire]

I'm happy with this from an r-code perspective. Could really do with an r-run from someone like @monishdeb @pradpnayak @elcapo

[jaapjansma]

Betty and I are reviewing PR's and we came across this PR.

• General standards
  • Explain (r-explain)
    • FAIL We missed a step by step instruction of how to reproduce this as a user. We managed to find it out ourselves but that was quite a bit of work. We have given this a FAIL because we do think @eileenmcnaughton can do better. However we don't think this blocks the merging.
  • User impact (r-user)
    • PASS: The change would be intuitive or unnoticeable for a majority of users who work with this feature.
  • Documentation (r-doc)
    • PASS: The changes do not require documentation.
    • COMMENTS: Although this a pass we discovered that the user guide does not have a section on how to handle partial payments. We have added an issue for updating the user guide: https://lab.civicrm.org/documentation/docs/user-en/issues/1 However it is not blocking for this PR.
  • Run it (r-run)
    • PASS: We registered for an event (Rain Forest Cup) as Tiny Tots and paid the $ 800 fee by credit card. We then changed the registration in the back end to Junior Stars. We saw the button record payment, record credit card payment appear. We also tested this in dmaster and the buttons did not appear.
• Developer standards
  • Technical impact (r-tech)
    • PASS: The change preserves compatibility with existing callers/code/downstream.
  • Code quality (r-code)
    • PASS: The functionality, purpose, and style of the code seems clear+sensible.
  • Maintainability (r-maint)
    • PASS: The change is trivial enough that it does not require tests.
    • COMMENTS: We have given this a PASS however based on the feedback from previous reviewing we have a gut feeling @eileenmcnaughton might say this needs a test as it involves payments ;-)
  • Test results (r-test)
    • PASS: The test results are all-clear.

@mattwire or @eileenmcnaughton could one of you merge this PR?

[mattwire]

Thanks @jaapjansma