CRM-18573 Don't force payment for backoffice pending contribution

Submitted PR at https://github.com/civicrm/civicrm-core/pull/8469

After discussion with Eileen, I'm not sure that this whole thing is a good idea.

1) How does the A/R financial account get determined with Payment Method/ payment_instrument_id (actually it might come from the financial type for the payment processor).
2) There may be an issue with being unable to search for cancelled or failed payments by date if the received_date is set null. Is it the case that cancel_date is always set in this situation.
3) Probably good.

Hi guys,

I believe Joe's first point references an issue that arose with the introduction of 4.3 where a contribution is initially recorded with no payment instrument results in "unbalanced transactions" - I searched around quite a bit and couldn't find any original tickets that explain why the payment instrument is required, but I found a few tickets from Sarah related to the fixes applied for the issue:

https://issues.civicrm.org/jira/browse/CRM-16548
https://issues.civicrm.org/jira/browse/CRM-13420

I am concerned that the requested change will cause a regression of the original issue.

I disagree with the second point in the initial description that states "When Contribution Status is changed to Pending or Failed, Payment Received should be set to null on screen" - I'm not sure what the use case is for changing a completed contribution to a status of pending, but for failed contributions (specifically check/ACH contributions), there could be quite a difference between the original receive date and the failure date (i.e. once verification is received or the check is returned) - the original contribution may have already been batched and would correspond to a possible deposit that is later returned on a bank statement (e.g. in the form of a negative amount due to insufficient funds on the payer side), so both the original receive date and the actual failure date are both valuable pieces of information that should be preserved.

In regards to the third point of "If Payment Received is null, and Contribution Status is changed to a value other than Pending or Failed, Payment Received should be defaulted to now.", I am not sure how this is different from the current default of "now" in the case of a new contribution, but in the case of an existing contribution, the same logic as explained above applies, and one could also argue that enabling the option to change certain aspects of an existing contribution, especially one that has already been batched, is against accounting best practices (but now I am going off on a tangent), but other issues reported with financial items could be exacerbated by these additional changes if allowed (e.g. https://issues.civicrm.org/jira/browse/CRM-18179).

Hope this helps when making a final decision,
Tamar

Ignoring until specific client requests come in to fix this.

I think the issue is both general and specific:

1) In principal, a pending contribution is one in which no payment has been made. In CiviCRM-world, that is how the pending status is used when a contribution is made via a contrib form/event registration using pay later, and that is how admin users typically use the system via the backend. It is good practice to create the contribution record so you clearly define the obligation to pay and can easily search for your outstanding "invoices," – but no payment has been received. So I would begin by arguing this issue out of principal – we shouldn't create a dummy payment record attached to an invoice when no payment has been received.

2) More specifically, if a pending contribution is created with a dummy payment record, it does weird things to the data. The bookkeeping report will report a payment method of "check"; there's no way to record a partial payment; to record a payment against the contribution you must edit the contribution and update it (which although is something we've been doing in Civi-world for a while, doesn't really make much sense); on the contrib tab you can click to expand the contribution and view these mystery non-existent payments.

Part of what I'm arguing for is more consistency and precision. The contribution record is not the same as the payment, and so the two should not be conflated. Since we've already moved in a good direction with events (better exposure for the payment records) we should do the same, more consistently, for the base contributions as well.

I've closed the PR as it was stale & it is unclear if there is an intent to bring it back to life, given the ticket is closed. Please re-open as appropriate

Re-opening since there are issues here that need to be addressed at some point. We need more clarity around the design for how to address them, particularly how to refactor the use of existing payment methods to specify the financial type required to find the relevant accounts payable account. Do we want to add a generic one? If so, it would need to be changed once a payment is received and the actual payment method is known.

More generally, we likely want to split out the payment method from its one to one association with the contribution so that it is a relationship with each payment (represented the records in civicrm_financial_trxn that have is_payment true).

CRM-19249 specs out changes for some of the issues in CRM-18573.

Brian Shaughnessy, can we close this issue, or could you specify what needs to be done in addition to CRM-19249? Thanks, Joe

in my current testing, no payment record is being created. so yes, I think this can be closed.
I don't personally think CRM-19249 is necessary – I think it was discussed because you thought there was a need to create the payment record. but if that's not the case, I don't think that is necessary.