Payments - Where do we store IDs from payment processor?

mentioned in issue core#468 (closed)

@eileen @JoeMurray @monish.deb @KarinG Are you able to comment / add your views on how it should work and how it actually works at the moment (@KarinG eg. for IATS).

mentioned in issue #53

Hi @mattwire - great to finally meet you in person earlier this month!

We implemented the iATS extension following what had originally gone into Core for payment processing: a unique reference (invoice hash -> invoice_id) is sent over to the payment processor -> and the payment processor sends back a transaction id/response.

Here's a screenshot of a report that we include with the iATS extension -> reconciliation template - it includes all the relevant bits needed for a two-way reconciliation. This report pulls in data from the transaction journal (from iATS Payments; there is a verify job that pulls the journals hourly) and then in this report we match it up with CiviCRM contribution data using the now labeled Invoice Reference (also known as invoice hash or invoice_id) field:

$this->_from .= "
       LEFT JOIN civicrm_iats_journal {$this->_aliases['civicrm_iats_journal']}
          ON {$this->_aliases['civicrm_contribution']}.invoice_id = {$this->_aliases['civicrm_iats_journal']}.inv \n";

This report is useful for staff to find orphan transactions. For example: payments that got completed in iATS Payments but never made it to completed status in CiviCRM e.g. b/c of connectivity issues on the way back or custom code on the CiviCRM side throwing errors.

