[00:21:03] XenoRyet: greg-g btw in case you missed it, the current main blocker we have is as per ejegg's e-mail, about "Invalid credentials" [00:28:48] AndyRussG: ack [00:31:22] :) [00:31:41] theres also now two things broken in the test environment XenoRyet greg-g [00:31:52] oh good [00:31:59] ah yeah that too [00:37:05] cstone: those are the "dlocal todo" in the etherpad? [00:41:15] I think those are something else greg-g [00:41:36] wrt local setup we're just talking new unpexpected errors [00:41:55] also my xdebug fortuitously broke [00:51:58] yeah greg-g just the current state not actively the todo [00:56:06] ok, so let's see about those exceptions [00:57:59] ejegg: we made a phab here https://phabricator.wikimedia.org/T332095 [01:05:22] thanks cstone! [01:05:42] I'm just trying to fix that other one in fillNestedArrayFields [01:06:13] so we did get pix to redirect in tech talk but its sending back apiv1 response ipns?? [01:06:22] oh what [01:06:30] boo [01:07:23] result=9&x_invoice=444498.1&x_iduser=jwales%40example.com&x_description=Wikimedia%20877%20600%209454&x_document=116332186&x_bank=PQ&x_payment_type=02&x_bank_name=PIX%20QR&x_amount=100.00&x_amount_usd=17.54&x_currency=BRL&x_control=EF0513729B7E23F15E8CACD33833BC88F0224E3C79DCBAB100109C9B266A58C9) [01:08:37] actually maybe I clicked on a wrong link [01:09:24] yeah okay ejegg I take that back that was my fault [01:09:57] oh ok [01:10:16] shoot, I can't reproduce that error in the arrayfields now [01:13:05] but I was able to test your patch finally ejegg ! unfortunately wikibugs is still banned but just left you a comment ejegg [01:13:13] oh thanks, looking [01:13:50] good catch! [01:15:36] ok, here's the updated patch: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DonationInterface/+/898834 [01:15:39] cstone: ^^ [01:15:46] thanks! [01:27:30] huh, chat transcript has that error coming from the CO bank transfer form [01:27:46] I can [01:27:53] 't reproduce it locally [01:28:00] but I can make it a bit simpler [01:28:15] oh wait, let's see if that would be fixed by damilare's Api.php refactor [01:45:15] lol - just got two emails from the school [01:45:15] 1) boys are skipping school [01:45:16] 2) erm we didn't mean your boys... [01:54:23] fr-tech Sebastian replied that "An internal team deactivated the account because they were not aware this was testing one and it had no transactions. I already asked to be enabled again. I will confirm once its done." [01:54:52] ah great wfan! [01:55:18] lul [01:55:19] this is related with that rejected_other_reason, so will see if get unblocked tmr [01:55:53] weird that credit cards werent disabled or pix hah [01:56:03] hmmm oki [01:56:10] in other news my xdebug is still not working [01:56:47] I went down a long rabbit hole to uninstall the php xdebug extension for my IDE and re-install it since the extension market site was down [01:56:52] yeah, we should let them know tmr that they might have a bug with deactivate account [01:56:53] finally was able to do that [01:57:02] and then xdebug is still broken [01:57:29] ahh thx so much wfan [01:57:59] ha, np, and my xdebug works after I update to 9003, if you have the same issue [02:00:39] fr-tech with this pix url I'm getting a fail page at the end even though its sucessful [02:00:48] https://localhost:9001/index.php?title=Special:DlocalGateway&appeal=JimmyQuote&payment_method=cash&payment_submethod=pix&recurring=0&uselang=en&language=en¤cy=BRL&amount=46&country=BR&fiscal_number= [02:01:13] 2023-03-15T01:59:42+00:00 a217c9784ab7 dlocal_gateway: 444509:444509.1 Processing donor return with query string values: {"status":"APPROVED","date":"2023-03-15T01:59:41.591Z","signature":"D7E9EEB50200026FFE0332DBE2B9226C700961C617E36DB3DB62681E0D3E94CC","payment_id":"R-413092-9afbdaa8-cba1-4fde-97f7-f0017316c53d","title":"Special:DlocalGatewayResult"} [02:01:13] 2023-03-15T01:59:42+00:00 a217c9784ab7 dlocal_gateway: 444509:444509.1 Displaying fail page for failed PaymentResult [02:10:18] hmmmm [02:11:40] {"code":3003,"message":"Merchant has no authorization to use this API"} [02:11:52] cstone: i do not think we are ready to make any request yet [02:12:10] how did you get the approve? with old credentials? [02:12:30] hah okya i was getting sucessful ipns so i thought it was something on our end [02:12:37] no new credentials just using that link above [02:13:21] the link you provided return the same error due to deactivated account, so probably wait a couple hrs to test again, as Sebastian suggested [02:13:58] huh it just worked for me [02:16:16] it shows up in the new console too hmm [02:18:01] We do have the dLocal call tomorrow, so let's stack up any and all weirdness to ask them about. [02:19:39] cstone: weird, I tried again with the link you provide, get a success return too haha, seems like it's not consistent, and I checked postman, still not working, so let's wait for tmr, it's not a stable env to test now~ haha [02:19:57] yeah i guess not so much the dlocal side just that we are showing a failure page [02:19:59] did you see that? [02:20:06] ahhhhh fixed xdebug [02:20:06] that it didnt go to ty page it went to an error [02:20:09] wooo AndyRussG !! [02:20:21] hahahhahah all I had to do was remove some broken watch expressions [02:20:23] blrrgghghhhhh [02:20:33] that was not a fun rabbit hole [02:20:39] with a terrible punch line too [02:21:44] I do not see that it redirect me to https://paymentstest3.wmcloud.org/index.php?title=Special:DlocalGatewayResult [02:22:11] nice AndyRussG~ [02:22:35] gonna hop off for dinner~ bye~ [02:23:04] cya! [02:23:14] yeah its late im gona go too and may everything just work tomorrow! [02:24:47] cya! [02:43:18] til in php type hints also try to cast values if possible https://stackoverflow.com/a/44724074 [02:44:00] might as well write a language where the "+" operator can double as a closing parenthesis [02:47:32] wfan: https://gerrit.wikimedia.org/r/899068 oops I just saw you'd claimed the task [02:47:39] sorry! [02:48:17] anyway it's a one-liner and I had to do something now that xdebug worked again.... [02:49:14] hmmm also I didn't do anything for the logging part, though in theory the exception thrown should cause that would get our attention on prod? [05:40:28] just got some nice news - my friend with cancer is 99.9% cancer free after chemo (ie the tumour went from 14cm to 0cm & they just did surgery & removed some breast & lymph nodes & only 1 lymph node had any left & even that seemed to be dying) [05:47:18] (03PS1) 10Eileen: CiviCRM 5.61.alpha [not for merge] [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/899187 (https://phabricator.wikimedia.org/T303986) [05:50:19] (03PS1) 10Eileen: [Not for merge] import config code [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/899188 (https://phabricator.wikimedia.org/T303986) [05:53:51] (03CR) 10CI reject: [V: 04-1] CiviCRM 5.61.alpha [not for merge] [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/899187 (https://phabricator.wikimedia.org/T303986) (owner: 10Eileen) [05:57:05] (03CR) 10CI reject: [V: 04-1] [Not for merge] import config code [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/899188 (https://phabricator.wikimedia.org/T303986) (owner: 10Eileen) [06:32:26] (03CR) 10CI reject: [V: 04-1] build: Updating dependencies [extensions/FundraisingEmailUnsubscribe] - 10https://gerrit.wikimedia.org/r/899210 (owner: 10Libraryupgrader) [13:08:26] (03CR) 10Jforrester: [C: 03+2] build: Update MediaWiki requirement to 1.40.0 [extensions/LandingCheck] (REL1_40) - 10https://gerrit.wikimedia.org/r/898328 (owner: 10Jforrester) [13:10:49] (03Merged) 10jenkins-bot: build: Update MediaWiki requirement to 1.40.0 [extensions/LandingCheck] (REL1_40) - 10https://gerrit.wikimedia.org/r/898328 (owner: 10Jforrester) [14:32:58] 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [15:35:04] fr-tech should we test out the dlocal stuff to see if it's still failing or not? [15:35:28] hiii fr-tech [15:35:31] +1 jgleeson :) [15:35:34] I'm gonna ssh on to frlog1002 and see if any of those URLs folks were using last night get through [15:35:43] feel free to add test links to the etherpad also [15:35:45] hey AndyRussG ! [15:35:56] sorry not etherpad, the other thingy [15:36:02] where's that again [15:36:14] the thingypad? [15:36:16] jk [15:36:17] https://docs.google.com/spreadsheets/d/1QbXZVJFdIzAhJgF8idkD7DCKMvdOyU2ON3BWQfenw6A/edit#gid=0 [15:36:30] that one [15:36:30] or the ehterthingy? [15:36:46] ahhh sorry, I'm out of coffee this morning [15:37:03] *etherthingy [15:37:05] ah yes good idea AndyRussG. I could do with one too [15:37:23] yeah I guess we could put links in either for now until we hand it over [15:37:33] I wasted a couple hours last night trying to fix my weireded out xdebug in my ide [15:37:43] only to find that it got fixed by deleting my broken watched expressions [15:38:29] yeah AndyRussG I caught the backscroll on that. it's not great that those things can break it entirely! [15:38:48] I often forget that I've set those for earlier debug runs [15:39:22] I guess it's a bug in the xdebug plugin for vscode. somehow the watch expression was throwing an exception [15:39:40] and every time I tried to step en the debugger, it stepped out of the current function [15:40:08] that said, I think there's some very unexpected behaviour on how Mediawiki is catching exceptions. not sure, will share later [15:40:34] also, did you know that PHP typehints can also serve to cast values to another type? [15:41:38] apparently it's a feature but it should be a bug [15:41:44] huh no I didn't [15:41:52] is it that coercive thing? [15:42:16] https://stackoverflow.com/a/44724074 [15:42:17] regarding dlocal: I'm still seeing Response Error(403) {"code":3001,"message":"Invalid credentials"} [15:42:21] yep [15:42:25] ahhh awww [15:42:48] oh yeah AndyRussG I feel like I've read about that in the past but completely forgotten it did that [15:43:46] I struggle to make a good argument for dynamic typing in PHP AndyRussG [15:43:54] 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [15:44:30] I can see why it might have been helpful when it was mainly used in HTML templates but at this point if feels like we could do with stricter typing and not just type hints [15:44:53] let's just instead design a language where ; means + and + means " [15:45:04] blol [15:45:06] -b [15:45:36] I guess python is also dynamically typed but doesn't seem to get as much flack on that point [15:46:30] ok coffee time [15:49:02] so last night I was pretty opinionated about a change I wanted to see in damilare's Api.php refactor patch ( https://gerrit.wikimedia.org/r/c/wikimedia/fundraising/SmashPig/+/894053 ) but I'd like to know if other people think those opinions make any sense [15:51:20] thanks ejegg, yes your opinions were spot on. There were places that could have been improved upon. Would love to hear what others think as well [15:51:39] coffee in hand [15:51:42] will also take a look [15:55:07] thanks! [15:55:25] that patch is a great improvement already [15:55:44] hmm, we seem not to be linting the dlocal.js [15:55:50] I'll fix that [15:56:39] (03CR) 10Ejegg: "Looking pretty good! I suggest being more lenient with unclassified card brands" [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/897945 (https://phabricator.wikimedia.org/T332091) (owner: 10Wfan) [16:09:48] 10Fundraising Sprint Everything I Merge I Merge For You, 10Fundraising Sprint F 2023, 10Fundraising-Backlog, 10FR-dlocal: Plan dLocal testing rollout - https://phabricator.wikimedia.org/T330801 (10Ejegg) Spreadsheet with some links: https://docs.google.com/spreadsheets/d/1QbXZVJFdIzAhJgF8idkD7DCKMvdOyU2ON3... [16:14:52] 10Fundraising Tech - Chaos Crew, 10Fundraising-Backlog, 10Recurring-Donations, 10fr-donorservices: Ingenico recurrings stopped at status 600 March 5th - https://phabricator.wikimedia.org/T331490 (10Cstone) There are 45 failmails I think are related to this titled FAILMAIL - ALERT: civi1001 (ingenico) Smash... [16:20:51] 10Fundraising Sprint Everything I Merge I Merge For You, 10Fundraising Sprint F 2023, 10Fundraising-Backlog, 10FR-dlocal: Plan dLocal testing rollout - https://phabricator.wikimedia.org/T330801 (10greg) >>! In T330801#8698789, @Ejegg wrote: > Spreadsheet with some links: https://docs.google.com/spreadsheet... [16:21:29] 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [16:22:07] 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [16:23:15] 10fundraising-tech-ops: Renew FR-Tech nessus license - https://phabricator.wikimedia.org/T332097 (10greg) I can't view T246032, can you add me? [16:28:40] 10fundraising-tech-ops: Renew FR-Tech nessus license - https://phabricator.wikimedia.org/T332097 (10Dwisehaupt) @greg Done. Let me know if you can't see it. [16:28:49] 10fundraising-tech-ops: Renew FR-Tech nessus license - https://phabricator.wikimedia.org/T332097 (10greg) Found the current contract in Coupa: https://wikimedia.coupahost.com/contracts/show/4953#summary Dates: 2022-06-24 through 2023-06-23 [16:30:22] 10fundraising-tech-ops: Renew FR-Tech nessus license - https://phabricator.wikimedia.org/T332097 (10greg) @Dwisehaupt is there a reference for the 'now fr-tech pays' decision? Just finding all of my citations :) [16:50:45] (03PS3) 10Wfan: Add credit card brand as payment_submethod from dlocal smartfield [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/897945 (https://phabricator.wikimedia.org/T332091) [16:52:23] (03CR) 10Wfan: Add credit card brand as payment_submethod from dlocal smartfield (034 comments) [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/897945 (https://phabricator.wikimedia.org/T332091) (owner: 10Wfan) [16:52:26] jgleeson: is it ok if I push some of the changes ejegg recommended and I implemented, so your review would be based on that as well [16:52:58] 10fundraising-tech-ops: Renew FR-Tech nessus license - https://phabricator.wikimedia.org/T332097 (10Dwisehaupt) @greg I'm going off of memory. There is an email thread on 2022-02-17 titled "Nessus software license for frack expiring and in need of renewal" but the conversation went away from that thread. Perhap... [16:53:46] sure damilare [16:53:54] tnx [16:53:58] ty! [16:54:27] (03PS11) 10Damilare Adedoyin: Refactor Dlocal API class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [16:55:02] (03CR) 10CI reject: [V: 04-1] Refactor Dlocal API class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [17:01:59] (03PS12) 10Damilare Adedoyin: Refactor Dlocal API class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [17:02:26] (03CR) 10CI reject: [V: 04-1] Refactor Dlocal API class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [17:09:34] 10Fundraising-Backlog, 10fundraising-tech-ops, 10FR-dlocal: frlog syslog filter for payments-dlocal - https://phabricator.wikimedia.org/T332092 (10Ejegg) Thanks! So I think the payments-listener-smashpig log lines are all from frpig. I was talking about the log lines from payments* which are initiated by cod... [17:21:39] (03CR) 10Wfan: [C: 03+2] Add mapping for 100 other error [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/899068 (https://phabricator.wikimedia.org/T332095) (owner: 10AndyRussG) [17:22:42] (03Merged) 10jenkins-bot: Add mapping for 100 other error [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/899068 (https://phabricator.wikimedia.org/T332095) (owner: 10AndyRussG) [17:25:40] 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [17:26:58] 10Fundraising-Backlog, 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) added fundraising-backlog for visibility [17:37:32] 10Fundraising-Backlog, 10fundraising-tech-ops, 10FR-dlocal: frlog syslog filter for payments-dlocal - https://phabricator.wikimedia.org/T332092 (10Dwisehaupt) Ah, thanks for clarifying. I'll have a look and will break those lines out of the payments-misc logs. [17:38:58] 10Fundraising Tech - Chaos Crew, 10Fundraising-Backlog, 10MediaWiki-extensions-CentralNotice, 10MW-1.40-notes (1.40.0-wmf.27; 2023-03-13): CentralNotice banners being shown too many times - https://phabricator.wikimedia.org/T331671 (10Pcoombe) It's also worth noting that localstorage is per site. So if a u... [17:42:48] 10Fundraising Tech - Chaos Crew, 10Fundraising-Backlog, 10MediaWiki-extensions-CentralNotice, 10MW-1.40-notes (1.40.0-wmf.27; 2023-03-13): CentralNotice banners being shown too many times - https://phabricator.wikimedia.org/T331671 (10DerHexer) @Pcoombe Yes, I sadly know. And in my opinion, I consider this... [17:42:52] 10Fundraising-Backlog, 10fundraising-tech-ops, 10FR-dlocal: frlog syslog filter for payments-dlocal - https://phabricator.wikimedia.org/T332092 (10Dwisehaupt) This has been done. ` [frack::puppet] 64620f57 Break out payments "SmashPig: dlocal:" logs to payments-dlocal (Dallas Wisehaupt:Dallas Wisehaupt) ` [17:44:58] 10Fundraising Tech - Chaos Crew, 10Fundraising-Backlog, 10MediaWiki-extensions-CentralNotice, 10MW-1.40-notes (1.40.0-wmf.27; 2023-03-13): CentralNotice banners being shown too many times - https://phabricator.wikimedia.org/T331671 (10AndyRussG) >>! In T331671#8699130, @DerHexer wrote: > @Pcoombe Yes, I sa... [18:15:34] (03PS13) 10Damilare Adedoyin: Refactor Dlocal API class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [18:16:18] (03CR) 10CI reject: [V: 04-1] Refactor Dlocal API class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [18:18:58] (03PS14) 10Damilare Adedoyin: Refactor Dlocal API class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [18:19:53] (03CR) 10CI reject: [V: 04-1] Refactor Dlocal API class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [18:24:52] 10Fundraising-Backlog, 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [18:26:03] 10Fundraising-Backlog, 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [18:30:07] 10fundraising-tech-ops: Renew FR-Tech nessus license - https://phabricator.wikimedia.org/T332097 (10greg) In an email conversation with Jennifer Cross about this. [18:30:24] 10fundraising-tech-ops, 10User-greg: Renew FR-Tech nessus license - https://phabricator.wikimedia.org/T332097 (10greg) a:03greg [18:36:25] (03CR) 10Jforrester: [C: 03+2] build: Update MediaWiki requirement to 1.40.0 [extensions/FundraiserLandingPage] (REL1_40) - 10https://gerrit.wikimedia.org/r/898301 (owner: 10Jforrester) [18:41:52] (03Merged) 10jenkins-bot: build: Update MediaWiki requirement to 1.40.0 [extensions/FundraiserLandingPage] (REL1_40) - 10https://gerrit.wikimedia.org/r/898301 (owner: 10Jforrester) [18:49:13] (03PS15) 10Damilare Adedoyin: Refactor Dlocal API class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [18:50:00] (03CR) 10Damilare Adedoyin: "Thanks ejegg for the detailed review, I've made some changes inline with your comments." [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [18:51:34] rockin damilare, i'll take a look shortly [18:52:49] tnx! [18:53:36] (03PS1) 10Ejegg: Dlocal: ensure new order IDs for each doPayment [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/899730 [18:53:51] jgleeson|dinner: ok, ^^^ should get us past all those duplicate order IDs [19:07:16] (03PS4) 10Ejegg: Add credit card brand as payment_submethod from dlocal smartfield [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/897945 (https://phabricator.wikimedia.org/T332091) (owner: 10Wfan) [19:07:21] (03CR) 10Ejegg: [C: 03+2] Add credit card brand as payment_submethod from dlocal smartfield [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/897945 (https://phabricator.wikimedia.org/T332091) (owner: 10Wfan) [19:20:29] (03Merged) 10jenkins-bot: Add credit card brand as payment_submethod from dlocal smartfield [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/897945 (https://phabricator.wikimedia.org/T332091) (owner: 10Wfan) [19:22:29] (03PS1) 10Ejegg: Update drupal to 7.95 [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/899734 [19:23:27] (03PS1) 10Ejegg: Re-apply WMF patches [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/899735 [19:27:30] (03PS1) 10Ejegg: Log rawResponse from dlocal before checking errors [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/899737 [19:33:21] 10Fundraising-Backlog, 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [19:33:54] 10Fundraising-Backlog, 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [19:34:58] (03CR) 10CI reject: [V: 04-1] Update drupal to 7.95 [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/899734 (owner: 10Ejegg) [19:35:03] 10Fundraising-Backlog, 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [19:37:35] 10Fundraising-Backlog, 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [19:39:31] 10Fundraising-Backlog, 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [19:40:59] 10Fundraising-Backlog, 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [19:43:53] 10Fundraising-Backlog, 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [19:48:26] 10Fundraising-Backlog, 10fundraising-tech-ops: Switch-over from civi1001 to civi1002 - https://phabricator.wikimedia.org/T329882 (10Jgreen) [19:58:32] (03PS1) 10Ejegg: Fix isSuccessStatus call [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/899770 [20:00:32] (03PS2) 10Ejegg: Fix isSuccessStatus call [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/899770 [20:01:57] (03CR) 10Cstone: [C: 03+2] "Looks good! Incrementing on back button" [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/899730 (owner: 10Ejegg) [20:05:16] (03CR) 10Ejegg: [C: 03+1] "This looks really good! I think you might be able to remove a few custom parameters from the input mappings." [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [20:07:35] (03CR) 10Ejegg: [C: 03+2] Re-apply WMF patches [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/899735 (owner: 10Ejegg) [20:07:46] (03CR) 10Ejegg: [V: 03+2 C: 03+2] Update drupal to 7.95 [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/899734 (owner: 10Ejegg) [20:09:42] (03CR) 10Damilare Adedoyin: [C: 03+2] "Thanks for this." [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/899770 (owner: 10Ejegg) [20:10:16] (03Merged) 10jenkins-bot: Fix isSuccessStatus call [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/899770 (owner: 10Ejegg) [20:11:31] damilare: thanks for fixing that. you were right to update it to read the normalizedStatus [20:11:42] (03Merged) 10jenkins-bot: Dlocal: ensure new order IDs for each doPayment [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/899730 (owner: 10Ejegg) [20:12:22] thanks jgleeson, sorry I missed it in the refactor [21:05:48] (03PS1) 10Umherirrender: Avoid passing null to str_replace [extensions/FundraisingEmailUnsubscribe] - 10https://gerrit.wikimedia.org/r/899787 [21:08:05] (03CR) 10CI reject: [V: 04-1] Avoid passing null to str_replace [extensions/FundraisingEmailUnsubscribe] - 10https://gerrit.wikimedia.org/r/899787 (owner: 10Umherirrender) [21:09:45] (03CR) 10Umherirrender: "check php" [extensions/FundraisingEmailUnsubscribe] - 10https://gerrit.wikimedia.org/r/899787 (owner: 10Umherirrender) [21:25:01] (03PS16) 10Damilare Adedoyin: Refactor Dlocal API class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [21:25:50] (03CR) 10Damilare Adedoyin: "Thanks ejegg, done!" [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [21:27:24] (03PS17) 10Jgleeson: Refactor Dlocal API class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) [21:30:22] damilare: I can see a few instances of methods getting the word "card" added to them in that patch [21:30:40] did you do that as part of the refactor to the api request stuff [21:31:27] you mean like this one: makeRecurringCardPayment [21:31:39] cardAuthorizePayment [21:31:44] authorizePayment [21:31:50] vs * [21:31:57] yh I did that to add some differentiation between the card vs bank authorize [21:32:12] would it be too much trouble to split that out and explain why in a commit [21:32:23] just so the refactoring stuff can be viewed alone [21:32:44] 10Wikimedia-Fundraising-Banners, 10Wikipedia-Android-App-Backlog: Japan fundraising messages in iOS app - https://phabricator.wikimedia.org/T332229 (10Pcoombe) [21:32:51] 10Wikimedia-Fundraising-Banners, 10Wikipedia-iOS-App-Backlog: Japan fundraising messages in Android app - https://phabricator.wikimedia.org/T332230 (10Pcoombe) [21:34:24] ohh they kind of go hand in hand, might be a bit difficult to separate [21:37:23] I guess one thing that I think is worth discussing is whether the API class should know about the mapping [21:38:27] when we were originally discussing it I felt like we were heading in a direction where the API class would receive the prepared request allowing us to take all of the mapping out into its own distinct layer [21:40:47] in the latest code, the mapping is an implementation detail of the API class [21:42:11] sorry this is just thoughts at first glance. I haven't gone through it all yet [21:43:24] No problem at all, yh the intermediary class between the provider and the api was what I had in mind originally as well. [21:43:53] (03CR) 10Ejegg: [C: 03+2] "Thanks, this looks good! It's especially nice to have each mapping individually testable." [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [21:43:57] for context, this is the test for PS1 where the mapper produces the output distinctly https://gerrit.wikimedia.org/r/c/wikimedia/fundraising/SmashPig/+/894053/1/PaymentProviders/dlocal/Tests/phpunit/ApiRequestMapperTest.php [21:44:23] (03PS2) 10Ejegg: Log rawResponse from dlocal before checking errors [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/899737 [21:44:27] (03Merged) 10jenkins-bot: Refactor Dlocal API class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/894053 (https://phabricator.wikimedia.org/T330425) (owner: 10Jgleeson) [21:44:30] hmm ejegg I think I might have some blocking change requests on that refactor [21:44:40] just in relation to how the layers are split out [21:45:01] (03PS2) 10Ejegg: Remove watchdogs from templating [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/897976 (https://phabricator.wikimedia.org/T288585) (owner: 10Eileen) [21:45:03] I just mentioned it briefly in chat [21:45:05] (03CR) 10Ejegg: [C: 03+2] Remove watchdogs from templating [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/897976 (https://phabricator.wikimedia.org/T288585) (owner: 10Eileen) [21:46:04] oh crap it merged [21:46:24] no worries, I can always undo those jgleeson [21:46:28] please go ahead [21:46:36] in a follow on patch I mean [21:47:08] (03PS2) 10Ejegg: Remove some more watchdogs [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/897977 (https://phabricator.wikimedia.org/T288585) (owner: 10Eileen) [21:47:18] 10Fundraising-Backlog, 10FR-AutoTY-Email, 10fr-donorservices: EoY receipt email donation format issue 2023 - https://phabricator.wikimedia.org/T332232 (10MBeat33) [21:47:39] (03CR) 10Ejegg: [C: 03+2] "Logging update looks good, but I wonder whether we should be catching those exceptions at all" [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/897977 (https://phabricator.wikimedia.org/T288585) (owner: 10Eileen) [21:47:48] I tried to -1 just to pause it. it might make sense after a full review but I felt like I wanted to understand that a bi tmore [21:48:45] oh shoot jgleeson i just saw your ping [21:48:55] sorry, didn't realize you were still looking at that one [21:48:57] so one thing I know we need to do is we need to make makePaymentApiCall protected [21:49:10] that doesn't be public anymore. I think we had a dreaded TODO on that one [21:49:18] sounds good - want to make a followup patch? [21:49:30] and currently that method is receiving a mapper instance [21:50:17] so I'm wondering if the API internals should know about the request mapper [21:50:34] I just added the makePaymentApiCall in this patch, did you mean the makeApiCall? [21:50:59] oh really! [21:51:13] ah sorry yes. I mixed those two up [21:51:32] jgleeson: so maybe it should be the PaymentProvider class calling the mapper and passing the already-mapped params down to Api.php? [21:51:42] I think we do it that way in at least one other provider [21:52:01] one thing we could do is move the makePaymentApiCall to the mapper, but it'd have to be called something else. Then the provider could call that [21:52:05] or what ejegg said [21:52:15] ejegg: I'm thinking maybe that might be a way to simplify the API class [21:52:31] I mean the Mapper would have to be called something else. [21:52:40] but I don't wanna make kneejerk suggestions [21:53:33] I guess a question to ask is, what do we want our payment provider classes to be? [21:53:44] are they our orchestration layer? [21:56:40] I guess it's our service layer [21:56:46] as that's what the client code sees [21:58:39] I always immagined it as the Orchestration layer, for both requests to the API and response to the Client [21:58:53] (03Merged) 10jenkins-bot: Remove watchdogs from templating [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/897976 (https://phabricator.wikimedia.org/T288585) (owner: 10Eileen) [21:58:58] sure, orchestration layer sounds about right [22:00:13] ok so if we agree that the payment providers are the service/orchestration layer then the next question I'd ask is what is our API class. the last time I tried to find something that resembles it, I found this https://martinfowler.com/articles/gateway-pattern.html [22:01:50] (03Merged) 10jenkins-bot: Remove some more watchdogs [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/897977 (https://phabricator.wikimedia.org/T288585) (owner: 10Eileen) [22:03:49] granted they might not be anything of the above [22:05:05] can we say it's similar to the Data Access Layer, maybe? [22:05:13] So i'm not sure what you call it, but I've just imagined the API class as knowing how to set headers, params and URLs, and format data for the post [22:05:33] yeah, Data Access Layer seems right [22:05:42] damilare: I feel DAL usually wraps storage [22:05:46] e.g. a database [22:05:58] ah hmm [22:05:59] https://en.wikipedia.org/wiki/Data_access_layer [22:06:22] right, actions like capture aren't data access [22:07:24] hmm [22:09:53] the more I think about it the more I'm struggling to make a case for putting the request transformation in the payment provider class. [22:10:09] it probably should know anything about mapping client params to a remote API [22:10:30] and we don't have any other layers so it probably does need to live in the API class [22:10:51] yeah, talking about it like this makes it sound like the SmashPig stacks should be three layers deep [22:11:08] in my head I always had the API classes down as really simple wrappers for the remote API [22:11:31] but if the transformation of client params to remote API params is complicated, something has to do that right [22:11:57] ejegg maybe but I'm trying to think of what we call that intermediate layer [22:12:00] PaymentProvider to be the facade, mapper to transform params back and forth, and Api for the wrapper [22:12:08] oh i don't know if we call it mapper [22:12:18] but somethign like 'adapter' [22:12:41] it might be overkill for simple calls like 'cancel' which just pass a single payment ID [22:12:49] and just return success or failure [22:13:04] earlier I meant to write, it probably shouldn't*** know [22:13:10] sry [22:15:30] ejegg: adapter sounds right to me [22:16:45] yeah, it's always nagged me that we map from ours -> theirs in one layer and theirs -> ours in another. Felt like we should do both directions in one place [22:16:49] yh looks like exactly what we need: https://en.wikipedia.org/wiki/Adapter_pattern [22:17:06] although then the PaymentProvider class becomes a pretty thin facade [22:17:20] if we had an adapter layer we could effectively decouple the client code calls from the API class and then hypothetically swap our the API to APIv3 in the future [22:17:21] I guess it could do more validation than it does now [22:17:37] without touching the provider classes [22:18:17] yep yep.... If we do want to try the 3 layer cake, let's take a quick look at what that would mean for other providers too [22:19:52] jgleeson: for right now, do you want to put up a small patch with at least that public -> protected change and any other small changes you were going to suggest on damilare's ? [22:23:25] yeah sure I can add a small patch for the visibility swap and get rid of that related TODO in the tests. just for clarity that isn't something damilare added, I got the method names mixed up, but it does still need doing so I'll do that first thing tomorrow. The TODO for that is here [22:23:27] https://github.com/wikimedia/wikimedia-fundraising-SmashPig/blob/master/PaymentProviders/dlocal/Tests/phpunit/ApiTest.php#L37 [22:25:17] ejegg: damilare I guess a good way forward would be to let the existing +2ed patch merge but keep the phab task open and add our notes about how to evolve it further by introducing an adapter layer [22:27:52] wow [22:28:12] I just asked chatGPT "show me an example of an adapter pattern between a service layer and gateway layer in php" [22:28:22] and it pretty much nailed it [22:28:26] lemme put this somewhere [22:32:09] ok I'll add a comment to the phab [22:32:16] with this example. i think it will help [22:32:23] I'll do that now [22:33:51] ejegg: to your point about whether we should be catching data formatting - the bigger question is probably if we should be formatting dates at all - we have to be doing 'something more' than what `strtotime` does or we should let the api strtotime it - it looks like we do some timezone handling but I kinda wonder why. [22:33:51] Anyway - the risk with the watchdog swaps is there are a whole lotta rabbit holes [22:35:48] oh yeah eileen. I guess we don't want to risk sending an ambiguous format like 3/4/85 [22:35:57] jgleeson: I could work on a follow on patch to add the layer tomorrow. I think it'd be good to have the task closed in this sprint [22:36:03] adapter** [22:36:05] but that whole 'catch and return empty string' seems pretty dubious [22:36:41] eileen: I was trying to play out what it would mean to store a TZ for each donor [22:37:04] we do get some donor relations tickets involving UTC confusion [22:37:10] or hawaii time confusion [22:37:40] anyway i think the TZ is only stored for users with logins now [22:38:00] so that seemed like a dead-end, unless we added a new communication field [22:38:55] it would be dead simple to get it from their browsers in javascript and just send it back from paymentswiki [22:40:38] (03CR) 10Ejegg: [C: 03+1] "This looks great! I might just add something after the loop to log a warning like "Please set a default rule in donation_rules.yaml", in c" [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/895731 (https://phabricator.wikimedia.org/T324299) (owner: 10Wfan) [22:41:02] 10Fundraising Sprint Everything I Merge I Merge For You, 10Fundraising Sprint Fish HEAD^, 10Fundraising-Backlog: Refactor Dlocal API class - https://phabricator.wikimedia.org/T330425 (10jgleeson) After our discussion about introducing an Adapter layer to Smashpig, I asked Skynet for an example, and it produc... [22:41:08] ejegg: damilare|away https://phabricator.wikimedia.org/T330425#8700364 [22:41:36] cstone ok, the parent patch is merged and this debug logging of rawResponse is ready for review: https://gerrit.wikimedia.org/r/c/wikimedia/fundraising/SmashPig/+/899737 [22:41:43] sure damilare|away although we might be extending the scope of the original task but I feel like it's worth doing [22:42:04] it might make sense to create a new or sub task [22:42:27] jgleeson: I'd say a new task makes sense for adding a whole new layer [22:42:39] yeah agreed [22:42:42] we could use that to discuss implications for other classes [22:46:58] I'll create a new task and link it to damilare|away's refactoring patch. I'll tag it under our refactoring epic [22:55:15] 10Fundraising-Backlog, 10fundraising-tech-ops, 10FR-dlocal: frlog syslog filter for payments-dlocal - https://phabricator.wikimedia.org/T332092 (10Dwisehaupt) [22:59:41] (03CR) 10Cstone: [C: 03+2] "Thanks for this, really helpful to have!" [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/899737 (owner: 10Ejegg) [23:00:16] (03Merged) 10jenkins-bot: Log rawResponse from dlocal before checking errors [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/899737 (owner: 10Ejegg) [23:02:33] 10Fundraising-Backlog, 10fundraising-tech-ops, 10FR-dlocal: frlog syslog filter for payments-dlocal - https://phabricator.wikimedia.org/T332092 (10Dwisehaupt) [23:03:12] huh, my git log in SmashPig doesn't show the 0.8.4.1 tag [23:03:15] did I do that? [23:03:15] 10Fundraising-Backlog, 10fundraising-tech-ops, 10FR-dlocal: frlog syslog filter for payments-dlocal - https://phabricator.wikimedia.org/T332092 (10Dwisehaupt) 05Open→03Resolved Verified working on frlog1002. [23:04:28] ok, that was b72a832768608b5feb77abd07c968c51142f254f [23:05:02] ooh, that's not on the master branch [23:05:03] oops [23:05:20] ok, I'm going to tag another SmashPig version to get that extra logging and fix the Pix success status [23:06:22] 10Fundraising-Backlog: SmashPig Adapter Layer - https://phabricator.wikimedia.org/T332236 (10jgleeson) [23:08:41] (03PS11) 10Wfan: Add recurring amount validation to DI level for dlocal IN bt recurring [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/895731 (https://phabricator.wikimedia.org/T324299) [23:09:19] 10Fundraising Sprint Fish HEAD^, 10Fundraising-Backlog, 10FR-dlocal: DLocal Audit - Test with new integration - https://phabricator.wikimedia.org/T324347 (10Cstone) Sebastian said today that he will try to get an audit file for us from the transactions on the test account. [23:09:55] (03PS12) 10Wfan: Add recurring amount validation to DI level for dlocal IN bt recurring [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/895731 (https://phabricator.wikimedia.org/T324299) [23:10:27] 10Fundraising-Backlog: SmashPig Adapter Layer - https://phabricator.wikimedia.org/T332236 (10jgleeson) [23:10:29] 10Fundraising-Backlog, 10MediaWiki-extensions-DonationInterface, 10Epic: [Epic] Architecture, separation of concerns and refactors in DonationInterface and SmashPig (2021) - https://phabricator.wikimedia.org/T291697 (10jgleeson) [23:11:37] (03CR) 10CI reject: [V: 04-1] Add recurring amount validation to DI level for dlocal IN bt recurring [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/895731 (https://phabricator.wikimedia.org/T324299) (owner: 10Wfan) [23:12:45] 10Fundraising-Backlog, 10fundraising-tech-ops, 10FR-dlocal: frlog syslog filter for payments-dlocal - https://phabricator.wikimedia.org/T332092 (10jgleeson) [23:13:37] 10Fundraising-Backlog, 10fundraising-tech-ops, 10FR-dlocal: frlog syslog filter for payments-dlocal - https://phabricator.wikimedia.org/T332092 (10jgleeson) [23:16:48] (03PS13) 10Wfan: Add recurring amount validation to DI level for dlocal IN bt recurring [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/895731 (https://phabricator.wikimedia.org/T324299) [23:17:34] (03PS1) 10Ejegg: Start linting dlocal.js [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/899844 [23:20:07] (03CR) 10Ejegg: "Oops, we also need to update the default ruleset shipped with DonationInterface, at config/donation_rules.yaml" [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/895731 (https://phabricator.wikimedia.org/T324299) (owner: 10Wfan) [23:23:05] (03CR) 10Wfan: [C: 03+2] "LGTM" [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/899844 (owner: 10Ejegg) [23:23:43] thanks wfan! [23:23:56] (03PS14) 10Wfan: Add recurring amount validation to DI level for dlocal IN bt recurring [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/895731 (https://phabricator.wikimedia.org/T324299) [23:24:06] I realise we aren't linting braintree.js either. gonna make a totally-low-priority patch for that too [23:24:35] Ah, I did not realized that we have config/donation_rules.yaml exist, just updated, thanks for your review :) [23:24:40] 10Fundraising-Backlog: SmashPig Adapter Layer - https://phabricator.wikimedia.org/T332236 (10jgleeson) [23:24:51] (03Merged) 10jenkins-bot: Start linting dlocal.js [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/899844 (owner: 10Ejegg) [23:25:12] 10Fundraising-Backlog: SmashPig Adapter Layer - https://phabricator.wikimedia.org/T332236 (10jgleeson) [23:28:31] heh, yep, i totally forgot it existed till just now too wfan [23:35:28] 10fundraising-tech-ops: Deactivate fundraising accounts for dmorgan - https://phabricator.wikimedia.org/T332240 (10Dwisehaupt) [23:43:29] 10fundraising-tech-ops: Deactivate fundraising accounts for dmorgan - https://phabricator.wikimedia.org/T332240 (10Dwisehaupt) [23:48:26] (03PS1) 10Ejegg: Lint braintree.js, make it pass [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/899871 [23:53:29] 10fundraising-tech-ops: Deactivate fundraising accounts for dmorgan - https://phabricator.wikimedia.org/T332240 (10Dwisehaupt) [23:55:07] 10fundraising-tech-ops: Deactivate fundraising accounts for dmorgan - https://phabricator.wikimedia.org/T332240 (10Dwisehaupt) Client ssl certificate revoked. Civi account moved to blocked. Verified all other offboarding elements did not apply. [23:55:41] 10fundraising-tech-ops: Deactivate fundraising accounts for dmorgan - https://phabricator.wikimedia.org/T332240 (10Dwisehaupt) [23:56:16] 10fundraising-tech-ops: Deactivate fundraising accounts for dmorgan - https://phabricator.wikimedia.org/T332240 (10Dwisehaupt) 05Open→03Resolved a:03Dwisehaupt [23:58:50] (03PS1) 10Wfan: Remove mapping for 100 use fall back to other error [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/899882 (https://phabricator.wikimedia.org/T332095)