[03:56:07] (03CR) 10AndyRussG: "Heyyy thanks so much for this! Smoke tested, works great! See inline for some nitpicks...... ;p" [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/800033 (https://phabricator.wikimedia.org/T308167) (owner: 10Ejegg) [05:45:28] (03CR) 10AndyRussG: [V: 03+2 C: 03+2] "Nice, thanks for this!!! :)" [wikimedia/fundraising/dev] - 10https://gerrit.wikimedia.org/r/808251 (owner: 10Damilare Adedoyin) [06:27:39] (03PS1) 10AndyRussG: Test pass-through of additional params for GatewayChoosre [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/808790 (https://phabricator.wikimedia.org/T310895) [07:33:23] Anyone have a minute to chat about banner impression analytics? [07:33:56] I'm trying to help WMDE with T298777 but my understanding of fr-tech impressions logging is many years out-of-date. [07:33:57] T298777: Prevent gaps when providing banner impression data of WMDE fundraising banners - https://phabricator.wikimedia.org/T298777 [13:05:27] (03PS12) 10Jgleeson: Tests: Front-end calls to charge one-time donation via Braintree [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/805906 (https://phabricator.wikimedia.org/T303419) [14:43:50] 10Fundraising Sprint Incantation optimisation, 10Fundraising Sprint Juggalology 2022, 10Fundraising Sprint Kermit hopping principle, 10Fundraising Sprint Localhost Hospitality Studies, and 2 others: Testmeister esMX email is different from what we have set up in th... - https://phabricator.wikimedia.org/T307704 [15:53:45] 10Fundraising-Backlog: Add donor details to createPaymentResponse - https://phabricator.wikimedia.org/T311428 (10Damilare) [15:58:36] (03PS1) 10Damilare Adedoyin: Add donor detail to Braintree createPayment response object [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808933 (https://phabricator.wikimedia.org/T311428) [15:59:16] (03CR) 10CI reject: [V: 04-1] Add donor detail to Braintree createPayment response object [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808933 (https://phabricator.wikimedia.org/T311428) (owner: 10Damilare Adedoyin) [16:02:31] (03PS2) 10Damilare Adedoyin: Add donor detail to Braintree createPayment response object [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808933 (https://phabricator.wikimedia.org/T311428) [16:02:57] (03CR) 10CI reject: [V: 04-1] Add donor detail to Braintree createPayment response object [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808933 (https://phabricator.wikimedia.org/T311428) (owner: 10Damilare Adedoyin) [16:04:31] (03PS3) 10Damilare Adedoyin: Add donor detail to Braintree createPayment response object [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808933 (https://phabricator.wikimedia.org/T311428) [16:08:02] (03PS20) 10Damilare Adedoyin: Front-end calls to charge one-time donation via Braintree [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/801734 (https://phabricator.wikimedia.org/T303419) [16:08:21] (03CR) 10Damilare Adedoyin: Front-end calls to charge one-time donation via Braintree (031 comment) [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/801734 (https://phabricator.wikimedia.org/T303419) (owner: 10Damilare Adedoyin) [16:18:09] 10Fundraising-Analysis, 10Fundraising-Backlog, 10WMDE-FUN-Team, 10WMDE-Fundraising-Tech: Prevent gaps when providing banner impression data of WMDE fundraising banners - https://phabricator.wikimedia.org/T298777 (10AndyRussG) [16:30:14] (03CR) 10Wfan: [C: 03+2] "Tested and looks all good, thanks~" [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808933 (https://phabricator.wikimedia.org/T311428) (owner: 10Damilare Adedoyin) [16:30:43] (03Merged) 10jenkins-bot: Add donor detail to Braintree createPayment response object [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808933 (https://phabricator.wikimedia.org/T311428) (owner: 10Damilare Adedoyin) [17:02:40] (03CR) 10Jgleeson: "Thanks for this Dami! Just a few comments and suggested updates inline our talk earlier. I can see it's been +2ed so if you agree with the" [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808933 (https://phabricator.wikimedia.org/T311428) (owner: 10Damilare Adedoyin) [17:06:48] hmm I wonder how CI is gonna play with a new patch on smashpig [17:07:29] damilare: I think we're gonna have to cut another new smashpig tag to get the DI tests passing which check the new payer fields on the frontend [17:09:30] Yes that’s right jgleeson [18:00:02] (03CR) 10Ejegg: Add donor detail to Braintree createPayment response object (031 comment) [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808933 (https://phabricator.wikimedia.org/T311428) (owner: 10Damilare Adedoyin) [18:04:33] (03CR) 10AndyRussG: [C: 04-1] "Heyy... quick update... I got it working with a "+" in the e-mail!! Patch coming up in a few..." [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/800033 (https://phabricator.wikimedia.org/T308167) (owner: 10Ejegg) [18:05:17] hiii fr-tech damilare jgleeson :) [18:06:34] ejegg ^ quick note, before you spend time digging into any changes in the dlocal "+" fix, gimme a sec to put together a patch that does allow "+"'s in e-mails [18:06:44] thx and apologies for disruption [18:08:50] oh interesting [18:10:30] howdy AndyRussG ejegg fr-tech [18:11:40] :) [18:12:48] ejegg: getting rid of this str_replace did it, at least for e-mail addresses [18:13:00] just gonna see if it also does the trick for street address etc [18:13:17] and if the e-mail address munger for donor_id can stay (dunno if it should) [18:13:19] https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/DonationInterface/+/70086a19b88c3a0b6a7f151d2c73bfbfd76424e3/astropay_gateway/AstroPaySignature.php#27 [18:14:01] (03CR) 10Jgleeson: Add donor detail to Braintree createPayment response object (031 comment) [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808933 (https://phabricator.wikimedia.org/T311428) (owner: 10Damilare Adedoyin) [18:15:50] AndyRussG: ahh shoot, i think that was explicitly in their instructions for calculating the signature. huh [18:16:56] huh funky [18:17:13] https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DonationInterface/+/293465 is the original patch that added it [18:17:27] looking for their doc now [18:19:06] damilare: I think that thing you discovered about php 7.3 return types not allowing child classes of the type might break this rule https://en.wikipedia.org/wiki/Liskov_substitution_principle [18:20:47] I'm pretty sure arg type declarations allows child classes of the type define [18:20:49] d [18:21:18] ejegg: yeah removing that str_replace also lets "+"'s through in street address [18:26:39] fr-tech anyone know what is the appropriate documentation for our dlocal integration? [18:26:56] checking AndyRussG [18:27:14] https://dlocal.com/developers/documentation.php?sec=streamline ? is on our wiki page for it [18:27:41] AndyRussG: https://documentation.dlocal.com/docs [18:27:58] yep that's it cstone [18:28:31] yep the L in SOLID jgleeson [18:28:42] yeah sure is [18:30:19] damilare: what happened to the tests btw? https://gerrit.wikimedia.org/r/c/wikimedia/fundraising/SmashPig/+/800779/3 [18:30:38] oh these are the wrong tests [18:30:47] we rebased them ones [18:30:58] (03PS13) 10Jgleeson: Tests: Front-end calls to charge one-time donation via Braintree [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/805906 (https://phabricator.wikimedia.org/T303419) [18:31:02] yup we rebased them into the main patch [18:31:05] there's the other ones [18:31:19] ahh right thx cstone fr-tech [18:31:33] I think we said we'd +2 the implementation, then the test afterwards. [18:31:47] thx also jgleeson :) :) [18:32:10] AndyRussG: those two docs looks different not sure if the one I posted is for a newer integration [18:33:12] AndyRussG: hmm, ok, I guess I don't see the + sign removal there [18:33:23] I wonder where we got that from [18:34:38] yep damilare ! [18:35:04] (03CR) 10CI reject: [V: 04-1] Tests: Front-end calls to charge one-time donation via Braintree [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/805906 (https://phabricator.wikimedia.org/T303419) (owner: 10Jgleeson) [18:37:14] jgleeson: hmmm dunno! the one cstone sent does seem to correspond tho [18:37:43] (03PS1) 10AndyRussG: Remove removal of "+" symbols in AstroPay signature [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/808986 (https://phabricator.wikimedia.org/T308167) [18:38:29] (03CR) 10AndyRussG: [C: 04-1] "Alternate approach: Ia38750e78227" [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/800033 (https://phabricator.wikimedia.org/T308167) (owner: 10Ejegg) [18:38:38] ejegg: ^ [18:38:46] will take a look [18:39:35] thx!!! [18:43:33] (03PS1) 10Damilare Adedoyin: WIP: Set granular donor details [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808988 (https://phabricator.wikimedia.org/T311428) [18:44:01] so here's the ticket for when we started removing those + signs: https://phabricator.wikimedia.org/T109335 [18:44:52] (03CR) 10CI reject: [V: 04-1] WIP: Set granular donor details [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808988 (https://phabricator.wikimedia.org/T311428) (owner: 10Damilare Adedoyin) [18:48:20] Hi fr-tech, jgleeson and I had a conversation about how granular we’d like to have the paymentResponse classes and thought it'd be great to have a definition to the structure. [18:50:19] damilare: so if we have processor-specific subclasses, doesn't that defeat the point of normalizing the response? [18:50:38] is this our preferred structure for properties: https://gerrit.wikimedia.org/r/c/wikimedia/fundraising/SmashPig/+/808988/1/PaymentProviders/CreatePaymentResponse.php or can we have nested arrays for groups: [18:50:39] https://github.com/wikimedia/wikimedia-fundraising-SmashPig/commit/22a0df7acdfdbca7ec05dfbcce68819474ce4dde#diff-4f6ca9912d38bca3d54be8e7884fcb80e9a1312c771af0e132e750afcf4ac090R78-R83 [18:51:42] damilare: that's a good question - maybe it should be a child object with its own properties? [18:54:13] I would keep it all on the CreatePaymentResponse object rather than a subclass in any case [18:54:17] yes ejegg, I think the idea of a child object is probably the nexus where both ideas meet. [18:55:03] and maybe have a convenience function ->hasPayerDetails() that will let us know if there is any interesting info in that child object [18:56:13] err, or ->hasDonorDetails(), I guess [18:57:26] AndyRussG: I'm trawling through ancient emails trying to figure out who suggested removing those + signs but I'm not seeing anything relevant around the time of this patch: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DonationInterface/+/232577 [18:57:57] It seemed to fix the problem we were having at the time [18:58:02] thanks ejegg [18:58:21] I like composition [18:58:42] AndyRussG: I suppose they could have change the way they urldecode the requests [19:00:59] damilare didn't we determine earlier that this breaks things in php 7.3 https://gerrit.wikimedia.org/r/c/wikimedia/fundraising/SmashPig/+/808988/1/PaymentProviders/Braintree/PayPalCreatePaymentResponse.php [19:01:42] yeah, I'd pull that down now [19:02:00] AndyRussG: there's also the possibility they treat the + signs differently on production and in sandbox [19:02:03] :( [19:03:17] ejegg: WRT to the properties. I was suggesting sticking with higher granularity of class properties and not so much any normalization. I think early on in the base classes for the smashpig refactor we were trying to define stuff at a granular level to allow *optional* use by gateway [19:03:42] (03PS2) 10Damilare Adedoyin: WIP: Set granular donor details [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808988 (https://phabricator.wikimedia.org/T311428) [19:04:15] (03CR) 10CI reject: [V: 04-1] WIP: Set granular donor details [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808988 (https://phabricator.wikimedia.org/T311428) (owner: 10Damilare Adedoyin) [19:04:42] I personally would prefer specific response child classes in scenarios when a gateway has custom properties we need to care about. Adding them to general classes /pollutes/ other implementations in the case when they are not used or relevant [19:05:13] however PHP doesn't allow it [19:05:34] so we have to drop 'em in the general class [19:07:29] this all came about after we realised the frontend patch isn't currently sending the donors first + last name along with the donation queue msg [19:07:42] which I figured we'd need before the +2 [19:09:58] in other news, Kendrick Lamar rocked Glastonbury last night! [19:20:54] ejegg: true, though I was indeed able to reproduce the issue in the sandbox (and your patch did also solve it there) [19:27:42] (03PS3) 10Damilare Adedoyin: Create donor details class and add it to the createPaymentResponse class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808988 (https://phabricator.wikimedia.org/T311428) [19:28:14] Nice! Did you attend in-person jgleeson ? [19:28:48] I wish damilare ha [19:29:08] It's a lottery these days [19:29:50] I've never been to Glastonbury but always wanted to! [19:33:37] ahh never heard about it, till it was over [19:34:48] You can catch the replay here damilare https://www.bbc.co.uk/iplayer/episode/p0cg05dl/glastonbury-kendrick-lamar [19:45:00] 10Fundraising Sprint Localhost Hospitality Studies, 10Fundraising-Backlog, 10Patch-For-Review: Add donor details to createPaymentResponse - https://phabricator.wikimedia.org/T311428 (10XenoRyet) [19:49:18] 10Fundraising-Backlog: Do not log CC exp date - https://phabricator.wikimedia.org/T86234 (10XenoRyet) 05Open→03Invalid [19:52:59] 10Fundraising-Backlog: Renewal of gcsip.com certificates - https://phabricator.wikimedia.org/T311211 (10Dwisehaupt) 05Open→03Resolved a:03Dwisehaupt We aren't doing certificate pinning for this certificate so no action is needed on our side. Closing. [19:53:21] 10Fundraising-Backlog: Dlocal certificate renewal notice - https://phabricator.wikimedia.org/T310730 (10Dwisehaupt) 05Open→03Resolved a:03Dwisehaupt We aren't doing certificate pinning for this certificate so no action is needed on our side. Closing. [20:18:05] 10Fundraising-Backlog: Do not log CC exp date - https://phabricator.wikimedia.org/T86234 (10Jgreen) We decided in todays FR Tech / Ops backlog refinement meeting to close this task and create a new one specific to what we're currently logging. New task is T311455. [20:20:04] ejegg: ah another question on the dlocal stuff... might there be any other code anywhere that runs the same signature generation logic (like some Civi queue consumer or audit processor)? [20:27:06] AndyRussG: no, it should just be there [20:30:48] (03PS4) 10Damilare Adedoyin: Create donor details class and add it to the createPaymentResponse class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808988 (https://phabricator.wikimedia.org/T311428) [20:31:13] (03CR) 10CI reject: [V: 04-1] Create donor details class and add it to the createPaymentResponse class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808988 (https://phabricator.wikimedia.org/T311428) (owner: 10Damilare Adedoyin) [20:32:36] (03PS5) 10Damilare Adedoyin: Create donor details class and add it to the createPaymentResponse class [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808988 (https://phabricator.wikimedia.org/T311428) [20:33:08] (03PS21) 10Damilare Adedoyin: Front-end calls to charge one-time donation via Braintree [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/801734 (https://phabricator.wikimedia.org/T303419) [20:39:59] ejegg: ah oki cool [20:40:22] 'cause I think I did see something about the same signature used in different dlocal API calls [20:41:18] yeah, we don't do any DLocal recurring so we don't have it under CRM [20:43:33] ahhh oki cool [20:43:37] no refund scripts? [20:43:39] https://dlocal.com/developers/documentation.php?sec=streamline [20:43:49] (search for "control string" in that page) [20:44:23] additional capture steps? [21:06:59] (03CR) 10Jgleeson: Fix cloud vm hostname used in proxy forward script. (031 comment) [wikimedia/fundraising/dev] - 10https://gerrit.wikimedia.org/r/771446 (owner: 10Jgleeson) [21:07:20] AndyRussG: thanks for pointing me to that old patch. I just replied [21:12:08] (03Abandoned) 10Eileen: Stock 5.50.1 [wikimedia/fundraising/crm/civicrm] - 10https://gerrit.wikimedia.org/r/792294 (https://phabricator.wikimedia.org/T308502) (owner: 10Eileen) [21:12:51] (03PS7) 10Wfan: Add report or search result for transactions [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808008 (https://phabricator.wikimedia.org/T310756) [21:14:18] (03CR) 10CI reject: [V: 04-1] Add report or search result for transactions [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808008 (https://phabricator.wikimedia.org/T310756) (owner: 10Wfan) [21:23:22] (03PS8) 10Wfan: Add report or search result for transactions [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808008 (https://phabricator.wikimedia.org/T310756) [21:24:00] (03CR) 10CI reject: [V: 04-1] Add report or search result for transactions [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808008 (https://phabricator.wikimedia.org/T310756) (owner: 10Wfan) [21:24:12] (03CR) 10Jgleeson: [C: 03+1] "Really like this! Thanks for the refactor. One small suggestion on the null vs empty strings subject but it's not a blocker." [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808988 (https://phabricator.wikimedia.org/T311428) (owner: 10Damilare Adedoyin) [21:25:29] (03PS9) 10Wfan: Add report or search result for transactions [wikimedia/fundraising/SmashPig] - 10https://gerrit.wikimedia.org/r/808008 (https://phabricator.wikimedia.org/T310756) [21:26:26] (03PS14) 10Jgleeson: Tests: Front-end calls to charge one-time donation via Braintree [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/805906 (https://phabricator.wikimedia.org/T303419) [21:32:18] (03CR) 10Jgleeson: "Looks like https://gerrit.wikimedia.org/r/c/wikimedia/fundraising/SmashPig/+/808988/ is now a depends-on to get this one working." [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/801734 (https://phabricator.wikimedia.org/T303419) (owner: 10Damilare Adedoyin) [21:33:41] (03PS1) 10Eileen: Merge branch 'master' of https://gerrit.wikimedia.org/r/wikimedia/fundraising/crm into test_it [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/809025 [21:33:43] (03PS1) 10Eileen: Civix upgrade - boilerplate cleanup [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/809026 [21:35:02] (03PS1) 10AndyRussG: Merge branch 'master' into wmf_deploy [extensions/CentralNotice] (wmf_deploy) - 10https://gerrit.wikimedia.org/r/809027 [21:35:40] (03CR) 10AndyRussG: [C: 03+2] Merge branch 'master' into wmf_deploy [extensions/CentralNotice] (wmf_deploy) - 10https://gerrit.wikimedia.org/r/809027 (owner: 10AndyRussG) [21:37:37] (03CR) 10CI reject: [V: 04-1] Tests: Front-end calls to charge one-time donation via Braintree [extensions/DonationInterface] - 10https://gerrit.wikimedia.org/r/805906 (https://phabricator.wikimedia.org/T303419) (owner: 10Jgleeson) [21:44:07] fr-tech, I managed to complete the end-to-end braintree webhook test locally and the signature verification parts are not that complicated, a few fns passing stuff around, so we can start porting it over to smashpig with confidence! [21:44:52] catch you all tomorrow [21:48:05] 10Fundraising-Backlog: Deleted Contact Overwriting Current Contact Record - https://phabricator.wikimedia.org/T311438 (10Aklapper) [21:57:19] (03Merged) 10jenkins-bot: Merge branch 'master' into wmf_deploy [extensions/CentralNotice] (wmf_deploy) - 10https://gerrit.wikimedia.org/r/809027 (owner: 10AndyRussG) [22:34:52] just got a ping from travel [22:35:54] eileen: ah yay! [22:36:04] GREAT~ [22:38:49] good luck on flights. just got my confirmation and the flights are different than the initial. [22:45:52] oh cool - she asked me if I was OK with the whole US get off the plane go through customs get back on rigmarole - I think that's the least of my worries :-) [22:46:33] hah [22:48:24] (03PS1) 10Eileen: Enable braintree [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/809035 [22:48:45] wfan: ^^ [22:49:12] aha~ eileen: thanks! [22:49:26] (03PS2) 10Wfan: Enable braintree [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/809035 (owner: 10Eileen) [22:49:28] but all that does is enable the module on rebuild [22:49:42] ie the same as manually enabling it on an already-built site [22:49:57] Nice, thanks for this [22:50:39] also - I didn't double check the module name - I guessed it [22:52:15] (03PS3) 10Wfan: Enable module braintree_audit [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/809035 (owner: 10Eileen) [22:53:05] (03CR) 10Wfan: [V: 03+2 C: 03+2] "Thanks then we no do not need to make module enable manually" [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/809035 (owner: 10Eileen) [23:06:25] (03Merged) 10jenkins-bot: Enable module braintree_audit [wikimedia/fundraising/crm] - 10https://gerrit.wikimedia.org/r/809035 (owner: 10Eileen)