**Conclusion: **we still very much work on the contribution level. Each recurring contribution (as in the screenshot; that's a Membership recurring daily) - is a unique contribution - a unique invoice_id - a unique trxn_id comes back to CiviCRM. If a payment has failed - we don't re-attempt to get another payment for that Contribution - it's simply marked Failed and next_sched_contribution_date be will rescheduled and determine when a next Contribution will be created (when depends n the REJect reason).

Hopefully this explains (a bit) how we work things.

@mattwire so your description suggests we have gateway ids that map to payment, to contribution and to recurring contribution, and that we have fields on the various entities that map to them. That all makes sense to me.

What are the gaps to actually storing it the way you suggest. I would probably put the invoice id in the invoice_id field not the contribution.trxn_id field though. (I can't recall which it is - we have an invoice_id field & an invoice_number field I think - but one of those anyway since IATS is using that field it probably makes sense for Stripe & it's less ambiguous with trxn_id field

I can also see from @KarinG's comments that IATS is still working around the fact we don't do a great job of exposing payment info in search screens in the UI. I guess that is one gap!

invoice_id is generated on the CiviCRM side of things before the payment is attempted. It is the old historic hash (I think it dates back to CiviCRM 1.4.x or something like that - early Paypal days). That invoice_id is sent over to iATSPayments - where it is recorded in the iATSpayments journal -> here's what that looks like for the first contribution in the report screenshot above:

invoice_id (generated in CiviCRM) -> recorded in iATSpayments
trxn_id (generated in iATSpayments) -> recorded in CiviCRM

That's our two way reconciliation. Necessary for clients who use multiple systems/frontends into one iATSpayments processing account. The different/uniqiue invoice_ids immediately show where the transaction request originated from.

PS - invoice_number in CiviCRM is an easy to read and short string (essentially contribution with a prefix).

I don't think you want invoice_id in your trxn_id field - those are very different (generated in CiviCRM vs generated by Stripe)

Using civicrm_contribution_recur.processor_id makes sense for your Stripe subscription_id (again I don't think you want to use trxn_id for that).

@mattwire thinking about this I would support adding a new field - e.g payment_processor_reference to the civicrm_contribution table. I'm generally reluctant to add new fields but I think trxn_id is too easily confused with the payment level trxn_id and I suspect there may well be many places in the code that confuse the 2. @KarinG seems to be indicating IATS gets around that by not supporting partial payments but we probably don't want to bake that into core in any way.

As @KarinG points out neither invoice_id or invoice_number could be used here since IATs / invoicing code have existing uses for those fields.

mentioned in issue core#905 (closed)

If a payment processor returns more than 1 result/reference per payment (something in addition to a transaction id) and if you want to store that additional result in CiviCRM - I think there are two options:

glue both fields returned by the payment processor together and use civicrm_financial_trxn.trxn_id to store it (since this is per payment)
create a table for where you log the complete response -> here's our iats_response_log for iATSPayments [don't be confused by invoice_num header - as you can see that is the invoice hash - the structure of this table predates the Core invoice_num field] - you can then join this table in a report to expose the additional data to admins.

Actually, iATSPayments extension totally supports partial payments. Here's what that looks like: trxn_id is stored at the payment level (always). Note we use glue to add a unix timestamp to trxn_id (Alan really likes very unique references!). That unix timestamp is the time we received the response from iATSPayments - so it's also a field returned by the payment processor.

In case of 1 payment start to finish trxn_id is also stored at the contribution level [where it resided historically before partial payments]. For most use orgs/use cases - contribution:payment are 1:1 and GUI is right there, so this is handy for admins. If orgs use partial payments they can but they will need to learn how to navigate/use the financial transaction reports.

Also noting that the use of invoice_id/hash in/by iATSPayments extension was modeled after how core payment processors (PayPal and Authorize.net) use invoice_id/hash.

So that really re-inforces that trxn_id is already being used in ways that mean something different to 'order id / invoice_id' in the Stripe context. This doesn't seem to be just a Stripe thing to me - ie. paypal has an order concept https://developer.paypal.com/docs/api/orders/v2/ -

We've had a few conversations about whether we concatenate check_numbers / trxn_ids in the UI so all are visible or whether we put our efforts into promoting the visibility of the payment trxn_ids in the contribution search instead. It has been off the boil for a while though. The risk is that fixing it in core breaks something in extensions though

It's not just Stripe. I've just done a 2checkout implementation and it would be the same as Stripe.

Invoice / Order ID Stripe = Invoice ID, 2Checkout = Order ID
Charge / Transaction = Unique "charge" attempt. Whether successful or not.
Invoice / Order ID = a chargeable item (which may have multiple transactions (charges) as each attempt to take payment has a separate (charge) transaction_id).
Charge ID = Each unique "charge" attempt. For stripe a failed payment may be retried multiple times, resulting in multiple unique charge IDs.

Sounds like PayPal is similar.

Ignoring issues around UI / core code constraints for now it makes sense that:

civicrm_contribution.trxn_id maps to the order / invoice ID.
civicrm_financial_trxn.trxn_id maps to the unique "charge" attempt. civicrm_financial_trxn.status_id should be used to record the "failed" status of a retried "charge" attempt.
We need an extra field on civicrm_financial_trxn to record the description/reason (eg. Card declined, Fraud check failed).

As an aside there was a conversation about the core financial reports not handling multicurrency (ie. just adding all the numbers together as if they are the same) so it may make sense that we work out what should be done, then come up with a transition plan to make it work in the code, UI, reports.

Add extra field on civicrm_financial_trxn; that would be helpful -> we've been storing that info into the Contribution Source field - to give admins easy/quick feedback on e.g. why recurring contributions failed:

re: "civicrm_contribution.trxn_id maps to the order / invoice ID."

Sorry - I'm not following... the CiviCRM invoice id is a unique id generated by CiviCRM prior to the Payment attempt. It is to be sent over to the Payment processor (all payment processors require a unique id/reference from the initiating system); Are you saying Stripe is then generating an additional invoice id (or order id) and that you want to store in the contribution trxn_id field?

Sorry - I'm not following... the CiviCRM invoice id is a unique id generated by CiviCRM prior to the Payment attempt. It is to be sent over to the Payment processor (all payment processors require a unique id/reference from the initiating system); Are you saying Stripe is then generating an additional invoice id (or order id) and that you want to store in the contribution trxn_id field?

@KarinG We have no control over the Order/Invoice ID for Stripe as they are generated when we setup the payment. They are guaranteed to be unique on the payment processor side, but there's nothing to stop them clashing with IDs generated by other systems (eg. in theory Stripe and 2Checkout could generate an Order ID that was the same, but for the different systems obviously).

all payment processors require a unique id/reference from the initiating system

So this bit is not true for all payment processors. For Stripe / 2Checkout we send a unique single-use "token" to the payment processor (which the payment processor generated on the users browser via javascript) but we MUST not save this, and it has no meaning once used anyway. Stripe / 2Checkout then send us a unique Order ID which we store. The "invoice_id" in CiviCRM means nothing in this context, and we are not able to supply it to the payment processor.

For processors where we can pass in a reference it does make sense to use the invoice_id field as it's guaranteed to be unique in CiviCRM. (Note that for Smartdebit we generate references locally based on an incrementing sequence to represent the subscription (recurring contribution) (this could probably have been done using the invoice_id if we were starting again), but not for each actual contribution which are simply matched by date).

I'm about to start reworking Authorize.net - this is what it does currently: $template->assign('refId', substr($this->_getParam('invoiceID'), 0, 20)); - because AuthNet restrict their reference to 20 chars. There is obviously a (very small) risk there that we will get a duplicate on the first 20 chars but at least we can control the ref.

You're obviously the expert on Stripe- so please do tell me if I'm out to lunch - but what about Stripe's metadata, like e.g. Link ID ('Attach your system's unique IDs to a Stripe object, for easy lookups. For example, add your order number to a charge')? Can you perhaps use that to pass along the CiviCRM generated invoice_id?

Re: Auth.net -> exactly. I was actually the one who trimmed that to 20char in Core (we migrated a client to us from another CRM and they were using Auth.net). But that's exactly what I'm referring to - a unique ref from CiviCRM to payment processor is very helpful for reconciliation (whether automated or manual).

Ok let's leave CiviCRM generated unique references out of scope & keep this issue to how we save unique references from the processor.

As @mattwire says in both CiviCRM & in Stripe there are 3 unique entities, each having a Stripe reference. We have obvious places to store the reference for payments and for subscriptions but there is no obvious field for the order/invoice/contribution.

From a data model point of view trxn_id is the obvious place and it would be easy to store data there. What I worry about is that there is already a push to hide fields in the UI that expose contribution fields & instead expose the relevant payment fields as the contribution fields are not necessarily accurate. Perhaps that would be by an opt in setting - I don't know. But the uncertainty around that field & all the fields that should have been moved rather than copied to the payments/financial trxn table makes me hesitate to recommend it.

added triaged label

added type:proposal label

added needs:concept approval label

CiviCRM generated unique references are in scope if the plan/proposal is to overwrite them with Payment Processor generated unique references.

I would prefer we add a .paymentprocessor_order_id (or something like that) field to the Contribution table to store the Payment Processor generated Order/Invoice ID.

OK - so sounds like @KarinG is agreement with this comment #57 (comment 18119) so I think we are OK to add a field here @mattwire

mentioned in issue #60

Just circling back around to this. Do we have agreement (in principle) that:

civicrm_contribution:

Requires a new field processor_id like we have in civicrm_contribution_recur. This would hold the unique invoice/order reference from the payment processor.
trxn_id is deprecated and should not be used - as it has been replaced by the one on civicrm_financial_trxn

civicrm_contribution_recur:

Deprecate trxn_id as it is ambiguous and unclear what it should be used for (when we have processor_id already).
Make it clear that processor_id is used to hold the unique subscription reference from the payment processor if one exists.

civicrm_financial_trxn:

Add description field which can be used to hold a string describing the individual transaction (eg. the payment processor may provide a decline reason "failed fraud check" or "manually authorised").

@JoeMurray @eileen @KarinG I'd like concept approval from you. I realise the actual implementation of the above could have various issues but let's address those once we've agreed in principal how it should work!

Sorry I'm only getting through a backlog of @ping messages in lab now. I like the direction the discussion has been going (I monitored but didn't weigh in earlier). My one suggestion is more related to maintainability and understandability than functionality.

