artfulrobot / uk.artfulrobot.civicrm.gocardless

A CiviCRM extension providing GoCardless integration to handle UK Direct Debits.
GNU Affero General Public License v3.0
5 stars 18 forks source link

Successful payment recorded as failed if mandate cancelled after payment created but before payment completed! #54

Closed artfulrobot closed 5 years ago

artfulrobot commented 5 years ago

This is an edge case to do with cancellations close to the time the first payment completes; such that the first payment actually completes after the cancellation(!)

  1. supporter creates mandate.
  2. GoCardless creates a payment
  3. supporter (or admin via GoCardless website) cancels mandate
  4. GoCardless successfully takes payment

This results in:

But it should result in:

Why it fails

When a mandate cancelled webhook is processed, the contrib. recur record is cancelled and so is any pending contribution.

When a payment confirmed webhhok is processed, it:

solution A

  1. Assumption: GoCardless will not create a payment if the mandate is cancelled.

  2. Implement the payment created webhook - this would have to find the pending contrib, set its trxn_id.

  3. Change the processing of payment confirmed webhook to look up the contribution by the trxn_id - whether that payment is Cancelled or Pending.

  4. Only update Contrib Recur to In Progress if it is not already that, and then before making the change do a GC API lookup to check it's not been cancelled.

solution B

Change processing of the mandate cancelled webhook so that it does not cancel the pending contribution record. This would mean that the recur record showed cancelled while the payment remained pending. Presumably then GC would either send a payment confirmed event, or a payment cancelled/failed - which should both be fine.

As with solution A, it needs: Only update Contrib Recur to In Progress if it is not already that, and then before making the change do a GC API lookup to check it's not been cancelled.

I think solution B is better on the grounds that it is simpler.

artfulrobot commented 5 years ago

Solution B implemented.