Abandoned payments aren't being cancelled

wmortada commented 3 years ago

v1.8 added a schedulded job to mark pending recurring contributions as failed and the related contributions as cancelled (see release notes).

The relevant code is here: https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/blob/master/api/v3/Job/Gocardlessfailabandoned.php#L26-L81

The first part of this seems to be working fine. The recurring contributions are identified and marked as 'Failed'. However the second part seems not to be happening. Most contributions are left as 'Pending'. Only a few are updated to 'Cancelled'.

Looking at our scheduled job log I can see that this job is generating an error:

Full message: Finished execution of GoCardless: mark abandoned Pending recurring contributions Failed with result: Failure, Error message: Expected one FinancialItem but found 0

Looking at the code I think it is the following lines that are causing the error: https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/blob/master/api/v3/Job/Gocardlessfailabandoned.php#L70-L75

It appears to fail because there are no financial items associated with the contribution (because it wasn't completed). I'm not sure if this is due to a recent change in CiviCRM, but have found reference to what sounds like a similar issue here: https://github.com/mecachisenros/woocommerce_civicrm/issues/32

We're using CiviCRM 5.33.5 on WordPress 5.4.6 and v1.9.3 of this extension.

wmortada commented 3 years ago

It sounds like the answer may be to use the Order API. This post from Jaap might be helpful: https://civicrm.stackexchange.com/questions/28197/what-is-a-line-item-and-what-is-a-financial-item-and-how-are-those-two-linked-t

artfulrobot commented 3 years ago

@wmortada I've added a test but the test does not capture your failing case. As the problem you have is intermittent, I wonder what other context info you can give on the difference between ones that do get marked Cancelled and those that don't?

(PS. the Order.create API does not allow changing the status to Cancelled, so I don't think it's that)

wmortada commented 3 years ago

@artfulrobot thanks for looking in to this. I wouldn't describe it as intermittent as it affects the majority of the contributions for our site. Of about 1300 pending contributions only around 60 have been marked Cancelled. I will try to dig into why some are getting marked Cancelled and others not. That would be useful information.

I don't confess to understand the details of how CiviCRM manages financial data so may have got the wrong end of the stick with this, but I was wondering if the issue might be that the contribution wasn't created correctly (i.e. it is missing related financial items). This then causes an error when you try to delete the contribution. Would this be fixed by using the Order.create API to create the contribution in the first place?

I haven't been able to find anyone else that is having the same problem so it is possibly just an issue specific to our configuration (or perhaps no one else has noticed?)

Upperholme commented 3 years ago

FWIW is it not useful to be able to differentiate between a cancelled contribution and an abandoned one. I'd guess that it would be helpful to admins to be able to identify cases where the user initiated the process but didn't complete, so that they can follow up and encourage them to complete. whereas the approach you might take with a cancelled contribution is somewhat different.

wmortada commented 3 years ago

Actually, it is far fewer contributions that are getting marked Cancelled.

I've run the following SQL to find contributions that are linked to failed recurring contributions:

SELECT cc.contribution_status_id, Count(cc.id) FROM civicrm_contribution cc
JOIN civicrm_contribution_recur ccr ON ccr.id = cc.contribution_recur_id
WHERE ccr.contribution_status_id = 4
GROUP BY cc.contribution_status_id;

I get this result:

Contribution Status	Count
2 (pending)	2032
3 (cancelled)	9
4 (failed)	3

Comparing the 9 contributions that are marked as Cancelled with the ones that are Pending, the only difference I can see is that the creditnote_id is recorded.

Looking at the civicrm_entity_financial_trxn table I can see that there is an entry for each of the Cancelled contributions but no entry for the Pending contributions. (Though this may just be a symptom rather than the cause i.e. they have an entry for the cancelled payment.)

I'm curious to know why those 9 contributions did get cancelled but the majority didn't. Can't see anything obvious that differentiates them and some are fairly recent so they aren't all a historic anomaly.

wmortada commented 3 years ago

I should add that this isn't actually causing us any issues so from this point of view it isn't a high priority to fix and probably not worth spending too much time on.

Whether the contributions are marked as Pending or Cancelled doesn't make a difference to us at the moment (it may possibly be an issue in future if we want to track payment failures but this isn't something we are doing currently). I was just flagging this because it was something the extension was supposed to be doing but didn't seem to be doing on our site.

