Page MenuHomePhabricator
Create Task
Maniphest T394399

No longer able to preview centralnotice banners
Closed, ResolvedPublic
Actions

Description

I can't see any centralnotice banners on the sites, either previewing https://sv.wikipedia.org/wiki/NASA?banner=B2425_0501_mlWW_dsk_p2_sm_twn_twin1&country=SE or faking a country which should have a live campaign https://en.wikipedia.org/wiki/Main_Page?country=BE

If I manually edit my GeoIP cookie to somewhere with a running campaign (e.g. BE), then I see the banners. Looking in the fundraising database we are also still logging impressions and getting donations from banners. So it seems just related to previewing via URL.

@Romaine reported this was occurring on Meta only and working on other projects at around 0750 UTC today. So I suspect something on today's train, possibly https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CentralNotice/+/1125253 (T380079) which was dealing with urls

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Make BundleSizeTest cross-compatible with <=1.44 and >=1.45mediawiki/extensions/CentralNoticewmf/1.45.0-wmf.1+9 -1
Make BundleSizeTest cross-compatible with <=1.44 and >=1.45mediawiki/extensions/CentralNoticemaster+8 -0
Fixes iteration of existing URLSearchParamsmediawiki/extensions/CentralNoticemaster+3 -6
Customize query in gerrit

Related Objects

Event Timeline

Pcoombe created this task.May 15 2025, 11:30 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 15 2025, 11:30 AM
Pcoombe triaged this task as Unbreak Now! priority.May 15 2025, 11:31 AM
Pcoombe added a comment.May 15 2025, 11:36 AM

Marked this UBN because it blocks online fundraising and centralnotice admin work, but it's not affecting end users (they still see correct banners) so I'm not sure it would warrant a rollback

jgleeson added a project: Fundraising Tech - Chaos Crew.May 15 2025, 11:39 AM
Romaine added a comment.May 15 2025, 12:55 PM

Previewing the banners is used as a check to make sure banners are shown correctly and link to the right page(s). If such a check is not possible, this may result in human caused errors are visible to end users, because they haven't been detected because previewing the banner is not possible.

gerritbot added a comment.May 15 2025, 2:53 PM

Change #1146648 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/CentralNotice@master] Fixes iteration of existing URLSearchParams

https://gerrit.wikimedia.org/r/1146648

gerritbot added a project: Patch-For-Review.May 15 2025, 2:53 PM
Jdlrobson-WMF subscribed.May 15 2025, 2:53 PM

There is an API request to https://meta.wikimedia.org/w/index.php?title=Special:BannerLoader&banner=undefined&uselang=en&debug=false
Debugging I was able to confirm getBannerURL is called with a data object with no banner key - which led to above patch.

Johannnes89 subscribed.May 15 2025, 3:55 PM
Ciell subscribed.May 15 2025, 4:15 PM
gerritbot added a comment.May 15 2025, 6:41 PM

Change #1146648 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] Fixes iteration of existing URLSearchParams

https://gerrit.wikimedia.org/r/1146648

ReleaseTaggerBot added a project: MW-1.45-notes (1.45.0-wmf.2; 2025-05-20).May 15 2025, 7:00 PM
Maintenance_bot removed a project: Patch-For-Review.May 15 2025, 7:30 PM
ehughes subscribed.May 16 2025, 10:13 AM
Jdlrobson-WMF added a comment.May 16 2025, 5:18 PM

Does this need backporting or is it okay to go out on the train next week? If backporting when (today? Monday?) and who ?

I was assuming it would go out with the train but didn't want to make assumptions since it has been marked with UBN.

KMorrow-WMF subscribed.May 16 2025, 7:58 PM
In T394399#10830721, @Jdlrobson-WMF wrote:

Does this need backporting or is it okay to go out on the train next week? If backporting when (today? Monday?) and who ?

I was assuming it would go out with the train but didn't want to make assumptions since it has been marked with UBN.

Just chiming in to say we do have quite a few fundraising banner tasks blocked by this (e.g., linguist proofing, building & QAing banners to be tested next week). Is it possible to have the fix applied by end of day Monday, May 19 at the latest?

gerritbot added a comment.May 19 2025, 6:31 PM

Change #1147839 had a related patch set uploaded (by Jgleeson; author: Jgleeson):

[mediawiki/extensions/CentralNotice@wmf/1.45.0-wmf.1] Make BundleSizeTest cross-compatible with <=1.44 and >=1.45

https://gerrit.wikimedia.org/r/1147839

gerritbot added a project: Patch-For-Review.May 19 2025, 6:31 PM
spatton awarded a token.May 19 2025, 6:39 PM
jgleeson added subscribers: greg, jgleeson.May 19 2025, 6:44 PM

@Jdlrobson I just pushed up a patch to fix the CI failures relating to the namespace fix here.

@greg mentioned that we wanna try and get this out in the next backport window. I'm unfamiliar with the backport process, so I'll see if I can figure that out next.

XenoRyet moved this task from Triage to Chaos Crew Backlog on the Fundraising-Backlog board.May 19 2025, 7:12 PM
greg moved this task from Backlog to Pending Deployment on the Fundraising Tech - Chaos Crew board.May 19 2025, 7:30 PM
Jdlrobson-WMF added a comment.May 19 2025, 10:32 PM

I chained your patches so they pass CI. You should be able to schedule them both for an available backport window using https://schedule-deployment.toolforge.org/

Let me know on Slack if you need help (post 9am PST). Since you are UK time you can probably get it out before I wake up though!

gerritbot added a comment.May 20 2025, 12:21 AM

Change #1147839 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@wmf/1.45.0-wmf.1] Make BundleSizeTest cross-compatible with <=1.44 and >=1.45

https://gerrit.wikimedia.org/r/1147839

Maintenance_bot removed a project: Patch-For-Review.May 20 2025, 12:30 AM
gerritbot added a comment.May 20 2025, 1:25 AM

Change #1147910 had a related patch set uploaded (by Ejegg; author: Jgleeson):

[mediawiki/extensions/CentralNotice@master] Make BundleSizeTest cross-compatible with <=1.44 and >=1.45

https://gerrit.wikimedia.org/r/1147910

gerritbot added a project: Patch-For-Review.May 20 2025, 1:25 AM
gerritbot added a comment.May 20 2025, 1:33 AM

Change #1147910 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] Make BundleSizeTest cross-compatible with <=1.44 and >=1.45

https://gerrit.wikimedia.org/r/1147910

Maintenance_bot removed a project: Patch-For-Review.May 20 2025, 2:30 AM
jgleeson added a subscriber: Ejegg.May 20 2025, 9:37 AM
In T394399#10837537, @Jdlrobson-WMF wrote:

I chained your patches so they pass CI. You should be able to schedule them both for an available backport window using https://schedule-deployment.toolforge.org/

Let me know on Slack if you need help (post 9am PST). Since you are UK time, you can probably get it out before I wake up, though!

It looks like @Ejegg merged that test fix and your patch into the wmf_deploy branch on CentralNotice last night, so I'll see if we can submit the tip of that branch to be deployed. Thanks for all the help!

jgleeson moved this task from Pending Deployment to Done on the Fundraising Tech - Chaos Crew board.May 20 2025, 10:22 AM
jgleeson added a subscriber: taavi.

Ok, we managed to get this out with a lot of help from @taavi, and @Pcoombe has confirmed banner previews are back. Thanks, everyone, for helping us with this!

Pcoombe closed this task as Resolved.May 29 2025, 3:39 PM
Pcoombe claimed this task.
Pcoombe removed Pcoombe as the assignee of this task.
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits