Christmas is long over, yet we have a wish: we want to introduce a new field to `civicrm_contribution`, (tentatively) called `sequence_number`. This will give contributions records belonging to a recurring contribution some self-awareness, by recording which installment in the series each contribution record represents.
Our itch to scratch here regards to direct debits, where we need a simple and reliable method for determining which installemnts have already been created, and which still need to be created. But let's think big: it seems to me that storing this information should be useful for other types of recurring contributions as well.
Note the emphasis on reliable. In the ordinary case, we should be able to derive the sequence number easily by counting the contribution records. However, in the Real World (TM), nothing ever goes as it should -- especially when computers are involved :-)
For one, counting only works if the contribution records in CiviCRM are complete. And while CiviCRM is meant to be omniscient, this might not always be the case: for example when importing from legacy systems; or if an IPN callback from the provider doesn't get through for some reason. Also, individual contribution records sometimes might be deleted or duplicated -- whether due to technical imperfections (impossible! yet real), human error (same...), or even intentionally -- for example when dealing with failed payments.
In such cases it would be tricky for smart humans, and virtually impossible for the dumb machine to figure out what happened. Of course these are all more or less exceptional events -- which is to say, they are actually quite common; and when they do occur, it is very comforting to have some reliable records helping with understanding the situation...
Or getting back to our own itch (how could we have ignored it for so long?): it is really crucial to reliably know the current sequence number -- even in exceptional situations -- if followup payments are generated internally by the payment extension: such as direct debits triggered from CiviCRM.
So, now that we established the rationale, you are surely itching to see the code? Well, I'm sorry to say we don't have it just yet -- but I will follow up as soon as I get to it, which I expect to be in a week or two, barring some medium-sized or larger catastrophe.
(Not worth holding your breath very much though: it will be just a simple schema change plus upgrade SQL, and some basic form code to display the field for recurring contribution installments.)
Ah, but I have saved up one really interesting question for you :-) What to do with the existing Payment Providers? Ideally, the sequence number shall be passed from the remote server in the IPN, and can be stored in our shiny new DB field directly. Now this would be so useful, that I fully expect at least some providers to omit this information. In such cases, we might want to cheat a bit, deriving the sequence number from the existing payment records instead. This wouldn't be really reliable though, which is the whole point of introducing the field in the first place... Still, it might be useful, if generic mechanisms start using this information. So what do you think?
Comments
Hi Antrik.
Checking the current version of PayPal standard documentation for recuring payments I see they return a payment cycle value with the IPN which is pretty close it seems to a payment number. If you know of another payment processor that also provides a payment number then I am happy to see this field added to the schema. I am not as keen on using such a field to store a CiviCRM generated value, since a date field could be used to sequence the payments.
I do not think there is added value beyond the sequencing provided by dates to filling a sequence_number field with CiviCRM generated numbers. I don't put much stock in this working while the dates don't because they would be handled/created by the same process and are thus not sufficiently independent. In auditing you want separate workflows to come up with the same result in order to increase your confidence in the result. But I don't think this is something I would oppose strongly if there were a different good rationale for it.
I don't want to start a long debate here about a tangential rationale you provide, but I think the workflow and processes that lead to a deletion of a contribution would likely not be prevented or diagnosed more easily with an additional field. Although it is theortically possible for a recurring payment to go through but the associated IPN to fail, I haven't heard of this and the circumstances and backup systems in place and date and authorizing transactions should be sufficient to sort thins out.
IPN processes are usually designed to be quite fault tolerant. For example, they tend to have generous retry settings, with up to several days between later retries, and there are alternative payment notification channels such as email or portals available as backups in case of problems such as IPNs not working for a few weeks even though payments are being processed.
A minor note to be settled at detailed design perhaps with input from Dave Greenberg is whether this should be a field in civicrm_financial_trxn which represents actual payments rather than the much higher level civicrm_contribution table. In the case of a recurring payment there should be a one to one correspondence, so the convenience of having the field on the large contribution record may outweigh the semantic elegance of pairing it with the financial_trxn that more properly represents a payment.
Why is the benefit of storing that paypal sequence number then? to support audits later to check to payment has been missing? Isn't Eileen approach more robust (store all the message from IPN)
I'm not sure it's worthwhile to add another column to a already quite bloated one given that:
- it's only for recurring payments not for "normal" ones
- and only if they have their PP IPN that handle that data
- it's not solving any pb of any missing payment if the sequence number is generated locally
@antrik, I'm not sure I understand why you need that sequence number for sepa, but it's a separate discussion. Why don't you add it as a field of sepa_sdd_contribution_group table?
more sense to store the externally derived data than to calculate data to fill the field. If some payment processors return this data that may well be adequate rationale to add the field. However, the alternate design pattern would be to add a field to store the extra data that is relevant to the specific processor. In that case we would be adding more or less a free-text 'settings' or 'metadata' field either to the contribution table or as Joe suggests the financial_trxn field.
This is a pattern we have discussed a little previously in other contexts - e.g adding a settings field to tables like contribution_page so that extensions that modify it don't have to create a table per extension - however, I have gone down the path of using a separate extension (entitysetting) for that.
I don't have a strong opinion on adding another sometimes-used field to civicrm_contribution or adding another field that is available for various processors to use 'as appropriate to that processor' - I'm just putting it out there as the alternative.
I agree that IPNs are not always completely reliable. Authorize.net, for example, does not repeat it's IPN calls & historically I have debugged various bugs that have caused IPNs to fail& be lost. As far as I know they are are all fixed now :-). However, we run a hack that causes all IPNS to be logged in their raw state to a table as soon as they hit the server.
I strongly agree with Xavier and Eileen that it would be better to store the full IPN callback string than to cherry pick out just one field from it that may need to be interpreted. I think rolling Eileen's hack/extension into core is a fine idea, and if that is not done then making your extension have a dependency on it would be a good second choice. One reason is that it provides more information that may be useful when things go wrong - more robust indeed. Another is that not all payment processors will have the same implementation of 'sequence number'.
I had written a different post along the lines of Xavier's question about why this field is needed, whether it could be added to specific payment processor extensions, and whether your colleagues in Project-60 are on-board with suggested design. I'll leave it to those more heavily involved to take up that discussion.
We have 2 separate things going on - & I'm not 100% sure which Joe is referring to
1) We have a hack (not an extension) that causes all IPNs to be logged. (https://github.com/fuzionnz/civicrm-core/commit/8045bc7bf0f4713ef63145d95e5022058153dba9) - this is a hack rather than an extension as it needs to over-ride code in the extern folder (because I want it logged before it has a chance to hit anything that might not work). I think we have general agreement in principle to add this to core & it is a contender for my time at the Tahoe sprint.
2) We have an extension which we use as a dependency for a number of other extensions which stores settings that relate to various entities https://civicrm.org/extensions/civicrm-entity-settings. We haven't discussed putting this into core but we can have that conversation.
Both store the data as a JSON array & hence neither is particularly queriable. The difference is that the first just stores all IPNs 'as they hit the server' with no analysis for forensic purposes & the second stores 'interesting data' keyed against a particular entity & retrievable via api.
The first is the responsibility of the file-first-hit, the second is the responsibility of the extension. Probably the entity-setting approach is more appropriate to Antrik.
It seems this issue has some strong opinions, and I share the concern about being able to match things up between CiviCRM and the payment processor. I'm just about to write some code to do this for a payment processor (iatspayments), so here's what I'm thinking.
I'm not sure if or how this might translate into a general framework for core - a lot of this kind of reconciliation will depend on the tools you get from your payment provider, but maybe someone with more accounting experience has an idea.
I assume you will ensure credit card details are not stored in the transaction details that you log.
I think we have customers compare a book keeping report against the payment processor report & keep that communication log stuff for forensics