artfulrobot commented 3 years ago

is it not useful to be able to differentiate between a cancelled contribution and an abandoned one.

@Upperholme you can.

Abandonment only relates to an abandoned mandate setup. In this situation you'll see that the Contrib Recur is Failed, and the payment (that was Pending) is now Cancelled.

If a donor cancels their DD, then the Contrib Recur will show Cancelled (not Failed), I believe. If they had made successful payments on the mandate before cancelling then there will be no Contributions in state 'Cancelled'. If they had not, i.e. they cancelled before the first one completed, then there will be one 'Cancelled' one.

So:

you can tell the difference between a cancelled and abandoned (Failed) Contrib Recur.
A Contribution cannot itself be "abandoned" because the user is not present to choose to abandon; it can be failed (e.g. no funds) or it can be cancelled though.

@wmortada I'll check some of my client data to see if I'm experiencing the same thing.

artfulrobot commented 3 years ago

@wmortada I checked and I don't see this problem on a client's site; they don't have any pending ones, and do have some cancelled ones, so it looks like it's working there at least.

Re Order.create vs Contribution.create:

Neither API is used by this extension to create Contribution records. Core does that.
Contribution.create is used in the Utils class to do various updates to contributions. I don't think the Order API supports such functions?
Order.create is used in the import scripts (not things that are normally used, but jussayin')

One possibly related thing is: do you have proper accounts set up for GoCardless? There was an issue a while back with this - see upgrade_0002() in CRM/GoCardless/Upgrader.php

I can't do anything about this until it's reproducable. (Ideally with a test.)

wmortada commented 3 years ago

Thanks for checking @artfulrobot. It's sounding like it might be a peculiarity of our set up so I'm going to close this. I'll take a look at the upgrade to make sure that has happened.

wmortada commented 3 years ago

As far as I can tell the accounts are set up correctly.

"payment_type": "2",
"payment_instrument_id": "8"

artfulrobot commented 3 years ago

@wmortada

Looking at the code I think it is the following lines that are causing the error: https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/blob/master/api/v3/Job/Gocardlessfailabandoned.php#L70-L75

Better not to link to master/main branch as it moves!

Do you mean this Contribution.get fails https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/blob/1.10.1/api/v3/Job/Gocardlessfailabandoned.php#L61

Or this Contribution.create fails? https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/blob/1.10.1/api/v3/Job/Gocardlessfailabandoned.php#L70

Also, heads up: I'm about to move this repo to lab.civicrm.org so may need you to open a new issue there when that happens.

wmortada commented 3 years ago

Hi @artfulrobot, I'll try to remember not to link to master next time!

I believe that it is the Contribution.create that fails. I've tried doing this manually with the API explorer and get the same error message:

    $result = civicrm_api3('Contribution', 'create', [
      'id' => xxx,
      'contact_id' => xxx,
      'contribution_status_id' => "Cancelled",
      'note' => "Mandate setup abandoned or failed [TEST]",
    ]);

The result is:

    {
    "is_error": 1,
    "error_message": "Expected one FinancialItem but found 0"
}

No need to open a new issue on GitLab as I've closed this one.

artfulrobot commented 3 years ago

That suggests there's something up in the deep financial records somewhere :scream:

Dunno if these help or not:

Check the financial accounts set up for your financial types and payment processors. If there are some account types missing, that can cause problems.

Here's some diagrams I drew to try to understand how civi's financial entities relate(!)

Note: there's some exposure of financial records when you View a contribution, or click its amount (when viewing from the Contributions tab on a contact record)

wmortada commented 3 years ago

Thanks @artfulrobot, that's really helpful. I'll take another look.

That diagram is great and helps to explain some of the complexity behind it. Perhaps it should be included in the dev docs?

artfulrobot / uk.artfulrobot.civicrm.gocardless

Abandoned payments aren't being cancelled #113