Define the logic that sets (or not) contribution receive_date in relation to payments

changed the description

Here's a small contribution of further context and background information.

It would be good in several areas of CiviContribute to more clearly articulate a separation between obligation to pay or cancellation of that, on the one hand, and the payments/failures/refunds, etc on the other. Then there are related communication tasks, including sending one or more receipts during the workflow, and thank you letters. Some countries like Canada also have separate official tax receipt modules with their own dates. Recurring contributions send out two receipts, one of which has links to update/cancel/{something else} the recurring series if the payment processor supports each of these actions. The DB and the UI should have dates that correspond to when there are transitions on both sides of this.

I believe historically we ended up with additional single fields like receive_date and cancel_date and thank_you_date because the schema was premised on original workflow of contribution + single payment + single action to cancel/fail + single thank you. Now that multiple payments are supported, and cancel/fail/refund on payments and the whole transaction as well as creation of a credit note that can be applied either partially to several other purchases or all at once to one, we should be doing a thorough review.
We may want a model where the contribution fields are a convenient way to get the currently accurate information for the 'invoice', with blow-by-blow history elsewhere in terms of all the changes of what is owed, payments and refunds including initiation, and interim or final status.
FYI, the revenue recognition date is for deferred revenue recognition. This is particularly relevant for recognizing annual membership payments on a monthly basis, and for delaying the recognition of event revenue until the month of the event.
In terms of the schema, I see the following. I'm including recurring fields for reference, and also because we're seeing some of the ideas that formerly were only relevant to recurring contributions, like retry dates, now being implemented for one-off payments by processors like Stripe.

civicrm_contribution.receive_date 'Date contribution was received - not necessarily the creation date of the record'
civicrm_contribution.cancel_date 'when was gift cancelled'
civicrm_contribution.receipt_date 'when (if) receipt was sent. populated automatically for online donations w/ automatic receipting'
civicrm_contribution.thankyou_date 'when (if) was donor thanked'
civicrm_contribution.revenue_recognition_date 'Stores the date when revenue should be recognized.'
civicrm_financial_trxn.trxn_date 'date transaction occurred'
civicrm_financial_item.created_date 'Date and time the item was created'
civicrm_financial_item.transaction_date 'Date and time of the source transaction'

civicrm_contribution_recur.start_date 'The date the first scheduled recurring contribution occurs.'
civicrm_contribution_recur.create_date 'When this recurring contribution record was created.'
civicrm_contribution_recur.modified_date 'Last updated date for this record. mostly the last time a payment was received'
civicrm_contribution_recur.cancel_date 'Date this recurring contribution was cancelled by contributor- if we can get access to it'
civicrm_contribution_recur.end_date 'Date this recurring contribution finished successfully'
civicrm_contribution_recur.cycle_day 'Day in the period when the payment should be charged e.g. 1st of month, 15th etc.'
civicrm_contribution_recur.next_sched_contribution_date 'Next scheduled date'
civicrm_contribution_recur.failure_retry_date 'Date to retry failed attempt'
civicrm_contribution_recur.failure_count 'Number of failed charge attempts since last success. Business rule could be set to deactivate on more than x failures.'

Adding a link to https://github.com/civicrm/civicrm-core/pull/17433 which I opened and then closed. It was an attempt at cleaning up the use of receive_date/trxn_date in our payment APIs.

added comp:CiviContribute comp:Payments git:civicrm-core sig:financials sig:payment triaged labels

A strong +1 to @JoeMurray's comment about separating the obligation to pay and the payment itself. For organizations that typically receive all donations in real time (i.e. online payments) and receive the balance in full for single line item contributions, the current model is simple and sufficient. But for any org handling more complex scenarios, the separation of concepts is critical.

I often will communicate and describe contributions as invoices (sometimes even relabeling it through word replacements). In the majority of cases, the invoice and payment are created/received in a single process. But in some cases a pending contribution (open invoice) is created and payment is received at a later time. Or an invoice is created/paid and then the amount adjusted, resulting in a balance due (partial payment). And there are many other scenarios.

I also agree with @JoeMurray's 4-point assessment. Much of what we're seeing is a result of older (simpler) structures that have evolved to accommodate more complex workflows -- but were not fully modified to truly embrace the more robust functionality now available.

