Better support in core for token payment processing
Over time token payments have become the preferred way of handling recurring payments. A while back we added the payment_token table for storage but we still have a situation where various processor extensions are solving the same problems separately (with varying degrees of maintainability). If we can converge on some improvements in core to better support this we can add tests in core & make the whole thing more reliable. Use of new methods would be recommended rather than required.
The focus here is on the regular scheduled job that finds recurring contributions that need to be charged, charges them and updates them in CiviCRM. I'm imagining we would add additional functions to the CRM_Core_Payment class that can be used and overridden by the various processors. We might visually separate them into a trait but I think it would be 'used' by CRM_Core_Payment.
The basic flow is
- updateSuccessfulPayments OR updateFailedPayments
Potentially status reporting - e.g emailing an administrator or logging to system log fits in at the end - I'm gonna leave that out of scope for now.
Get Due Payments
There are 2 types of payments here - those that are naturally due and those that are due to be retried due to previous failure. I think we should have 2 methods
- getDueScheduledPayments &
Preprocess due payments Generally we create a pending contribution here (repeattransaction, status-pending)
We also attempt to 'lock' the recurring contribution so that it won't be retried the same day if the script fails or if there is a concurrent process. I'm imagining the functions to be
- lockRecurringContribution (more on this down below)
** Submit payments to gateway*** This generally uses 'doPayment'
** Update Successful Payments Update Failed Payment** We call completetransaction for success & unlock the recurring contribution. Fails is the area we are trying to converge on
The main ways currently in use for 'locking recurrings' are to
- change the status on the recurring contribution
- add a pending contribution (& use the presence of that in a pending state as a blocker to further processing)
- push out the next_sched_contribution_date and failure_retry_date before starting so it won't be retried again before the schedule is due
After some thought I think we should do more with the status on the recurring contribution. My main reservation at the moment is that processors that use that method are overloading the 'Pending' status which actually means 'no payment yet received' - but I think that stems back to another design decision that I think was a mistake. Originally contribution, contribution_recur, pledge & pledge status were set up to share an option group. This causes a bunch of issues & hacky handling & as of now only contribution & contribution recur share statuses. I think we should separate them out into 2 option groups & then have some extra statuses that reflect the lock situation e.g
- 'Processing' - this would be our 'locked' status & nothing more would happen until that is resolved
- 'Failing' - this would mean one or more fails have happened but we haven't given up yet
This would mean that to be processed the next_sched_contribution_date needs to be now AND the status needs to not be 'Processing' OR the status needs to be 'Failing' and the failure_retry_date needs to be now.
Note that separating out the option group also addresses some other issues so I will likely do it in a 'numbers are all still the same' way sooner rather than later and we will be in a better position later.
Unlocking recurring Success is easy - we just complete the transaction & push next_sched_contribution_date & null our failure_count & failure_retry_date
Failure is hard. The fails could be
script does not complete. In this case the contribution is left in the 'Processing' status - in some cases people will manually intervene here and in some cases audit information from the processor will update the recurring. However, if, after a month it is still in Processing then it should be tried again. This feels like the case for pushing out next_sched_contribution_date BEFORE attempting payment
payment fails with a temporary error related to the server (server says come back later)
payment fails with a temporary error related to the user (card is overdrawn)
payment fails with a permanent error (card is cancelled, stolen)
payment fails with a permanent user error because they have cancelled the token at the gateway
Generally in a temporary error we will try again (using the failure_retry_date) and in a permanent error we will cancel the recurring contribution, in the case of the user-cancel it might make more sense for some sites to set it to completed....
Overall I think if we start to get to some agreed additional functionality to support in core / add to the interface & keep tested that is a good thing. I feel like I need to ponder it for a bit & this will be an ongoing thing. But there are 2 things I think are worth doing now
- add 4 new exception classes to core (all extending PaymentProcessorException)
- create a new contribution_recur_status_id option group - ensure the values exactly match the contribution option group & chip away at updating the various code places to use it (they should still work if using the old one as the numbers will be the same - this is how we managed it on pledges)
Initial discussion at https://github.com/iATSPayments/com.iatspayments.civicrm/pull/273
EDIT NOTE - 'Being processed' has been updated in this test to 'Processing' - since that was what was agreed. Comments below may be confusing without realising the text used to refer to Being Processed