[07:26:55] (CR) jerkins-bot: [V: -1] Localisation updates from https://translatewiki.net. [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/509250 (owner: L10n-bot) [09:20:46] (CR) jenkins-bot: Localisation updates from https://translatewiki.net. [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/509226 (owner: L10n-bot) [14:29:46] (PS5) Mepps: Create contact and opt-in from just email address. Log if we get an already existing email that is not tied to any contacts, but take no other action. [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/506837 (https://phabricator.wikimedia.org/T217710) [14:31:37] (CR) Mepps: "Made a quick update but I have a question on the log message." (1 comment) [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/506837 (https://phabricator.wikimedia.org/T217710) (owner: Mepps) [14:32:14] (CR) jerkins-bot: [V: -1] Create contact and opt-in from just email address. Log if we get an already existing email that is not tied to any contacts, but take no other action. [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/506837 (https://phabricator.wikimedia.org/T217710) (owner: Mepps) [14:33:24] (PS6) Mepps: Create contact and opt-in from just email address. Log if we get an already existing email that is not tied to any contacts, but take no other action. [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/506837 (https://phabricator.wikimedia.org/T217710) [14:45:32] hi fr-tech [14:45:41] hi ejegg! [14:45:48] i'm going to try running the adyen job from frdev for a bit and see if it has the same failures [14:46:04] ejegg mepps hellosies-ohs [14:46:17] great ejegg [14:46:19] hi ejegg mepps AndyRussG ! [14:46:29] jgleeson hey :) [14:46:30] hi AndyRussG mepps jgleeson! [14:46:51] ejegg, I'm currently running the adyen job locally but it's exiting with 0 [14:47:04] jgleeson: yeah, that's what I'm finding too [14:47:11] so still in head-scratching territory [14:47:53] And when I step through the destructor code I don't see anything in the SOAP bits running [14:48:17] just the disconnections from redis [14:49:18] hmm [14:49:29] ejegg, do you need to be on the vpn to use adyen staging details? [14:49:32] (PS1) Mepps: WIP: Started test for email only but ran into some questions [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/509421 (https://phabricator.wikimedia.org/T217710) [14:49:46] jgleeson: no, just make sure you have the credentials [14:50:05] I pulled them form payments-wiki-testing in the localsettings repo [14:50:10] 20-Don... [14:50:16] yep, those should do it [14:50:26] hmmm I'm getting errors when trying to submit [14:50:28] I'll dig into it [14:50:30] well, for setting up the things in DonationInterface anyway [14:50:43] ah, is there elsewhere I need to configure? [14:51:14] oh, there's a README in SmashPig/PaymentProcessors/Adyen/Tests about how to make up some IPN messages locally [14:51:44] right, so Adyen is pretty async. We just request auth on Payments-wiki, then wait for an IPN [14:52:11] info: "[8a7703e81504bbafa8dc4333] Exception caught: Skin code not configured" [14:54:31] jgleeson: hmm [14:54:38] ejegg, looks like the skinCode key wasn't present in the detials on payments-wiki-test, however adding them in still doesn't fix it [14:54:48] oh, let me check [14:55:59] ejegg, sorry that's a mistake, the skin code is there [14:56:08] I was looking at astropay [14:56:41] skincode begins with L [14:56:49] (in the details I have) [15:00:24] yep [15:01:35] ah shoot [15:13:57] Fundraising-Backlog, FR-Civi-Planned-Giving, FR-Civi-Prospect: New total fields needed for MG prospecting - https://phabricator.wikimedia.org/T222958 (NNichols) [15:41:35] breaking for a while, back for standup fr-tech! [15:50:25] ejegg: hi! :) [15:52:58] Fundraising-Backlog, FR-Astropay: RedPagos (cash) form in Uruguay fails to load with a fatal exception - https://phabricator.wikimedia.org/T222961 (Pcoombe) [16:00:27] AndyRussG: shoot, I have some random notes about FRUEC to paste into gerrit. Any preference which patch I put 'em on? [16:01:29] ejegg oh hey cool! Maybe just the last one in the trail? Or if it's easier, just tack them on the end of the etherpod? [16:02:09] ok, lemmee see [16:03:34] (PS1) Umherirrender: Avoid var declaration only for return [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/509451 [16:05:50] !log moved adyen smashpig job runner to frdev1001 [16:05:54] Logged the message at https://wikitech.wikimedia.org/wiki/Server_Admin_Log [16:09:15] (CR) Reedy: [C: +2] Avoid var declaration only for return [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/509451 (owner: Umherirrender) [16:10:21] hmm? Why didn't that job actually get scheduled on frdev? [16:13:44] (CR) Reedy: [C: +2] Move files to match class name [extensions/ContributionTracking] - https://gerrit.wikimedia.org/r/507640 (owner: Umherirrender) [16:20:37] (Merged) jenkins-bot: Avoid var declaration only for return [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/509451 (owner: Umherirrender) [16:26:43] (Merged) jenkins-bot: Move files to match class name [extensions/ContributionTracking] - https://gerrit.wikimedia.org/r/507640 (owner: Umherirrender) [16:37:56] (CR) jenkins-bot: Move files to match class name [extensions/ContributionTracking] - https://gerrit.wikimedia.org/r/507640 (owner: Umherirrender) [16:44:19] (CR) jenkins-bot: Avoid var declaration only for return [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/509451 (owner: Umherirrender) [16:53:42] Fundraising-Backlog, Fr-CiviCRM-dedupe-FY2017/18: Civi dedupe: queries yielding http 500 errors or blank screens - https://phabricator.wikimedia.org/T222902 (MBeat33) p:Triage→High Calling this High priority as a contractor is do deduping unable to work because of it. [16:54:48] Fundraising-Backlog: New mobile iFrame errors - https://phabricator.wikimedia.org/T221385 (MBeat33) #584030 update tried using iPhone 6 Version 11.2.6 and Chrome browser, as well as Safari sent screenshot showing a blank iframe/card entry section [17:00:13] fr-tech: meeting on alerts is IRC? [17:02:06] ejegg: we were going to meet about CN work. [17:02:25] the alerts stuff AndyRussG was just finishing and then event logging [17:04:06] ejegg dstrine AndyRussG I can meet now [17:04:25] ok cool andy and I are in the call [17:04:56] ejegg we're in the call now [17:04:58] oops, getting out of the quiet room here [17:29:02] ejegg: did u end up putting the fruec notes you mentioned ^ somewhere? [17:29:43] AndyRussG: working on copying em up to one of the later patches [17:30:03] actually... lemme just make a do-not-merge commit [17:30:04] ejegg: ah ok cool beans, thx, yea no rush! [17:34:04] (PS1) Ejegg: [do not merge] Code review [wikimedia/fundraising/FRUEC] - https://gerrit.wikimedia.org/r/509473 [17:34:24] AndyRussG: ok, they're just added as comments in that do-not-merge commit ^^ [17:44:01] ejegg: nice! thx!!! [18:13:28] Fundraising-Backlog, fundraising-tech-ops, FR-Ingenico: install nginx 1.14.1 from stretch-backports on payments servers, enable TLSv1.3 - https://phabricator.wikimedia.org/T222974 (Jgreen) [18:34:56] Fundraising-Backlog, FR-Astropay: Please remove "Provincia Pagos" from Argentina cash options - https://phabricator.wikimedia.org/T222977 (Pcoombe) [18:41:27] ok, that seems to actually run the drush command [18:53:14] fr-tech heads-up, looks like the automagic adding of reviewers on Gerrit is brokin [18:53:29] oh noes [18:53:47] which project are you seeing it on? [18:53:55] e.g. https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/CentralNotice/+/508720/, https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/CentralNotice/+/502772/ [18:54:13] hmmm [18:54:38] I wonder if it's gerrit or zuul where that happens? [18:54:52] jgleeson: there are two ways to make it happen [18:55:00] gerrit lets you watch repos [18:55:14] and then there's a page on wiki that lets you watch regexes [18:55:16] I also saw an error just now on gerrit about the "reviewers" module not being loaded or something [18:55:22] but that wiki page depends on a bot [18:55:28] which can be kind of fragile [18:55:32] ejegg we're all correctly signed up there: https://www.mediawiki.org/wiki/Git/Reviewers#mediawiki/extensions/CentralNotice [18:56:12] what's the other way? [18:56:18] hmm, and the bot logs don't show obvious breakage: https://tools.wmflabs.org/gerrit-reviewer-bot/ [18:57:05] AndyRussG: https://gerrit.wikimedia.org/r/#/settings/projects [18:57:33] well, those 'watched projects' at least get the changes on one's dashboard [19:00:57] XenoRyet|afk, ejegg, cstone mepps, could you link me to the frontend optin patch so I can do the end-to-end test as part of my review? it would also be good to have a queue message to play around with [19:01:11] please!** [19:01:22] jgleeson: the frontend optin stuff is all merged [19:01:31] you can just update DonationInterface [19:01:41] ejegg: thx... yeah just alerted folks on -releng [19:01:57] and go to Special:EmailPreferences/optin [19:02:07] ah ejegg, let me find the commit in the log so I can see the changes in isolation (and figure out how to set it up) [19:02:20] or for the fancy version, Special:EmailPreferences/optin?variant=wle_001 [19:03:23] jgleeson: also, looks like the settings in payments-wiki-testing were out of date [19:03:28] I pushed a new version of them [19:03:44] ah cool! thanks [19:05:43] btw ejegg, did you manage to simulate the ProcessCaptureRequestJob flow locally and confirm it's exiting with 0? [19:09:18] (CR) Jforrester: [C: +1] CentralNoticeHooks: Bundle configuration var into startup module [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/502772 (https://phabricator.wikimedia.org/T219342) (owner: D3r1ck01) [19:12:00] ok great, got the optin stuff showing up locally. Thanks ejegg. Will pick up on review a little later as I'm breaking now [20:13:50] jgleeson: yes, I ran through the whole thing [20:14:02] I had to use the test harness to make some IPN calls [20:14:25] there's a README on that in SmashPig/PaymentProviders/Adyen/Tests/ [20:14:51] there's a faker.py script to make some IPNs, with matching pending messages [20:15:30] but since I was working from sandbox donations created through the front end [20:15:46] I just used the auth.xml IPN file [20:16:07] and edited it so the merchant reference and the psp reference matched those from the front end logs [20:34:30] ok fr-tech, I just deployed a settings update that should make it so only I get the failmails from adyen jobs for now [20:34:51] such privilege! [20:35:03] anyone curious about how to do that can take a look at the last commit in the settings repo [20:35:26] thx..! [20:42:25] looks like XenoRyet|afk's patch got some review from eileen [20:42:45] what's the latest version of the other patch mepps and cstone? [20:43:36] also, oops, that smashpig-level config doesn't affect the failmail from process-control [20:43:41] so everyone's still getting spammed [20:44:14] we didn't make that overridable in the job yaml, did we? [20:45:20] nope, appears not [20:51:48] ejegg this is the latest one https://gerrit.wikimedia.org/r/#/c/wikimedia/fundraising/crm/+/506837/ [20:52:09] mepps and cstone should we restore PS4 (the squashed version) of https://gerrit.wikimedia.org/r/506837 [20:52:22] PS6 still seems to be sitting on top of a WIP patch [20:58:41] cstone want me to do that? [20:59:21] sure if you want is there an easy way in gerrit to show all the different patchsets at once? [20:59:34] fundraising-tech-ops, Gerrit: move main fundraising puppet repo to gerrit - https://phabricator.wikimedia.org/T222840 (hashar) [21:00:10] cstone: hmm, I thought there was some way in the UI to restore an old patch set [21:00:15] but now I don't see it [21:00:37] the numbers at the top let you switch between them but i didnt know if i could compare two of them or something [21:00:38] maybe just push the one you've got locally again? [21:01:00] ah, to compare them you can use that 'Diff against:' dropdown near the center of the page [21:02:37] ok, right, doing that, it looks like PS6 fixes one thing, but undoes the squashing and reverts an earlier-merged patch [21:02:47] the sha of the commit of old patchsets should be in your local repo, if you ever pulled them down, so you can do git diff sha1..sha2 (or git diftool -d sha1..sha2) [21:02:57] yeah, that's probably easiest [21:03:01] I think git review might have a flag for specifying patchset to download too [21:03:11] just get your local repo dialed into the one you want and do another git review [21:04:04] ah yeah I do that by accident too often (like after pushing rebase on the UI and then not downloading that PS) [21:05:55] would that let me see more than 2 at once? [21:06:49] hmmm you mean like a 3-way diff? I guess not, or maybe... hmmm [21:07:16] meld can do a 3-way diff on its own [21:07:34] so you could git checkout sha-whatever for each patch set you want to look at [21:07:41] then copy it to a different directory [21:08:34] then say meld dir-version-1/file.php dir-version-2/file.php dir-version-3/file.php [21:10:13] ok cool [21:10:14] you could use the git difftool -d shaX shaY to compare several pairs of versions, one after another, without checking any out [21:11:41] also the Web UI for Gerrit is pretty handy at comparing different PS's, just click on a file and at the top of the diff there are buttons to change the PS being compared [21:13:28] (PS1) Ejegg: Call getGlobal on class, not interface [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/509527 (https://phabricator.wikimedia.org/T222961) [21:13:47] yeah i guess in this case its been touched by a bunch of different people but not really changed so I wanted to see more than two [21:15:51] cstone you can compare PS4 with PS6 with that dropdown if you want [21:15:58] then compare PS5 with PS6 [21:16:08] yeah thats working [21:16:54] did we want to keep the changes in PS6 but just squash it again? squish it? hah [21:17:35] cstone: we do want to keep the import fix (get rid of stray Dompdf line) [21:17:47] but the bits that erase the doNotSolicit logic are a regression [21:18:27] (PS1) Ejegg: Enable dlocal form for UY / cash [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/509528 (https://phabricator.wikimedia.org/T222961) [21:18:47] ah I see okay [21:19:03] so maybe easier for you locally [21:19:11] to just check out your own PS4 [21:19:17] and then fix the Dompdf line [21:19:27] yeah okay [21:21:16] (CR) jerkins-bot: [V: -1] Enable dlocal form for UY / cash [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/509528 (https://phabricator.wikimedia.org/T222961) (owner: Ejegg) [21:21:52] jeez, real picky about comment formatting [21:22:27] (PS2) Ejegg: Enable dlocal form for UY / cash [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/509528 (https://phabricator.wikimedia.org/T222961) [21:27:36] ejegg for what both eileen and maggie changed in the else should I put the latest of that back in [21:28:26] Fundraising-Backlog, FR-Astropay: Please remove "Provincia Pagos" from Argentina cash options - https://phabricator.wikimedia.org/T222977 (DStrine) This can be removed on donate wiki [21:28:58] cstone: looks right [21:29:52] cstone but i guess i still don't get why we're ignoring an email when we find it's already in use as a non-primary. [21:30:35] If we're determined to do *something* with all the emails we get, and someone gives an email on an opt-in form that's already associated with a contact [21:30:56] it seems like we should treat that as them saying 'yes, send me emails, at *this* address' [21:31:15] so maybe we should opt them in and make that address primary [21:34:26] anyway, we can deal with non-primary emails in another patch if you want [21:34:39] maybe leave a TODO as a clue [21:39:47] hmm yeah ive somehow completely confused myself on what is happening here [21:40:42] cstone: ok, let's start just with PS4 + zap Dompdf exception + make that change in the else { } block? [21:40:55] hah yeah that works [21:41:22] but so youre saying if the email is found but tied to another contact as non primary, it should be recorded cause they changed the email to that [21:42:13] cstone: yeah, in another patch [21:42:57] okay less confused now [21:43:58] I think we would change the getContactsFromEmail to return the is_primary field too [21:45:14] oh hmm, /are/ we actually filtering on is_primary? Not seeing where that happens [21:51:59] (PS7) Cstone: Create contact and opt-in from just email address. Log if we get an already existing email that is not tied to any contacts, but take no other action. [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/506837 (https://phabricator.wikimedia.org/T217710) (owner: Mepps) [21:52:42] ejegg i never finished looking into what is_primary , its tied to an email and then each contact can have multiple emails, but only one is_primary [21:52:51] missing a ? on the end of that statemnt [21:55:03] (PS8) Cstone: Create contact and opt-in from just email address. Log if we get an already existing email that is not tied to any contacts, but take no other action. [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/506837 (https://phabricator.wikimedia.org/T217710) (owner: Mepps) [21:57:16] also didnt mean that to be two patches good when you actually add your changes to commit [21:58:29] cstone: I'll review that one as it is now, and if I do anything about the is_primary it'll be a different commit [22:00:11] cstone: but yeah, so emails are stored in the civicrm_email table [22:00:22] which has a foreign key to civicrm_contact [22:00:32] so a contact can have multiple emails [22:00:51] they should only have one email with is_primary=1 [22:01:00] and they should only have one email for each location_type_id [22:01:27] e.g. a contact can have a home and a business email, with the home marked as is_primary [22:01:48] in fact, they should always have exactly one marked as is_primary=1 [22:02:11] ah okay [22:07:57] hmph, running unsubqc locally does nothing [22:08:24] well, it outputs "Executing: drush_unsubscribe_queue_consume" then "Command dispatch complete" [22:08:30] but it's not touching the queues [22:08:56] what am i doing wrong? [22:09:26] hmm i am getting new contact created [22:11:09] oh weird, not finding the module? [22:11:36] Fundraising-Backlog, FR-Astropay: Please remove "Provincia Pagos" from Argentina cash options - https://phabricator.wikimedia.org/T222977 (Pcoombe) @DStrine No it can't. It's on the payments page, see the link. [22:12:30] ok, got it enabled. and trying again [22:14:47] (CR) Ejegg: [C: +2] "Looks good!" (2 comments) [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/506837 (https://phabricator.wikimedia.org/T217710) (owner: Mepps) [22:15:06] oops, those 2 comments were the same draft applied to 2 old PSes [22:16:23] (PS1) Ejegg: Whitespace and import cleanup [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/509535 [22:16:37] cstone: quick formatting follow-up ^^^ [22:16:44] and... I gotta head out! [22:19:26] (Merged) jenkins-bot: Create contact and opt-in from just email address. Log if we get an already existing email that is not tied to any contacts, but take no other action. [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/506837 (https://phabricator.wikimedia.org/T217710) (owner: Mepps) [22:22:31] (CR) Cstone: [C: +2] Whitespace and import cleanup [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/509535 (owner: Ejegg) [22:27:01] (Merged) jenkins-bot: Whitespace and import cleanup [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/509535 (owner: Ejegg)