dev/financial#139 contribution receive_date shouldn't change when payments come in

[agh1]

Overview

The contribution receive_date would change when payments arrive on a pending contribution. This can cause reporting mayhem (your June totals may go down if you run the report after some payments arrive in July), and it generally goes against fundraisers' accrual perspective of donations.

This is a regression appearing in 5.18, but addressing it ran into issues dating to users' expectations from the "Update pending contribution status" search action (expressed in CRM-17146). Because it was using the IPN code, users assumed that the date specified on the payment should actually update the contribution's date because that code treated the two identically.

The solution was to rework that form so it strictly records a bunch of payments. The dates will become trxn_date on the payment, and the contribution receive_date is left intact. The contribution statuses will get updated according to the normal rules (which should always result in "Completed" because at least as of now the form will only let you record payments in full). If you want to edit contribution statuses, you can always batch update with a profile.

That meant I could remove the contribution status field, and in the process I renamed the action "Record payments for contributions" to better reflect this. The main use case is when someone checks the mail and has a stack of checks.

See dev/financial#139 for extensive discussion.

Before

1. Recording a payment that completes a contribution updates the contribution receive_date to the payment's trxn_date.

2. There's a search action called "Update pending contribution status".

   

3. Someone using "Update pending contribution status" is invited to manually set the contribution status, which could lead to unexpected results.

   

After

1. The contribution receive_date does not change unless you manually change it, even if it becomes completed.

2. The search action is renamed "Record payments for contributions".

   

3. There is no field to set the contribution status on that search action. The help text suggests using batch edit with profiles.

   

Technical Details

I added and updated a bunch of tests, including several instances where they are testing the opposite result of what they used to test.

Comments

It might be a nice improvement to that multiple payment form to allow for payments with different amounts, and it should work fine setting the contribution status to Partially Paid as appropriate. Similarly, it might be nice to allow payments on partially paid contributions. Both were out of scope here.

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

[agh1]

Jenkins retest this please

[eileenmcnaughton]

I tested this and it works. The button name is wrong - but I can do a follow up PR and I think it would be better to allow the payment amount to be altered - that can also be a follow up.

More seriously we have lost some functionality - ie the ability to bulk update from Pending to Failed / Cancelled.

There are so many issues with the code & functionality for that that I don't know how to start - however, the question remains - does anyone actually use it?

There is also so much good in this PR from a code maintainability point of view and more reliable clearer functionality.

I'm going to merge this but @agh1 I think we still need something - a blog? dunno - to try to surface if people use this for cancel / fail and perhaps a way to highlight this change / that functionality loss. I could probably put together an extension to replace it - but it's also possible no-one ever uses it...

[agh1]

@eileenmcnaughton can't you update contribution status with a profile in the batch update? This PR adds a line to the help text on the form suggesting that. Jul 29, 2020 11:54:14 PM Eileen McNaughton <notifications@github.com>:

…

I tested this and it works. The button name is wrong - but I can do a follow up PR and I think it would be better to allow the payment amount to be altered - that can also be a follow up. More seriously we have lost some functionality - ie the ability to bulk update from Pending to Failed / Cancelled. There are so many issues with the code & functionality for that that I don't know how to start - however, the question remains - does anyone actually use it? There is also so much good in this PR from a code maintainability point of view and more reliable clearer functionality. I'm going to merge this but @agh1[https://github.com/agh1] I think we still need something - a blog? dunno - to try to surface if people use this for cancel / fail and perhaps a way to highlight this change / that functionality loss. I could probably put together an extension to replace it - but it's also possible no-one ever uses it... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub[#17777 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAM2XR2ZUUSPUNQ4TPL3IS3R6DVGLANCNFSM4OVCNKFQ]. [https://github.com/notifications/beacon/AAM2XRYBMQMEBYWK2A3QQQ3R6DVGLA5CNFSM4OVCNKF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOE6ZZ56Q.gif]

[eileenmcnaughton]

@agh1 OK - I guess it should be highlighted when we do the release too