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

Payment Method is null for recurring GoCardless payment #63

Closed yorkshirerose closed 4 years ago

yorkshirerose commented 4 years ago

I've just been trying to set up a smart group (using Advanced Search) with the following criteria:

This causes errors ("DB error: unknown error") and then all smart groups appear to break.

If I use Receive Date = last 30 days instead of Recurring Payments = last 30 days, it's fine.

When I look through the error log, I did notice "Ignoring exception thrown by nullHandler: -1, DB Error: unknown error". I also noticed, when poking around, that the Payment Method for the transaction (not the contribution) is null, despite being a mandatory field (see screenshot).

Untitled

Site is running WordPress 5.3.2 and CiviCRM 5.19.4 and GoCardless extension 1.8

artfulrobot commented 4 years ago

@yorkshirerose Hmmm. I don't see that - I checked 2 of my sites that use this extension, and I checked recently created (this month) contrib recurs and older ones (July).

If the transaction is completed you should not be allowed to click that pencil icon as it should say "contribution is tied to a processor" or something, so you're def right that it's missing something, but how that happened I don't know.

The create contribution code seems to rely on passing the contribution_recur_id for identifying the payment processor; it does not explicitly set payment_processor in Contribution.create API calls.

I think there have been some major changes behind the scenes in core on payments APIs. There were some other issues with 5.19 that were addressed by GC 1.9beta2 https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/blob/1.9beta2/README.md#19-for-civicrm-519

So I'm left wondering

It you're able to run SQL, what's the output of this query?

select cc.payment_instrument_id, count(*) 
from civicrm_contribution cc 
inner join civicrm_contribution_recur cr on cc.contribution_recur_id = cr.id 
inner join civicrm_payment_processor pp on cr.payment_processor_id = pp.id 
inner join civicrm_payment_processor_type pt on pp.payment_processor_type_id = pt.id 
where pt.name='GoCardless' 
group by cc.payment_instrument_id;

(this query shows each payment instrument (i.e. method) and how many contributions have it for all GoCardless contribs)

simonjohnparker commented 4 years ago

Funnily enough, I've just spotted this on our site and in googling the issue, found this, and realised the request is actually about our site @yorkshirerose ?

Does this need your investigation Rich? I'm hoping this isn't going to affect any of the recurring memberships that are going to be taken next year?

artfulrobot commented 4 years ago

I'm hoping this isn't going to affect any of the recurring memberships that are going to be taken next year?

If you're on 1.9beta, then subsequent payments are recorded using repeattransaction which will copy the data from the last contribution. As it looks like you have corrected your existing contribution data, it should copy those and be OK.

But something is/was odd, so it deserves investigation - I know you're going to ask someone to look at this, just updating here for general benefit of other readers.

mattwire commented 4 years ago

@artfulrobot I reproduced this on master using a local buildkit. I configured a contribution page in exactly the same way as the original site (Contribution page: no confirmation; stripe+gocardless; membership priceset with memberships set to auto-renew required). When the webhook comes in the payment does not get a payment_instrument_id set. Switching to Payment.create API fixes the issue - #70

artfulrobot commented 4 years ago

Thanks (again!) Matt. Do the tests pass with this change?

Nb. Also, this seems related https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/issues/67 with backwards-fix in (as yet unreleased) https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/commit/589f92d856f9f4e6eb6b8f8d1d1080891e31af8e#diff-88b95ffe133e110a0db7562f15afd0c9R216

I wonder if this could be at the nub of this one, too: https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/issues/68