A bit unofficial, but posted in the project channel (https://wikimedia.slack.com/archives/C092G2CFB43/p1764952320563829) so that portion is ready for signoff

^ different deployment, but we can probably leave this in signoff instead of moving it and then moving it back in two hours

@aude to backport on Tuesday, December 2nd (with @LMora-WMF and potentially @Volker_E shadowing)

Adding to the sprint since it got merged

Heroes, thank you!

Loren told me the role="region" issue is not a problem, and I was able to confirm the aria live region is now present, so this can be signed off!! It's THREE (3) minutes past 5 so I am setting a bad example by still being online 😔 apologies team

Done and QAed and backported - to be tested in prod whenever T409650 gets picked up, but signing off as we're pretty confident with this one

Looks good - only one failure and it's the infamous 5 items grid

Feels a little bit cheating that I'm signing this off and I worked on it, but I can certify that the aforementioned patch was backported so signing this off (thank you Katie!!)

Loren to work on (thank you!!)

yolo

@aude graciously added me to the preference on enwiki, and I was able to confirm that this is working as intended - I was able to see the experiment using we-3-3-4-reading-list-test1-en, and unable to see it using we-3-3-4-reading-list-test1. Signing this off

Sadly, turns out I can't QA on english wikipedia either since I don't have the preference set 🙃 to feel like I did something I reviewed the code and it looks good to me

I will QA this since Edward is out and it's past Thursday now

I think the save_to_reading_list vs save_article_to_reading_list discrepancy is fine, since the spec seems to have both - I'll leave a comment in the spec, but we can sign this off 🤞

Discussed in DMs and slack - I mistakenly thought this could be tested as such, but it will only work on english wikipedia (and potentially beta, but we don't have the hidden preference set)

(events will appear in the console!)

From meeting: since we didn't add ourselves to the preference on beta, this will need to be tested on testwiki any time starting tomorrow

From meeting: you will need to turn off the beta feature and then run mw.xLab.overrideExperimentGroup('we-3-3-4-reading-list-test1', 'treatment'), etc in the console to test this - can be tested on testwiki

Posted in the team channel: https://wikimedia.slack.com/archives/C08TPPG26E8/p1762550160467979

Reviewed both queries and either approach seems fine to me - I would say that constitutes 80% of a code review, but given we have validation in the script that has been 200% code reviewed I feel like this is good enough for us to use 🤞

That should be fine - we can put the extra 500 in either Arabic or French (or both), but let's limit especially on English so that we're not adding much more than the 200k we promised Amir

No longer stalled yay

Steph to by end of day tomorrow double check the list, do backports if possible (womp womp Friday), if not ensure we have the authoritative list of things that need to go out Monday and ideally coordinate and schedule deployers

Lowering the priority as we're not considering this the worst case if it doesn't make the launch - that said, Steph to review and @Jdlrobson-WMF to co-review

Hsuanwei to sign off

Updated the phab ticket, but looks good to me - idk who should do QA, but ideally this would get looked at by at least one more person before we sign it off

(sorry for lurking and sorry to Edward for giving you more work 😭)

It can be QAed!! We should now be able to turn on the reading list feature using either we-3-3-4-reading-list-test1 or we-3-3-4-reading-list-test1-en where previously we could not

Clarified that the above is fine in the ticket - we needed to override the experiment grouping and then check the console, as events don't get sent in the override case

From slack:

I forgot - the reason it wasn't working is because events logged through the experiments platform aren't actually sent if you override yourself into the experiment. They get put in the console. Using mw.xLab.overrideExperimentGroup( 'we-3-3-4-reading-list-test1', 'treatment' ) I was able to see the following in my console:

click {
  "action_subtype": "view_article",
  "action_source": "reading_list",
  "action_context": "{\"article_count\":6}"
}

Steph to sign off bc of the issue in T405002

Steph to verify why the heck this isn't working

QAed by Sneha, so should be signed off by someone else

Volker and Steph both reviewed!!

Nothing's gonna be a blocker coming out of this, we can't reproduce the original bug 🎉

Confirmed can't be QAed - to be signed off by someone that isn't Steph!

Volker will try to review, and Steph will also try to review

Also @Volker_E are you still taking a look at this or does it need another asignee?

Bringing this back up to high as I believe it was only medium in the context of last sprint

more phab stalking ignore me 😇

On the bolder side of things I've done in phabricator - moving this to signoff as we confirmed in slack that both of the failures caught were working as intended (thank you all who looked!!)

Katie to confirm when she has time, but I think this is not testable easily, correct

DB config:
https://noc.wikimedia.org/db.php?dc=eqiad (unclear where x1 is, but not immediately necessary)

Posted in the team channel - I think we can keep this ticket closed and open a new one once Sneha/Hsuanwei/Volker weigh in. I'll tag them too

Follow up work for this ticket will be to do analysis of how widespread this issue is in production, and decide whether to do a deduplication effort or advice of workaround, etc

1.0.0-beta.1 is what we agreed upon - discussion to be had in eng enclave about whether any of this matters

Explicitly, the work of this ticket is to assume the same approach or class of approach that fixed T381289 will fix this - please try to apply said fix, and if it doesn't work, let the team know and we'll regroup ☺️

Probably me then Anne when she's back, unless Jon disagrees

Confirmed this works locally! Moving to QA (sorry Edward)

As of now, we're unable to reproduce the issue we saw on test wikipedia that was a potential blocker - Katie to look at this ticket when she has time (also Steph fill the description in!!)

Katie to review patch once it's ready

Katie to add replication steps for local repro, then Steph to final review the patch

Volker to double check the review (thank you!)

I would propose the names be we-3-3-4-reading-list-test1 and we-3-3-4-reading-list-test1-en, but let's give a (short) period of time for eng to bikeshed on names

Skipping straight to sign off as it's already been reviewed and can't be QAed

moving into sprint as it's been approved to be merged

@SToyofuku-WMF to add charts and graphs from the DB side, courtesy of Amir

Jan to reach out in Slack!

Loren to uncomment out the library loading, then put back in code review for the second patch

We're assuming this one can't be tested until T406930 as the library is currently commented out

No worries! Thanks for taking a look ☺️

The point was that this one makes it more clear how to navigate to your saved pages without having to save something else to get the popup - we were hoping the dialogue would overlap on top of the ephemeral notification and cover it entirely, but if that's not the case we might want to figure out how to hack it if possible

And into QA - someone who cares more than me can figure out the pointing calculus, but leaving it untouched for now

Moving into the sprint as the patch was merged

Apologies - I was waiting for @Ladsgroup to confirm the script looked good to him, but I guess he already did that