[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Page MenuHomePhabricator

Gravy dupe contributions cleanup
Closed, ResolvedPublic

Description

We've had reports of PayPal donations made through Gravy coming back in via the PayPal audit and being recreated as direct PayPal donations. Let's get a count of these and remove the duplicates.

Let's also confirm that we're ignoring gravy-originated transactions that come in via the direct backend processor audit files.

Event Timeline

So, this query counts all PayPal Gravy dupes. There's 2470.

1SELECT
2 COUNT(*) AS gravy_paypal_duplicates
3FROM
4 civicrm.civicrm_contribution c1
5WHERE
6 invoice_id LIKE '%|dup%'
7 AND EXISTS (
8 SELECT
9 1
10 FROM
11 civicrm.civicrm_contribution c2
12 WHERE
13 c2.invoice_id = SUBSTRING_INDEX(c1.invoice_id, '|dup', 1)
14 AND c2.trxn_id LIKE 'GRAVY%'
15 )
16 AND payment_instrument_id = 25
17 AND receive_date >= DATE_SUB(
18 CURDATE(),
19 INTERVAL 1 MONTH
20 );

and then amending it slightly to show the results

1SELECT
2 *
3FROM
4 civicrm.civicrm_contribution c1
5WHERE
6 invoice_id LIKE '%|dup%'
7 AND EXISTS (
8 SELECT
9 1
10 FROM
11 civicrm.civicrm_contribution c2
12 WHERE
13 c2.invoice_id = SUBSTRING_INDEX(c1.invoice_id, '|dup', 1)
14 AND c2.trxn_id LIKE 'GRAVY%'
15 )
16 AND payment_instrument_id = 25
17 AND receive_date >= DATE_SUB(
18 CURDATE(),
19 INTERVAL 1 MONTH
20 )
21LIMIT 10;

AKanji-WMF moved this task from Triage to DRI Backlog on the Fundraising-Backlog board.

To confirm this batch only includes the PayPal donations from the one-hour test, I pulled out the min and maximum dates of the dupe donations and the dates line up.

MariaDB [civicrm]> SELECT 
    ->     MIN(receive_date) AS earliest_date,
    ->     MAX(receive_date) AS latest_date
    -> FROM 
    ->     civicrm.civicrm_contribution c1 
    -> WHERE 
    ->     invoice_id LIKE '%|dup%' 
    ->     AND EXISTS (
    ->         SELECT 
    ->             1 
    ->         FROM 
    ->             civicrm.civicrm_contribution c2 
    ->         WHERE 
    ->             c2.invoice_id = SUBSTRING_INDEX(c1.invoice_id, '|dup', 1) 
    ->             AND c2.trxn_id LIKE 'GRAVY%'
    ->     ) 
    ->     AND payment_instrument_id = 25 
    ->     AND receive_date >= DATE_SUB(
    ->         CURDATE(), 
    ->         INTERVAL 1 MONTH
    ->     );

+---------------------+---------------------+
| earliest_date       | latest_date         |
+---------------------+---------------------+
| 2024-11-06 18:43:35 | 2024-11-12 10:39:16 |
+---------------------+---------------------+

If we can get this query into a searchkit, you can delete them easily from the UI and it will get all the different pieces

Looking for them a different way, I get 5221 lines in the audit file with the telltale Gravy descriptor string:

ejegg@civi1002:/var/spool/audit/paypal/completed$ grep 'Payment for items' TRR-202411* | cut -d',' -f 2 | sort | uniq | wc -l
5221

We should be able to use that string as a way to drop them. I'll try to check that list of IDs against Civi. Very curious that there would only be 2470 with the invoice ID duplicated.

@Dwisehaupt or @Jgreen I've got a patch in the puppet checkout in my homedir on frpm that should hopefully fix this

0001-Avoid-double-filing-Gravy-Paypal-transactions-in-aud.patch

Could you please push that to the puppet repo?

Pushed. It'll hit the servers on their next puppet run.

Date: Tue, 19 Nov 2024 14:16:00
From: Puppet Notify <fr-tech-ops@wikimedia.org>
Subject: [frack::puppet] e3138b437 Avoid double-filing Gravy / Paypal transactions in audit (Elliott Eggleston:Dallas Wisehaupt)

M       modules/civicrm/templates/audit/paypal-audit.yaml.erb

Thanks, @Dwisehaupt!

So that should filter them out as long as they keep sending the same descriptor string (the IPN filter relies on the same thing). I did a query on wmf_contribution_extra looking for gateway_txn_id matching the list of transaction IDs grepped out of the TRR files and found the same 2470 count that @jgleeson found searching for the |dup string. I guess some of those lines must not have been actual successful transactions.

Searchkit here also with 2470: https://civicrm.wikimedia.org/civicrm/admin/search#/edit/3619

Spot checking them I've found a couple so far that were indeed duplicates but we only have the paypal one in civi:
221301618.1
221286122.1 - this one has the situation where they used a different email in the form, then you can't search on their email in the gravy console

Thanks, @Cstone, for adding that Searchkit; it makes it much easier to look through them. However, I noticed in the SQL of the Searchkit that we're not confirming that an existing Gravy one exists (using the EXISTS query) unless I'm missing that somewhere. Was that tricky to add? I guess the count is the same,, so we might be ok, but I added that check to ensure they had a corresponding Gravy row before we started deleting them en masse.

Spot checking them I've found a couple so far that were indeed duplicates but we only have the paypal one in civi:
221301618.1

Do you mean we only have the Gravy one in CiviCRM? I see the Gravy donation here, but no PayPal, which makes me wonder why the Audit didn't pull this one in too. Another bug maybe?

221286122.1 - this one has the situation where they used a different email in the form, then you can't search on their email in the gravy console

Yeah this is annoying. I've added a ticket for this issue. T380356: Gravy Paypal email mismatch problem Until we fix it, we need to search for these cases on merchant ID or 'Label', which is what gravy uses to refer to the Paypal email address, e.g. My Paypal donations

@Dwisehaupt or @Jgreen I've got a patch in the puppet checkout in my homedir on frpm that should hopefully fix this

0001-Avoid-double-filing-Gravy-Paypal-transactions-in-aud.patch

Could you please push that to the puppet repo?

Thanks @Ejegg , for finding a quick way to stop these.

Could we add a comment to this config template explaining why something as generic as:

Item Name:
    - Payment for items

stops Gravy-initiated audit records from being processed and also why we want to reject those, to save our future selves from scratching our heads looking at it if we ever need to update or add to it.

There's a comment above the reject block specific to 'PayPal grant disbursement emails', so it would be helpful to clarify that this rejection is related to something else.

We need to make sure Gravy never changes this string if we're going to detect it here like this. I haven't seen the files, but I wonder how reliable you think this will be. Should we confirm with Gravy?

Spot checking them I've found a couple so far that were indeed duplicates but we only have the paypal one in civi:
221301618.1

Do you mean we only have the Gravy one in CiviCRM? I see the Gravy donation here, but no PayPal, which makes me wonder why the Audit didn't pull this one in too. Another bug maybe?

Ahh I see what happened, so there are both, the Gravy donation went to a new contact while the paypal one went to the already existing contact
cid=65777192 only gravy
cid=14467421 everything else

Interesting!

Spot checking them I've found a couple so far that were indeed duplicates but we only have the paypal one in civi:
221301618.1

Do you mean we only have the Gravy one in CiviCRM? I see the Gravy donation here, but no PayPal, which makes me wonder why the Audit didn't pull this one in too. Another bug maybe?

Ahh I see what happened, so there are both, the Gravy donation went to a new contact while the paypal one went to the already existing contact
cid=65777192 only gravy
cid=14467421 everything else

@Cstone, should we delete these using your new Searchkit now that we're fairly confident we've got the right batch?

XenoRyet set Final Story Points to 4.