Per your question -- I definitely don't think the contribution receive_date should be modified when the Payment.create API is used. I've run into that before and it's a real pain to work around (particularly with data imports, where you end up creating the contribution, creating the payment(s), and then must "fix" the contribution receive_date).

Even in simpler data models and usage I don't think we should make assumptions about the receive date when storing a payment.

In the past there have been proposals to rename 'receive_date' to contribution_date to more accurately reflect the need for a data that serves as the date of the order rather than when we got the monney.

I've always been against renaming contribution.receive_date because it feels really scary -but I can see that it's not the right term and I think adding an additional field is less scary -

ie contribution_date would

use DB defaults to be created as the current date
expose it for create & get via api
not change any existing flows but potentially expose in reports etc.

If we did that it would make sense to alter receive_date each time a payment is received (as happens now)

I'm not sure we need another date field like that. It feels like denormalisation, too - the date of the last payment/change is available in other records.

Renaming that field is scary - just think how many custom reports and extensions must rely on it! - but I don't think there's much harm in not updating it. A code (and SQL column) comment saying "The date the order was received." would suffice, I think.

My opinion is that we should NOT be updating receive_date when payments (or other transactions) occur.

I agree with not updating it

Very much agree with not updating receive_date when payments occur and agree with not renaming receive_date. It's just not necessary. A comment to clarify 'date order received' would help if you feel clarification is warranted.

mentioned in issue core#1831 (closed)

I strongly believe that receive_date should not be updated when the payment is received. This is a regression within the past year: I just tested a bunch of versions, and it appeared in 5.18.

I also want to address something @artfulrobot said:

for most payments that are made in a single lump, it's probably the most important date: when did we get the money (not just the promise/hope/intent of money)?

This is only true for organizations that use cash-based accounting rather than accrual. As used in most reasonably-sized organizations, accrual-based accounting would consider the date of the promise to pay--the original receive_date of the pending contribution--as the date when the revenue is recognized.

In particular, this is really important in fundraising: the fundraiser's work is reflected when the donor says they'll give, not when the cash hits the bank.

The status quo doesn't even work for cash-based accounting, though, except for contributions that are paid with a single payment: the date is only modified when the final payment switches the contribution status to Completed. Cash based accounting records each bit of money as it's received.

Changing the date around makes it even worse: it's a problem if you say you had $100,000 of donations in June but then POOF! the total is only $99,000 because a payment for a $1,000 contribution arrived this week.

Now, imagine what a nightmare this would be after the end of the year: contributions made in the old year but paid in the new year would get counted in both years' financial reports if the old year's report was run before the payments arrive.

A client first noticed this regression when a report of fundraisers' amounts raised showed someone soliciting a lot of money while she was on vacation. She had made the ask and the contribution was recorded the prior week, but when the contribution arrived, the date got adjusted.

I feel like this discussion endorses changing the Payment.create action such that receive_date is no longer updated and I would accept a PR that did that.