processor_id as a field name seems confusing. On the standard pattern for semantics of fields in CiviCRM, it should contain the id of the processor related to the record it is on, in this case civicrm_contribution and civicrm_contribution_recur. What you're suggesting it means is the order id or invoice id sent back from the payment processor. Could we change the name to something more along these lines, like processor_order_id? The description for a field in a MySQL Comment is nice, but the field name should not be misleading.

I think that's largely correct although 'deprecate trxn_id' is mostly philosophical in that we would encourage use of payment trxn_id fields for any payment level information and the new field (I'm probably with processor_reference since id implies a table connection & I feel it was a bad name choice) for any contribution level information but I don't see any practical changes in the short term.

In the medium term I think we'd grandfather in exposing the payment trxn_id (and payment instrument it, check number etc) in the place where we currently expose trxn_id so it might in the long term become invisible in the normal UI. I don't really think we would ever actually remove it.

@adixon @bgm @andrewhunt I feel you also have an interest

hmm failed ping ^^

All - thanks for your efforts in cleaning things up, it sounds like you've got a good way forward. I really like both the goals and conclusions - e.g. that the transaction id field in the contribution table is misleading and should be a payment level field. Also great that you've come up with an idea for a meaningful and now documented-ish use for the processor_id field in the recurring contribution table - that was a terribly labelled field, and when I first came across it I searched in vain for any documentation or reference to it in code or forum or even feedback from developers!

If there's a good place to put documentation for stuff in progress, I'd be happy to review/edit and even write.

Noting here that civicrm_contribution_recur should also have a source/description field

@AlanDixon yes - the first challenge is probably figuring out what is documented where & where should it be documented :-)

In discussion at the BCN sprint, it was concluded that the id should be recorded at the payment level so we'll add order_reference on civicrm_financial_trxn. There is the possibility that a contribution can be paid by more than one payment processor.

See my comment on your pull request, I think your conclusion is flawed, or at least lacking clarity. Perhaps you could describe what goes in the order_reference field and which piece of code would be responsible for providing it?

PR https://github.com/civicrm/civicrm-core/pull/15468

@AlanDixon so the discussion came down to (ie the group discussion at the sprint)

