[00:01:23] ejegg: so the only code path that would hit the lines you are changing is coming from the recurring queue consumer - is there any reason to think there would be any difference in input pararms or required treatment from there? [00:01:50] hmm, good question. maybe I should mark this WIP till i take a harder look at it [00:02:37] I guess my gut says that there is probably not a huge amount of contact updating going on from incoming recurring queue [00:03:01] maybe paypal? [00:03:05] i dunno though [00:03:22] I see signs of name field updates & address & email [00:05:19] 10Fundraising-Backlog, 10Wikimedia-Fundraising-CiviCRM, 10fundraising sprint Parallel processing roller coaster, 10fr-matching-gifts: Omit useless entries when syncing matching gifts data - https://phabricator.wikimedia.org/T264944 (10Ejegg) OK, it looks like the NULLy rows aren't being created via the mat... [00:16:58] (03CR) 10Eileen: [C: 03+1] "So my digging says that this would only ever affect calls from the recurring queue." [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/714434 (owner: 10Ejegg) [00:34:08] (03PS1) 10Cstone: Remove (int) to stop rounding down of below 1.00 amounts [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/714445 (https://phabricator.wikimedia.org/T285396) [00:45:41] (03CR) 10jerkins-bot: [V: 04-1] Remove (int) to stop rounding down of below 1.00 amounts [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/714445 (https://phabricator.wikimedia.org/T285396) (owner: 10Cstone) [00:47:04] (03CR) 10Eileen: "cstone test fail looks like a red herring - my guess it's throwing an exception due to type hinting" [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/714445 (https://phabricator.wikimedia.org/T285396) (owner: 10Cstone) [10:10:49] 10Fundraising-Backlog, 10MediaWiki-extensions-CentralNotice: Translation for Central Notice banners needs improvement: please simplify and lower the workload! - https://phabricator.wikimedia.org/T288577 (10Nikerabbit) Translatewiki.net is a website. The translation extension is called just Translate. This loo... [11:00:41] 10Fundraising-Backlog, 10Wikimedia-Fundraising-CiviCRM: Campaign Notification list - https://phabricator.wikimedia.org/T289478 (10RLewis) @Eileenmcnaughton yep I thought it was a bit harsh haha [11:08:28] (03PS1) 10Ejegg: Ignore Authentication IPNs [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714557 [11:08:48] fr-tech looks like ^^^ should stop the failmail [11:09:33] i'm on baby duty so if someone else could deploy that'd be great! [12:41:08] look ejegg|away [12:41:11] looking* [12:42:45] damilare: I'm gonna take a look at this patch ejegg|away has pushed to fix the failmail we're getting so can i ping you afterwards and we can do our check-in then? [12:43:08] probably gonna be after 2pm, with testing and deployment [12:43:55] ah just seen your email. sure we can cancel [12:46:30] Thanks jgleeson [13:13:33] (03CR) 10Jgleeson: [C: 03+2] Ignore Authentication IPNs [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714557 (owner: 10Ejegg) [13:14:06] (03Merged) 10jenkins-bot: Ignore Authentication IPNs [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714557 (owner: 10Ejegg) [13:29:56] (03CR) 10Jgleeson: [C: 03+2] Merge branch 'master' into deployment [wikimedia/fundraising/SmashPig] (deployment) - 10https://gerrit.wikimedia.org/r/714567 (owner: 10Jgleeson) [13:30:32] (03Merged) 10jenkins-bot: Merge branch 'master' into deployment [wikimedia/fundraising/SmashPig] (deployment) - 10https://gerrit.wikimedia.org/r/714567 (owner: 10Jgleeson) [13:51:43] 10Fundraising-Backlog, 10FR-Adyen, 10Epic: Epic: Adyen reintegration, Drop In Web - https://phabricator.wikimedia.org/T277120 (10Jgreen) [14:02:21] thanks jgleeson! [14:04:00] oh hmm, now it's looking for a 'cancel' WSDL class? [14:07:49] well dang, I guess it's not finding that because of the autoloader? Seems to be finding the other classes in that file OK [14:26:34] hmm ejegg [14:26:48] should we be hitting the new adyen api endpoint? [14:26:58] https://7816c1e8e6da0afe-WikimediaFoundation-checkout-live.adyenpayments.com [14:27:06] we havent switched over cancel yet [14:27:18] it's still using wsdl for that [14:27:25] and it's just not able to load the class [14:27:43] i'm looking in the logs, and I don't see anything over the past month that would have even tried to cancel [14:28:09] I wonder the checkout folder is breaking the include path [14:28:12] if* [14:28:50] jgleeson|ish: this is the smashpig job runner, so it's all the SmashPig codebase [14:28:55] not deployed under anything else [14:28:55] ah no nvm that's another project lol [14:28:59] yeah [14:29:12] sorry on the phone I'll shut up [14:47:35] (03PS1) 10Ejegg: Adyen: Use REST for cancelling payments [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714578 [14:47:55] so ^^^ needs testing, will try doing some locally [14:48:03] (03CR) 10jerkins-bot: [V: 04-1] Adyen: Use REST for cancelling payments [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714578 (owner: 10Ejegg) [14:48:44] oh hey, there WERE tests around that - why didn't I find them in my first look around? [14:49:35] I was just running Payment Provider (SmashPig\PaymentProviders\Adyen\Tests\phpunit\PaymentProvider) [14:49:38] ✔ Good cancel payment [15:03:27] Jeff_Green: we need to add a couple more Apple Sandbox accounts which require real email inboxes. Could you add advancementtest3@wikimedia.org and advancementtest4@wikimedia.org as aliases also for fr-tech please? [15:03:50] jgleeson: sure [15:04:10] thanks! [15:06:15] jgleeson: oh, fr-tech@ -- that list is administered by OIT [15:06:28] oh wait I read it backward. I get it now :-P [15:07:52] done, they'll take about 15 min to go live [15:11:07] (03PS2) 10Ejegg: Adyen: Use REST for cancelling payments [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714578 [15:12:07] ok, so the other question is why are we getting so many duplicate payment attempts right now (i.e. what is prompting these cancels) [15:12:15] let's look at the payments logs [15:14:48] so 104176517 seems to have only one pass through the front end [15:14:57] hmm, but there are two auth IPNs [15:15:40] hmm, they have the same pspReference and same reason code [15:15:52] let's see, one seems to have a bit more data than the other [15:17:43] thanks Jeff_Green [15:18:32] ok, looks like there's a second auth with the same pspReference but a different eventDate and an extra entry in the additionalData array [15:19:05] so what does that look like in the console? [15:22:14] so it's settled [15:22:22] because we captured it after the first IPN [15:23:09] and the history section on the console just shows the authorization event that they sent us first, no extra event around the time of the second AUTH IPN [15:26:58] ejegg: I did notice some note in the docs about Adyen changing the notification messages [15:27:09] maybe they're testing stuff? [15:27:48] https://docs.adyen.com/development-resources/webhooks/understand-notifications#event-codes [15:28:02] We may introduce new event types from time to time. When setting up notifications, we recommend that your server does not expect a predefined set of values. [15:28:47] Kinda hard to not handle specific values unless they give us something else concrete to go off? [15:28:55] resultcode maybe [15:29:33] hmm no I don't see anything numeric [15:30:11] eventCode is a string [15:32:30] jgleeson: oh, i think they just mean that we shouldn't fail on unexpected values [15:33:08] so instead of just adding a new empty subclass i should have changed the logic not to throw an exception when we don't find a matching class [15:35:16] I also feel like this area of the code is hard to test locally [15:35:39] the only way I'd had it working previously is setting up ngrok with the ipn listeners hitting my local env [15:35:51] we could do with adding that to the docker setups [15:36:08] ah yeah, we have a little IPN injector script [15:36:22] and that faker.py XML generator [15:37:02] but it doesn't replicate weirdness like we sometimes get on production [15:37:13] just found this https://www.mediawiki.org/wiki/Fundraising_tech/Queue_testing [15:37:24] ah yep [15:37:37] that Adyen section is still up to date for the old integration [15:37:59] oops, almost [15:38:16] that last step refers to the old job runner [15:38:29] AndyRussG: didn't we have a ticket somewhere for ngrok in docker. i feel like you tested that at some point [15:38:57] oh bah, no, there's another reference to Stomp [15:44:11] jgleeson: I think it is set up in docker I was using it lemme see [15:44:35] looks like that doc was updated halfway through the queue switchover [15:50:18] I was messing around with https://localhost:9006/adyen [16:02:12] sorry to come in late to this but why is it hitting the new endpoint? [16:10:56] cstone: it's not yet, they just seem to have deployed some new IPN code [16:11:33] it's adding a whole new category of event: Authentication (not Authorisation) [16:11:40] which was the first flurry of failmails [16:11:50] i guess the failmail I opened had POSTs going to the new endpoint [16:11:59] lemme look at it again [16:12:12] ohh, right, I think we are sending captures to the new one now? [16:12:29] sorry, you're right [16:12:33] I didn't look at that [16:13:02] I was confused by that cstone. the capture process is calling approvePayment which ejegg recently updated to use the rest endpoint [16:13:09] ahh okay so its two different things? [16:13:26] approvePayment is what we called it internally [16:13:28] ah okay i guess I thought that was still behind the switchover flag but its not? [16:13:36] i think to not make it too card-specific [16:13:49] but it might be nicer to rename it capture [16:14:06] cstone: no, the switchover flag is only controling whether or not we send the capture [16:14:10] ah okay [16:14:25] the captures are working against the new endpoint though, right? [16:14:47] yeah, most all of those are working [16:14:54] yeah ejegg. it was only the payments boxes with the network block I think [16:15:03] can't remember exactly why [16:15:25] oh that's it, we'd never made api calls from payments to adyen [16:15:25] jgleeson: they're in a stricter PCI scope according to fr-tech-ops' reading of the rules [16:15:42] because the donors go there to put in raw CC numbers [16:15:46] as we'd previously used the redirect if I remember rightly [16:16:18] so anyway, we seem to be getting these new errors because we're getting double Auth IPNs for the same payment [16:16:47] and for the two I spot-checked, I didn't see anything in the payments-wiki logs to suggest the donor had tried twice [16:17:22] when the donor does try twice, I think we would see different Adyen-side gateway txn ids (pspReference) for the attempts [16:17:44] and the IPN listener logs show these coming in with the same merchant reference and the same pspReference [16:17:55] just the second one a few minutes later has more additionalData [16:18:03] though the extra field is nothing we use [16:19:17] So... our logic assumes that a second auth with the same merchant ref after a successful capture is a mistaken second payment attempt [16:19:23] and tried to cancel the duplicate [16:19:26] *tries [16:19:35] but we're having class loading issues [16:19:44] maybe because WSDL has a hundred classes in the same file [16:20:11] anyway, maybe we shouldn't even be cancelling in this case, as it's not a different attempt, just a duplicate IPN [16:20:43] let's see, can we tell if the one we originally captured with this merchant ref has the same pspReference? [16:24:02] hmm, perhaps [16:24:18] let's see, we'd want to add a new ACTION_ constant to drop those [16:28:32] ejegg: I see the same pspRef across the 2xAUTHORISATION and CAPTURE messages [16:30:34] (03PS1) 10Ejegg: Ignore second Auth IPN with same IDs [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714597 [16:30:40] maybe that will do it? ^^^ [16:31:07] (03CR) 10jerkins-bot: [V: 04-1] Ignore second Auth IPN with same IDs [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714597 (owner: 10Ejegg) [16:31:58] (03PS2) 10Ejegg: Ignore second Auth IPN with same IDs [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714597 [16:33:09] (03CR) 10Ejegg: [C: 04-1] "Let's save this for later - maybe I762876ac860b805279664836 is the better way to deal with the new duplicates we're seeing today" [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714578 (owner: 10Ejegg) [16:41:47] lookin [16:45:39] hmm ejegg on the one I was comparing before the gateway_txn_id is set to false in the $dbmessage [16:46:06] psp_ref-1346298209106352 [16:47:01] so I don't think that one would fall into your ignore block [16:49:46] I'll check the pending tbl to see if that's normal [16:51:39] look standard for adyen [16:51:58] I guess we don't know the gateway_trx_id when we drop it in pending? [16:58:39] looks like we log the pending message for redirects here https://github.com/wikimedia/mediawiki-extensions-DonationInterface/blob/9901e12b56eb93f25eeac893f82ddacd608096b4/gateway_common/donation.api.php#L34 [16:59:17] i had no idea how this stuff worked... [17:01:52] (03CR) 10Jgleeson: [C: 04-1] "Check inline" [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714597 (owner: 10Ejegg) [17:10:32] ejegg: it looks like we have a 'captured':'true' property in $dbMessage['message'] that you could check maybe. that gets set here https://github.com/wikimedia/wikimedia-fundraising-SmashPig/blob/cba1e1a1d2492c54d0b473ba3ffe1c4477e2b139/PaymentProviders/Adyen/Jobs/ProcessCaptureRequestJob.php#L117 [17:11:15] jgleeson|brb: yep, that's what we check already, and when we find it we try to cancel the txn [17:11:30] assuming that it's a second payment attempt [17:11:36] but these are not actually duplicate attempts [17:11:40] just duplicate IPNs [17:12:06] so when we see that captured:true property, we want to ALSO check if it's for the exact same gateway txn ID [17:12:21] and when it's the same txn ID we can just ignore the message and not try to cancel [17:13:20] oh shoot, but you're saying those pending rows don't actually have a pspReference on them [17:13:34] ok, so what we need to do is add that to the pending row when we add the captured:true [17:15:02] (03PS3) 10Ejegg: Ignore second Auth IPN with same IDs [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714597 [17:15:04] OK, PS3 should also set that field ^^^ [17:15:19] gotta run a quick errand [17:38:17] (03CR) 10Jgleeson: [C: 03+2] "Ok this makes sense. Let's give it a whirl!" [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714597 (owner: 10Ejegg) [17:38:49] (03Merged) 10jenkins-bot: Ignore second Auth IPN with same IDs [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/714597 (owner: 10Ejegg) [17:41:37] I'll push that out [17:46:45] (03PS1) 10Jgleeson: Merge branch 'master' into deployment [wikimedia/fundraising/SmashPig] (deployment) - 10https://gerrit.wikimedia.org/r/714612 [17:49:01] jgleeson: ah yes ngrok works fine, just following the instructions you put on the wiki and using Docker URLs instead of vagrant [17:49:27] (03CR) 10Jgleeson: [C: 03+2] Merge branch 'master' into deployment [wikimedia/fundraising/SmashPig] (deployment) - 10https://gerrit.wikimedia.org/r/714612 (owner: 10Jgleeson) [17:50:08] (03Merged) 10jenkins-bot: Merge branch 'master' into deployment [wikimedia/fundraising/SmashPig] (deployment) - 10https://gerrit.wikimedia.org/r/714612 (owner: 10Jgleeson) [17:50:36] ahh ok thanks AndyRussG [17:50:50] :) [17:51:15] looks like adyen added our new shop urls for Apple Pay AndyRussG [17:51:34] jgleeson: ooooh fantastic :) [17:51:39] hopefully we can try stuff now! [17:51:43] yeee [17:51:52] also hi fr-tech :) :) [17:58:01] ok that change is now live ejegg|afk hopefully it'll quiet down the failmail. I guess the fact that the cancel failures prevented the api calls on those worked in our favour. probably wouldn't hurt reaching out to adyen to ask why we're now seeing two auth notifications for the same trxn [17:58:44] gonna grab dinner [18:02:25] fr-tech don't wish to distract from other stuff, just thought I'd mention, if anyone has some feedback here, that'd great! https://phabricator.wikimedia.org/T288830#7302725 [18:10:15] thanks jgleeson|dinner ! [18:21:38] ok, let's make sure that the captured transactions are still getting into Civi fine [19:23:38] oh nice, we got an employer file update notification [19:24:11] oh shoot AndyRussG I meant to take a look at that code this morning. [19:24:14] Looking at it now [19:24:53] 10Fundraising-Backlog, 10Recurring-Donations, 10fr-donorservices: Civi: Auto Recurring Fail email, batch of copy corrections - https://phabricator.wikimedia.org/T289617 (10MBeat33) [19:26:13] hmm, so the only non-alert case is when we have data.result.errors, which I guess are supposed to be validation errors [19:38:42] or when we have a result.isFailed [19:47:53] 10Fundraising Sprint Nitpicking, 10Fundraising Sprint Octopus Untangling, 10Fundraising Sprint Pretending This Isn't Happening, 10Fundraising-Backlog, and 2 others: Documentation: UML activity and sequence diagrams for all processors - https://phabricator.wikimedia.org/T141617 (10DStrine) [21:43:16] sorry about missing the check in this am - I had a bad night's sleep [21:53:15] 10Fundraising-Backlog, 10Wikimedia-Fundraising-CiviCRM, 10fundraising Sprint NULL calorie food cart, 10fundraising sprint Parallel processing roller coaster, and 4 others: Recurring donors segmentation criteria Civi/Acoustic - https://phabricator.wikimedia.org/T283798 (10Eileenmcnaughton) @KHaggard so to c... [22:02:09] great accounts jgleeson :) [22:03:29] tech talk? [22:03:46] :) [22:04:15] heading off shortly eileen over here [22:06:53] AndyRussG: do yo have another email I could use for your Apple Pay test account? I can't send the invite to your wmf email as it seems to be linked to another account. [22:08:39] eileen: earlier today I set up test accounts for Harold & Madge Bishop on our Apple Pay account. Hopefully that means something to you! :) [22:11:25] jgleeson: no - I just looked them up tho [22:13:23] ahh that's a surprise. it was hugely popular here in the 00s [22:13:38] I guessed it made it closer to home first [22:14:16] maybe it just travelled west from Aus [22:15:50] (03PS1) 10Eileen: Remember location settings [wikimedia/fundraising/crm/civicrm] - 10https://gerrit.wikimedia.org/r/714643 (https://phabricator.wikimedia.org/T289100) [22:27:52] (03PS1) 10Eileen: dev/core#2769 use php email validation not hacked qf [wikimedia/fundraising/crm/civicrm] - 10https://gerrit.wikimedia.org/r/714644 (https://phabricator.wikimedia.org/T288644) [22:48:41] (03PS1) 10Eileen: Remove (int) to stop rounding down of below 1.00 amounts [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/714649 (https://phabricator.wikimedia.org/T285396) [22:50:47] (03CR) 10Jgleeson: "I'm also seeing that test fail locally, and agree with Eileen that the test is likely failing due to an exception elsewhere." [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/714445 (https://phabricator.wikimedia.org/T285396) (owner: 10Cstone) [22:54:17] (03CR) 10Ejegg: "curiouser and curiouser, the test passes for me locally! Something locale or timezone dependent, perhaps?" [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/714445 (https://phabricator.wikimedia.org/T285396) (owner: 10Cstone) [23:02:01] ok something is up [23:02:12] (with that patch/tests) [23:02:29] might depend on which other tests are run before it? [23:02:40] https://phabricator.wikimedia.org/P17073 [23:02:50] now it's passing but failing on master [23:02:59] something is wrong with that test [23:03:00] hah [23:04:40] oh eileen maybe because if it's a submodule the .git directory is actually hidden under the parent repo's .git directory now [23:05:20] (03CR) 10Jgleeson: "ok now I think it's something wrong with that particular test as this patch is now passing for me locally but master isn't..." [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/714445 (https://phabricator.wikimedia.org/T285396) (owner: 10Cstone) [23:07:41] (03CR) 10AndyRussG: [C: 03+2] Remember location settings [wikimedia/fundraising/crm/civicrm] - 10https://gerrit.wikimedia.org/r/714643 (https://phabricator.wikimedia.org/T289100) (owner: 10Eileen) [23:17:30] (03PS5) 10Eileen: Stock CiviCRM 5.41 rc [wikimedia/fundraising/crm/civicrm] - 10https://gerrit.wikimedia.org/r/713378 [23:22:17] cstone: yeah - it passed like this https://gerrit.wikimedia.org/r/c/wikimedia/fundraising/crm/+/714649 [23:29:08] hah nice eileen [23:55:44] (03CR) 10Cstone: [C: 03+2] "Woo for no test failures, looks good!" [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/714649 (https://phabricator.wikimedia.org/T285396) (owner: 10Eileen) [23:56:44] (03CR) 10Cstone: "recheck" [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/714445 (https://phabricator.wikimedia.org/T285396) (owner: 10Cstone) [23:56:56] bah i didnt want that recheck haha