Page MenuHomePhabricator
Create Task
Maniphest T210983

CentralNotice protects CentralNotice templates endlessly each time a template is editted, polluting history and logs
Closed, ResolvedPublic4 Story Points
Actions

Description

Hi. Please take a look for example at this page history. Every time the template is amended via Special:CentralNoticeBanners CentralNotice issues a new protection to the page. This is unnecesary and unnecessarily pollutes the page history and the protection log. Please have CentralNotice check if the page is protected and its settings before issuing a new protection

In the long term I suggest a dedicated namespace for CentralNotice stuff, protected by default (via $wgNamespaceRestrictions) to the desired levels. Issuing indefinite protections cause Special:ProtectedPages to become increasingly long as well, making it difficult to patrol.

Thank you.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 3 2018, 9:07 AM
Pcoombe added a subscriber: Pcoombe.Dec 3 2018, 7:49 PM
• MarcoAurelio added a comment.Dec 4 2018, 11:59 AM

In addition, it clutters Special:RecentChanges badly as well (which is happening at Meta right now). Thanks.

gerritbot added a subscriber: gerritbot.Dec 7 2018, 8:37 PM

Change 478257 had a related patch set uploaded (by Ejegg; owner: Ejegg):
[mediawiki/extensions/CentralNotice@master] Don't re-protect banner if it's already protected

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

Ejegg added a subscriber: Ejegg.Dec 14 2018, 12:26 AM

I couldn't replicate this either, and was pretty confused about why it would happen, since there's code in WikiPage::doUpdateRestrictions that exits early if the protections are already as they should be. I think it's a result of the way we're saving new banners - we first save a blank page, then do an edit. Now we're adding protection on the blank page, and protecting again on the edit. The checks inside doUpdateRestrictions must be calling on a data store that isn't immediately updated in the production cluster, so it always shows the protection twice.

Looking at that page history I see the two protection actions in the same minute, but for the later edit at 9:15 there's no additional protection in the log.

Ejegg added a comment.Dec 14 2018, 4:57 PM

OK, I guess we're NOT actually saving twice in the same form submit, and I see doubled protections in the logs up to two minutes apart. My new theory is that it's due to DB replica delay, since Title::loadRestrictions always goes to the replica. It also caches restrictions, but with a key tied to the revision ID, which should be different on the second save / protect.

• MarcoAurelio added a comment.Dec 14 2018, 5:10 PM

Hello. The same happens with {{{text}}} banner content, cfr. https://meta.wikimedia.org/w/index.php?title=MediaWiki:Centralnotice-WMCO_Wikivacaciones2018-text2/es&action=history and protection log). Thanks for investigating this issue. Best regards.

gerritbot added a comment.Dec 14 2018, 8:08 PM

Change 479752 had a related patch set uploaded (by Ejegg; owner: Ejegg):
[mediawiki/core@master] Option to load restrictions from DB_MASTER

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

Ejegg claimed this task.Dec 18 2018, 12:11 AM
Ejegg triaged this task as Normal priority.
gerritbot added a comment.Dec 18 2018, 3:24 PM

Change 479752 merged by jenkins-bot:
[mediawiki/core@master] Option to load restrictions from DB_MASTER

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

gerritbot added a comment.Dec 20 2018, 12:57 AM

Change 480574 had a related patch set uploaded (by Ejegg; owner: Ejegg):
[mediawiki/core@master] Pass READ_LATEST in $flags to Title::loadRestrictions

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

gerritbot added a comment.Dec 20 2018, 4:42 PM

Change 480990 had a related patch set uploaded (by Ejegg; owner: Ejegg):
[mediawiki/core@master] WikiPage::doUpdateRestrictions checks DB_MASTER

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

gerritbot added a comment.Jan 7 2019, 5:29 PM

Change 480574 merged by jenkins-bot:
[mediawiki/core@master] Pass READ_LATEST in $flags to Title::loadRestrictions

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

gerritbot added a comment.Jan 7 2019, 8:28 PM

Change 480990 merged by jenkins-bot:
[mediawiki/core@master] WikiPage::doUpdateRestrictions checks DB_MASTER

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

gerritbot added a comment.Jan 8 2019, 10:48 PM

Change 478257 abandoned by Ejegg:
Don't re-protect banner if it's already protected

Reason:
Core change was merged!

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

Ejegg moved this task from Pending Deployment to Done on the Fundraising Sprint A series of unfortunate event handlers board.Jan 16 2019, 4:25 PM

This is now deployed to all wikis. Now let's keep an eye on the protection log to see if it stops getting multiple entries

Ejegg closed this task as Resolved.Jan 22 2019, 10:11 PM

Looking at https://meta.wikimedia.org/wiki/Special:RecentChanges?namespace=8&limit=500&days=7&urlversion=2 I still see plenty of protection for formerly-unprotected translated messages, but I don't see any duplicates like we had before.

Ejegg set the point value for this task to 4.Jul 5 2019, 1:24 AM
Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or 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