@eileen I started doing this but I noticed that this runs smack against CRM-17146. I think I have the right change, and I wrote a test to confirm it (see https://github.com/civicrm/civicrm-core/pull/17777) but it appears that there's a test to confirm the exact opposite introduced in https://github.com/civicrm/civicrm-core/pull/6712. That follows changes introduced in https://github.com/civicrm/civicrm-core/pull/6684, which is in response to someone thinking that setting the transaction date in the bulk update of contribution status should change the receive_date (CRM-17146).

I think they're dead wrong and those tests are trying to lock in bad behavior. However, I want to be careful because they are there.

@andrewhunt I think this conversation is a who's who of folks/shops with educated opinions, minus @jitendra who patched CRM-17146.

There's a significant chunk of issues on dev/financial that revolve around the need to separate obligation-to-pay/paid, and a decent number of folks willing to contribute resources. I think we're overdue to resolve #43 and its siblings, but my understanding is that the work of @mattwire @eileen @artfulrobot et al. to reduce the number of code paths for recording contributions is a prerequisite. Is that accurate?

I think the real problem is that this "update contribution status" form makes no sense under the new payments paradigm.

What would/should happen if I selected "Failed" or "Pending Refund" from the status drop-down? I think this is the sort of stuff that went away with the move to recording payments for completing individual contributions.

It more accurately should be a "bulk record payments" form. If someone really wants to bulk edit statuses (or other fields) without recording payments, they can use the batch edit with a profile.

This form is what the person in CRM-17146 was working with, and I think now the consensus might be that the "Transaction Date" should be on the payment only, not the contribution.

The comments in the code for that form give me tons of confidence :

  // set some fake input values so we can reuse IPN code

  // @todo calling CRM_Contribute_BAO_Contribution::completeOrder like this 
  is a pattern in it's last gasps. Call contribute.completetransaction api.

So you are saying we fixed a form issue by changing the BAO?

No, sorry, I'm saying that form reflects old thinking about how contributions and payments work, and I suspect that someone who actually tries using it to set the contribution status to anything besides "Completed" will have surprises, either because the status won't take effect or the payment won't be recorded right.

I think the logic of CRM-17146 was "okay I'm editing a bunch of contributions to show they're paid, so this "transaction date" should be receive_date". Also, that form is working through the IPN method to complete the transaction, with one set of params for the contribution and payment, so I suspect there's no way to use that to set the date on the payment distinct from the contribution.

If we instead decide that form is a way to add payments that complete the selected contributions, the contribution status field should go away, and the transaction date can clearly refer to the payment's transaction date.

Out of scope here, but a nice thing once the dust settles, is the opportunity to change that amount column to an editable field so it would be possible to record a partial payment.

@andrewhunt Can you add a note how to find that form? It's one of the actions after find contributions?

@mattwire go to Find Contributions (or get contributions in Advanced Search), then it's the action "Update pending contribution status".

I'm just working on "future recurring start date" functionality for Stripe (similar to what IATs already does @KarinG ).

This creates a pending contribution with receive_date some days into the future and no financial transactions (because it hasn't happened yet). What is missing is the date the contribution was created and there is no create_date for a contribution. We really want to record both the "date the promise was made" - create_date?, the expected receive_date - in this case the date that Stripe will generate the invoice, and the actual transaction_date (the date which Stripe successfully takes the payment).

@rich @andrewhunt Does it make sense to add a create_date field to the Contribution entity so we always have the date it was created?

I can't see a problem with adding a created_date field.

Or perhaps the problem is actually (my?) use of the receive date for future recurring start dates? I wouldn't want to add a field that might create new confusion and complexity, just to support a fairly narrow use case.

Keeping the receive date as the date of the commitment makes more sense to me, and then instead rethink how future recurring start dates might be better implemented.

e.g. for cheques, the receive date is not the date the payment actually goes through, right? We do have a separate field for tracking the date of payments against a contribution.

A better implementation might be a way to create a 'pending' payment of some kind? That could also be used with ACH/EFT and would better reflect what's actually going on?

We also have to consider what the admin sees - currently the only date they see before a payment comes in is the receive_date and they need to know when they are expecting to actually receive a payment for that contribution (it is only too common with failed IPNs to have abandoned contributions in pending state).

Perhaps instead of the created_date we use receive_date as the created_date (as it already is in most cases unless you are dealing with future start dates or have triggered the "updated when the payment came in"). Then add a expected_date field or similar and expose that in the UI instead of receive_date, falling back to receive_date if we have no expected_date.

The issue with creating a pending payment is it doesn't deal well with failures and retries and does not give a clear indication of when the contribution is expected to be settled (it may be settled a day or so after the expected date for various reasons but we'd still want to respect the expected date).

That opens another can of worms.. the date on which linked memberships etc. should be set to. And whether those dates come from contributions or recurring contributions.. :-(

Generally (except for deferred revenue as covered below) I'd say the receive_date should be the date when the revenue hits the books, regardless of when the payment arrives, and most of the time that would be the date a contribution is created. The situations when a create_date might matter are more like if I were entering paper contribution forms today from a fundraiser last weekend. The receive_date would be July 4, but the create_date would be July 10 (and the payment's trxn_date would be whenever they send a check next week).

Recurring contributions are funny because at least the second one onward are separate contributions that should be recorded on the day they arrive. You haven't made a commitment to pay in August, September, etc. when you set up a monthly recurring contribution today. You're just setting something up for convenience. (This is the distinction from a pledge where you promise to pay the whole amount in a bunch of payments.)

From one perspective @AlanDixon you might say that if I set up a recurring contribution that should run on the 15th of each month and I decide to cancel tomorrow, before anything runs, I haven't reneged on anything. By that measure, you'd be right in considering the receive_date to be the date when the first recurring contribution runs, and the first contribution would be treated identically to the subsequent ones.

On the other hand, it might also be reasonable to treat the first contribution differently because it was initiated by a person and not a schedule. The receive_date would be the date you fill out the form, and that contribution would just have a payment arriving a few days later. This treats the initial contribution in a similar way to normal one-off contributions. When the recurring contribution hits again on August 15, you're recording a new contribution and payment all at once, so those dates are all the same. The trick here is that you'd need a place to stash the setting to run the payment on the 15th, and the recurring contribution code would need to check to see if it needs to create a fresh contribution or just record the payment to the pending one in the series.

@mattwire to your last paragraph about the date that memberships relate to, I think that's what the revenue recognition date is attempting to cover. If my membership expires at the end of the year, I fill the form to renew today, and I mail a check August 1, the receive_date should be today, the payment trxn_date should be 2020-08-01, and the revenue_recognition_date should be 2021-01-01. My contribution should be counted when someone wants to know how much membership revenue there was in 2021.

This is a different can of worms, perhaps somewhat orthogonal to the specific problem we're trying to solve, i.e. I'd like to ignore the bookkeeping aspect and focus on the functional problem of handling future start dates.

@AlanDixon I just think we're close to having our story straight with receive_date being the date revenue is recorded from a bookkeeping perspective, so the question really comes down to whether you consider the situation of a future start date to be

Just like the subsequent contributions in that they happen when they happen (and receive_date would be the date it's scheduled to run), or
Just like a one-off contribution that's paid later in that the promise is made right away (and receive_date would be the date it's set up).

Just noting that there's a precedent for having a created_date separate from a start_date in CiviPledge, which is conceptually very close to what we're discussing here.

Yeah, you're right and the payment system is fairly tied into the bookkeeping which would make it a bit of a mess, so that was a bad idea.

A new, mostly empty and ignored field labelled "expected date" might work, though it doesn't feel right either.

Here are five ways I can think of when we get pending contributions that are not immediately completed:

A pay later contribution.
An off-site processor that doesn't return (payment may or may not actually happen).
A future contribution commitment.
A recurring contribution that gets initiated from CiviCRM that never completes (at least, we have that w/ iATS).
An ACH/EFT contribution, pending completion information from the bank.

What we want is a way to handle all of these possibilities: cleaning up, alerting, reporting, etc. because they all are bad-ish temporary states that we don't want to have hanging around but we can't resolve immediately. The iATS extension handles 3 + 5 in fairly ad hoc ways in system jobs, and might do 4. in the future.

Have there been other related discussions about handling pending contributions? Is there a mechanism that we can create to handle all these and other potential pending contribution states?

I'm imaging some kind of new entity that can describe why the contribution is pending, when it might be resolved, whether any action is required (automated and/or manual), etc.

Or maybe the existing actions table is the right thing to use?

I don't know how/where it's set off the top of my head, but we have a rudimentary "why the contribution is pending" reflected in the "(pay later)" and "(incomplete transaction)" suffixes in the display. Maybe those could be standardized and extended.

Ah yes, good point, whatever that mechanism is would be a good place to start.

The distinction there comes from whether there's an associated financial transaction. Which kind of loops back to the original post at the top which I've just reread.

I think my conclusion is: can we use our existing financial transaction system to identify a future payment like this? It's not quite like a pay later, but maybe similar enough?

It would seem like a good opportunity to provide more of that visibility of the transaction information to administrators referred to above.

I believe the difference between "Pending (Pay later)" and "Pending (Incomplete transaction" is just whether or not the is_pay_later. There are no financial items/transactions for a pending contribution so we can't infer any info from them.

I don't believe this is correct. I took a look at the code and an example and found a financial transaction entry for a pay later, and noticed that there was a test for the existence of this transaction when determining whether or not it was a pay later or incomplete transaction.

@artfulrobot I think we have agreement that contribution receive_date should not be updated by a payment - would you be able to do a follow-up PR to https://github.com/civicrm/civicrm-core/pull/17688 to "fix" that?

I think this is the PR that does that https://github.com/civicrm/civicrm-core/pull/17777

@andrewhunt then discovered that the PR caused another test to break.

I think that means we have to do 'something' with that form too - ideally making receive_date an option. I'd like to find time to look at it as cleaning up that form would remove a blocker on cleaning up IPN stuff in general - but I might not get to it.

I think the minimum is the form could pass in receive_date to completeOrder (yuck ) and then the update would only happen if that is passed in

Ah, I missed that. Is it currently possible to do "Batch update via profile" on receive_date. In which case we have an alternative and could remove the Update pending contribution status and add a note to upgrader/release notes about the alternative. That's one less bit of broken(ish) code to maintain in core!

Yeah - I'm on the fence about that - perhaps we can do an emoji poll on it - ie change behaviour (& get rid of cruft / inconsistencies) and announce in release note vs ensure that form uses parameters to keep the behaviour the same just for that form.

I probably lean to the latter since I think it keeps this 'bite' pretty small & from looking at the code I think it's quite straight forward

I think there are a few things someone might want to do that relate to the "Update pending contribution status" form, in descending order of likelihood:

Record payments for a bunch of contributions (like when you have a stack of checks that came in the mail).
Update the status for a bunch of contributions without recording payments (maybe you want to clear out a bunch of pending/incomplete contributions).
Update the receive_date for a bunch of contributions (like maybe they were entered manually with the current date but they really should be back-dated to when someone filled out a paper form).

The first thing is the only thing that actually works on that form. It records payments whether you like it or not. The title is a bit of a misnomer and maybe dates from the era before recording payments. The latter two things should be done using batch update via profile.

I do not think anyone is visiting that form intending to record payments and set the contribution status to something other than "Completed" (which recording a payment would trigger anyway)--and I don't think it would work consistently, either. I also do not think anyone is visiting that form intending to record payments and modify the receive_date of the contributions. (I mean, I don't think most people are thinking too hard about the nuance here, but they're not expecting it to work differently than recording payments one at a time.)

Consequently, if we were very specific that a reworked form is "Record payments in bulk", that would do the trick.

I think here's a rough roadmap for this:

Rework "Update pending contribution status" to be "Record payments in bulk" and make it do just that.
Clean out the things that were built to handle CRM-17146 since the payment date should be recorded correctly if the bulk form is reworked, and the form processing won't be going via the IPN methods anyway. - Rework the tests from CRM-17146 since they are testing the opposite of what we want.
See if the APIv3 Contribution.completetransaction changes in https://github.com/civicrm/civicrm-core/pull/17777 are needed anyway after all this (the test should be included, presumably)

I just finished work on PR #17777 which should take care of all of this--both the receive_date issue and the multiple payments form. Please take a look and let me know what you think.

mentioned in commit core@3e93468d

mentioned in commit core@39052b78

mentioned in commit core@5d9f8015

mentioned in commit core@15074087

mentioned in commit core@fd7525a8

mentioned in commit core@a9e97cdf

mentioned in commit core@e52f7008

Thanks @andrewhunt for the now-merged PR. Can we close this? Or does this need a documentation update so we can improve community understanding of what's supposed to happen?

I think we can close it

closed

changed the description

Reference	date	observed current behaviour
➊	contribution `receive_date`	Shown in lists. Create date, then updated.
➋	contribution `revenue_recognition_date`	NULL - don't know when/if this gets set
➌	financial item 1's `create_date`	Date contribution was created
➍	financial item 1's `transaction_date`	Date completing (or latest?) payment made
➎	financial trxn 1's `trxn_date`	Could not observe; is not created (think this is a bug see link above)
➏	financial trxn 2's `trxn_date`	Date of the completing payment

Define the logic that sets (or not) contribution receive_date in relation to payments

Child items ...

Activity