sometimes payment processors need to record an 'order reference' - ie the processor reference for an order/contribution
it's not inconceivable that an order/contribution has payments from more than 1 processor - so they could wind up 'fighting' over that field. On the other hand financial transactions / payments have a payment_processor_id field so can only relate to one.
most processors simply won't care but those that do need to record an order_reference have a place to store it. (At this stage we are imagining processors will take care for storing these values & we are just adding an agreed place to store them but we might add more api support later

We now have order_reference on civicrm_financial_trxn: https://github.com/civicrm/civicrm-core/pull/15468 This can be used (once we've got API support) to record an order/invoice ID generated by the external payment processor (as required by eg. Stripe, 2checkout).

mentioned in commit core@1f34d30a

mentioned in commit core@dedc9a94

changed the description

@mattwire we added order_reference but I think some more docs are needed - we probably need to add it to the Payment.create api whitelist - since that got lost when I did the 5.19 patch & also find some way to document.

I'm kinda keen to close this issue but I think documentation needs to be done

mentioned in issue #83

@eileen Both docs and APIs need to catch up before we can close this. I've commented on #76 about where the API needs to go to do this.

The APIs need to be able to store / retrieve this information and do something transparent with legacy data (eg. trxn_id on contribution instead of order_reference on financial_trxn). Otherwise we won't be able to transition across to use these new fields.

Moved from #76:

Storing payment processor order references

We've added the order_reference field to the payment (civicrm_financial_trxn). The idea is that it will replace the trxn_id on contribution but we need to expose via API and implement methods that allow us to transition. For example a modified version of this is required https://github.com/civicrm/civicrm-core/pull/14748 so that a search by trxn_id actually searches all linked payments order_reference fields (but falls back on the contribution trxn_id for legacy data).

At the same time we should expose trxn_result_code via the API so this can be used (as agreed at the sprint) to store information returned from the payment processor such as card declined, pending verification

Note that not all payment processors currently use the contribution trxn_id field in the same way but we need to support something that allows payment processors to transition without worrying about where data was stored historically - this could be via a map function on the individual processor perhaps?

The new field should be exposed for Payment.create - trxn_result was exposed her https://github.com/civicrm/civicrm-core/pull/15537 - I don't know whether Payment.get returns them or not yet.

I think the expectation about where we are going with a search for 'trxn_id' is that it would search for the trxn_id in the civicrm_financial_trxn records - let's leave that out of scope. I'm sure other gitlabs focus on it.

The order_reference field was discussed in the context of providing a place for payment processors to manage references to a gateway-provided-reference for the 'order' - but not one that would be populated except by processors who need to store that so it shouldn't need to be exposed in the UI but if that's a requirement for Stripe we should look at the UI to make sure it can be overriden

The order_reference field was discussed in the context of providing a place for payment processors to manage references to a gateway-provided-reference for the 'order' - but not one that would be populated except by processors who need to store that so it shouldn't need to be exposed in the UI but if that's a requirement for Stripe we should look at the UI to make sure it can be overriden

Ok, it is pretty much essential that it's exposed via the UI!! Pretty much all the processors that I'm aware of work in this way and currently use trxn_id on the contribution record. Stripe, GoCardless are just two. It is the most important thing to the end user as it's what allows them to cross-reference between CiviCRM and their payment dashboard.

@mattwire so to be clear - we are still expecting the civicrm_financial_trxn.trxn_id to be where the value is stored for any ids that represent the payment but IN ADDITION most of the processors you work with have a second reference field that represents the ORDER and that (not the payment one) is what users see when they log in?

In the payment display we already concatenate several values into the payment method on rendering - perhaps if order_reference is available it could be after trxn_id in brackets like the extra info on visa is in the payment method field

@eileen Yes that's correct. In general the ORDER maps directly to the contribution and represents a set of attempted, successful, failed, refunded payments for that ORDER. As we acknowledged at the sprint it is theoretically possible that the contribution might be paid by multiple processors / topped up by a cash payment etc. which is why we put the order_reference at the payment level.

But the order_reference is most useful to the end user at the top (contribution) level and I would propose that it replaces the trxn_id field in the contribution UI by default (in fact probably concatenates contribution.trxn_id + distinct(n*payment.order_reference)).

I agree with you that it should also be displayed in brackets after payment.trxn_id.

I think concatenating it into the trxn_id (where present) in the UI is fine - but any changes to deprecate contribution fields in favour of any new fields in the UI should be out of scope of this issue as it's a whole kettle of worms & the main use case put forwards at the sprint was that when managing a recurring contribution the processor needed to be able to send the order_reference back to the processor & that use case is met by adding docs for the new field & ensuring the Payment api exposes it. Burying a conversation about changing core forms at the bottom of a long issue discussing having a place to plonk data is not going to give good clarity

mentioned in issue #82

mentioned in issue extensions/smartdebit#8 (closed)

mentioned in commit core@bee2b57c

Payments - Where do we store IDs from payment processor?

Child items 0

Activity