Page MenuHomePhabricator

DynamicPageList (Wikimedia) no longer integrates properly with flagged revisions
Closed, ResolvedPublicBUG REPORT

Description

tl;dr: The stablepages and qualitypages paramters are being ignored by DynamicPageList. Cause seems to be removal of FLAGGED_REVISIONS constant from flaggedrevs extension, preventing dynamicpagelist from detecting that flaggedrevs is installed.


Hello and thank you for your work.

Russian Wikinews suddenly encountered a problem that was not known before. It is related to patrolling ("flagged revisions").

The website is programmed to schedule new articles, created by general public, for "review" by admins (also "editors"). After "review" it gets "published" or not. It only should get "published" when an admin *patrols* it and then adds template {{yes}} (which is a redirect to {{публиковать}}). "Publication" generally means placing article to category https://ru.wikinews.org/wiki/Категория:Опубликовано (there are more details, but let's be simple now).

If a non-admin adds {{yes}} to the article, there should be NO publication despite this template because the article is NOT patrolled.

This worked always before but suddenly stopped working now. We don't know why and suggest it was related to server software updates.

Please help to fix.

We have a test anonymous article: https://ru.wikinews.org/?curid=509000 — it shouldn't be published (unpatrolled), but it is published.

  • Steps to Reproduce:

Create an anonymous article with any text and place {{yes}} within.

  • Actual Results:

Article is "published" (placed into category "Категория:Опубликовано") despite it is not *patrolled*.

  • Expected Results:

Article is not "published" because it is not *patrolled*.

Event Timeline

ssr created this task.Sat, Oct 5, 12:24 PM
Restricted Application added subscribers: Liuxinyu970226, Aklapper. · View Herald TranscriptSat, Oct 5, 12:24 PM
Aklapper updated the task description. (Show Details)Sat, Oct 5, 12:44 PM

Hmm, what do these templates do exactly and how is this problem related to the patrolling code in the MediaWiki software?

It sounds like there are two different and separate steps:

It only should get "published" when an admin *patrols* it and then adds template {{yes}} [...] If a non-admin adds {{yes}} to the article, there should be NO publication despite this template because the article is NOT patrolled.

Why do you perform the first step ("an admins patrols it"), if you state that only the second step (using a template) actually makes the article "patrolled"?

ssr added a comment.Sat, Oct 5, 2:12 PM

Patrolling is needed for several other things like approving non-editor edits to articles and to comments — there are "Комментарии:" space for comments like here https://ru.wikinews.org/wiki/Комментарии:Фонд_Викимедиа_зарегистрировал_ячейку_в_Санкт-Петербурге they are embedded at the bottom of each article. Also there is patrolling of categories and something more.

"Publication" should mean only combination of {{yes}} and a patrol. Because anonymous user who place {{yes}} without patroller approval should not be allowed to be published. Anonymous don't have patrol rights, and his {{yes}} should not publish article — but it is published now, and this is the error.

@Krassotkin is now the subscriber here, he knows more details because he did some scripts, and I am only an admin who is not able to write scripts. He will tell more.

ssr added a comment.Sat, Oct 5, 2:32 PM

Also there is this: "unpublished" articles should not appear in these side boxes: https://ru.wikinews.org/wiki/Фонд_Викимедиа_зарегистрировал_ячейку_в_Санкт-Петербурге — boxes with violet headers to the right from the text (2 violet headers). This unpatrolled article now appears in both — at the top (compare the title).

I can't tell exactly how a combination of not-patrolling and placing in category "Категория:Опубликовано" correctly excludes articles from "publication". Let's wait for @Krassotkin to answer.

"Publication" should mean only combination of {{yes}} and a patrol.

Does that imply that there is no problem with MediaWiki's Patrolling in this task, but "only" with "publication" instead, which seems to be based on local templates?

ssr added a comment.Sat, Oct 5, 5:02 PM

"Publication" should mean only combination of {{yes}} and a patrol.

Does that imply that there is no problem with MediaWiki's Patrolling in this task, but "only" with "publication" instead, which seems to be based on local templates?

May be! This is when my competences fail, I judge as an user, not developer. But if nothing was locally changed, how could that happen? @Krassotkin didn't tell me he changed something (no one else here could), he only was surprised and asked to help with this task and explain in English which is not native to both of us.

"Published" ({{yes}}+patrolled) news appear here https://ru.wikinews.org/wiki/Категория:Опубликовано — if some 1 of those 2 are absent, it should not appear.

Sasha also said RSS is affected and gave link https://ru.wikinews.org/w/index.php?title=Служебная:NewsFeed&feed=atom&categories=Фонд_Викимедиа%7CОпубликовано&notcategories=Не%20публиковать&namespace=0&count=15&ordermethod=categoryadd&stablepages=only (even harder to understand!)

ssr added a comment.Sat, Oct 5, 5:22 PM

I just can now add that patrolling also affects comments https://ru.wikinews.org/wiki/Комментарии:Фонд_Викимедиа_зарегистрировал_ячейку_в_Санкт-Петербурге

It is connected in a complicated way (article patrol + comment patrol = allowing/disallowing newer comments/discussions display) and I tried to test some now an got totally lost for now, sorry. Maybe later I can try more and conclude some better explanations.

ssr added a comment.Sat, Oct 5, 5:54 PM

Oh, yes, feel free to make test edits if you like. As long as it's not late night here (Moscow time), I will try to help with it as I can

Neolexx added a subscriber: Neolexx.Sat, Oct 5, 6:57 PM

I don't see so far how it could be related. Unless we group by the criterion "something strange all suddenly started to happen" but it would be a supercategory for 99% of bugs at the phab :-)

The test article has been never "patrolled" (reviewed in FlaggedRevs terms) - so it is not "patrolled":

And some other article has been "patrolled" since its creation, because created and edited only be autoreview rights holders:

It means nobody "patrolled" the later news, it has been naturally born that way.

It is all correct expected behavior. Maybe Krassotkin does "stabilisation" actions - but it doesn't look like. And in any case Krassotkin is not alone who can do it.

And Публиковать template does exactly one thing: adds the page to category Опубликовано if the page is not in the main namespace: {{#if:{{NAMESPACE}}||[[Категория:Опубликовано]]}}
The purpose of it is unknown to me but it definitely has nothing to do with patrolling or stabilisation and any user can do such edit.

ssr added a comment.Sat, Oct 5, 8:40 PM

Well, I have updates now.

The problem is in DynamicPageList extension. ( {{yes}} template may be ignored now )

DynamicPageList extension directly influences "publication" process.

By default, DynamicPageList showed patrolled articles and didn't show unpatrolled.

When things broke recently, DynamicPageList began to show ALL pages whenever they are patrolled or not.

Patrolling no more controls DynamicPageList on/off. That is the problem.

DynamicPageList should be fixed to SHOW patrolled and IGNORE unpatrolled articles. AS IT WAS.

All local lists, categories, robots, RSS depend on this, that's why it needs to be fixed ASAP.

Arbnos added a subscriber: Arbnos.Sat, Oct 5, 10:50 PM
Neolexx added a comment.EditedSun, Oct 6, 10:13 AM

The problem is in DynamicPageList extension. ( {{yes}} template may be ignored now )

As the extension doc states:
stablepages determines whether or not to include stable (flagged) pages when using Extension:FlaggedRevisions. The value can be exclude (don't list), include (list stable and non-stable. default), or only (only list stable pages). Requires FlaggedRevs to be installed to work.”

As your publishing process described, any related DynamicPageList call needs to have stablepages=only parameter. In the sample template such parameter is missing. So it acts default (list stable and non-stable) as expected and documented.

Unless I missed something important I would propose to close this ticket as "not a bug but a documented feature".

ssr added a comment.Sun, Oct 6, 11:04 AM

In response, Sasha keeps being angry and rejects your proposal to close ticket. He is wondering why 14 years things were normal and now they have broken and why do you say they are still normal while they are in fact broken.

...
I would propose to close this ticket.

I'm very grateful for your desire to help. But we do not need good advice, all the more erroneous. Please see page and code https://ru.wikinews.org/?curid=1345 for the same not reviewed news "Фонд Викимедиа зарегистрировал ячейку в Санкт-Петербурге" (https://ru.wikinews.org/?curid=509000).

All we need is for the developers to correct their mistake and everything starts working the same way as many years before.

The lack of editorial review is very critical for Wikinews. This bug should be fixed ASAP.

Koavf triaged this task as Unbreak Now! priority.Sun, Oct 6, 12:32 PM
Koavf added a subscriber: Koavf.

I escalated the status--this is pretty vital to how ru.wn functions.

Neolexx added a comment.EditedSun, Oct 6, 12:40 PM

To be clear I am not in any relations with the WM development team. I am not even a programmer (by my diploma). So my closing proposal doesn't have any authoritative weight. But honestly the way the problem has been last time described - it was a bit like "we used to use DynamicPageList against of its documented settings for desired results; now it refuses to work against of the documented settings - please fix it".

So it should be more light shed on our news publication steps to understand where and what became different.

Maybe indeed your FlaggedRevs became screwed somewhere. Or some DynamicPageList update. I tried a few category listings using stablepages = exclude, only and even played with qualitypages -
https://ru.wikinews.org/wiki/Обсуждение_участника:Neolexx/test
source: https://ru.wikinews.org/w/index.php?title=Обсуждение_участника:Neolexx/test&action=raw

The lists are all the same while the first article was never reviewed yet: [https://ru.wikinews.org/w/api.php?format=xmlfm&action=query&prop=revisions&titles=Конференция%20по%20поводу%20Большой%20Книги%2026%20сентября%202019%20года&rvlimit=max&rvprop=ids|timestamp|user|flagged

So maybe somehow FlaggedRevs stopped worked. Or DynamicPageList stopped support FlaggedRevs. Or I misread DynamicPageList docs.

Aklapper lowered the priority of this task from Unbreak Now! to Needs Triage.Sun, Oct 6, 9:57 PM
Aklapper removed a project: MediaWiki-Patrolling.

I still do not see how this problem is related to the patrolling code in the MediaWiki core code base itself if you can trigger that behavior already by writing the name of a template into a wiki page. Hence I'm removing MediaWiki-Patrolling.

Given that this problem only happens on one out of >900 sites so far, and given the number of changes I'm resetting the Priority level for now. (Also see https://www.mediawiki.org/wiki/Phabricator/Project_management#Setting_task_priorities )

As it was mentioned in T234715#5549402, I don't think that DynamicPageList (Wikimedia) *code* is related here. There are no recent relevant changes lately in https://phabricator.wikimedia.org/diffusion/EAND/history/master/

For recent FlaggedRevs changes, see https://phabricator.wikimedia.org/diffusion/EFLR/history/master/ , however note that software changes of "the last seven days" get deployed on Wednesdays to ru.wikinews as per https://wikitech.wikimedia.org/wiki/Deployments/One_week

I still do not see how this problem is related to the patrolling code in the MediaWiki core code base itself if you can trigger that behavior already by writing the name of a template into a wiki page. Hence I'm removing MediaWiki-Patrolling.

I think its just a language barrier issue, and by "patrolling" the user actually means what we usually refer to as flagged revision review, and not RC patrol.

I think what the user is reporting, is that in dynamicPageList extension, the stablepages=only parameter, which should exclude pages not managed by flagged revisions, is no longer working.

The default value of stablepages is 'exclude', since the very beginning (f798ae02d418f63301da636d1d21719ead743773).

The main code of DynamicPageList (aka. Intersection) has been unchanged for years, though.

As an alternative solution, you could add an AbuseFIlter that prevents non-admins from adding {{yes}}

ssr added a comment.EditedMon, Oct 7, 1:01 AM

There are also symptoms that I suppose may be "side-effects" but I'm not sure.

We have "patrolling" (flagged reviewing) of categories, which generally not only contain pages and subcategories, but also extra design implemented by adding {{Оформление категории}} at the top of category source wiki-code. Since current issues came, I have noticed that I have to "flagged review", when needed, a category TWICE (instead of once). I see "unreviewed change" notification at the top, I press "review", but _notification persists_. If I look into history, I see review was made (current version is light-blue). But there is a notification about need to review! If I press "review" once again, notification finally disappears.

( an example of this may be https://ru.wikinews.org/wiki/Категория:Александр_Кержаков but the symptom is fragile I don't know if you can reproduce exactly what I see )

Also, some service notifications became of yellow background (were of other color I can't recall exactly). Primarily, edit preview notifications.

ssr added a comment.Mon, Oct 7, 1:49 AM

And one more. As I said, we have comments attached at each article. When "not broken", each new comment in comments had to be "flag-reviewed" because only "approved" users saw it and unregistered users didn't see until "review". The need for "review" was not exclusively for comments, it could also work with articles (a new comment caused notification on whole article).

That was not intended, this situation came technically by itself. In fact, that meant _pre-moderation_ of comments. So that, "third-party" visitors, who commented, couldn't see their comments until an "admin" pressed review. Admin could press it not instantly, and real-time discussions were generally impossible.

After "things broke", "reviewing" seem to no longer affect comments, and they now seem to appear instantly, so real-time comments became possible!

AND THIS IS GOOD! We do not want pre-moderation of comments, we want visitors to talk in real-time! We can apply post-moderation if needed.

Bawolff added a comment.EditedMon, Oct 7, 5:16 AM

Ok, so not exactly sure if the stablepages=only issue is what is being reported here or not, but that issue appears to be caused by d3259c2b7ca12d / 4eba7cf0f3e4, which (Accidentally I assume) removed the FLAGGED_REVS constant, which dynamic page list was looking for to decide if to enable flagged revs integration

Change 541140 had a related patch set uploaded (by Brian Wolff; owner: Brian Wolff):
[mediawiki/extensions/intersection@master] Fix detection of FlaggedRevs

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

Koavf removed a subscriber: Koavf.Mon, Oct 7, 5:36 AM
Bawolff added a comment.EditedMon, Oct 7, 5:58 AM

Well, I have updates now.
The problem is in DynamicPageList extension. ( {{yes}} template may be ignored now )
DynamicPageList extension directly influences "publication" process.
By default, DynamicPageList showed patrolled articles and didn't show unpatrolled.
When things broke recently, DynamicPageList began to show ALL pages whenever they are patrolled or not.
Patrolling no more controls DynamicPageList on/off. That is the problem.
DynamicPageList should be fixed to SHOW patrolled and IGNORE unpatrolled articles. AS IT WAS.
All local lists, categories, robots, RSS depend on this, that's why it needs to be fixed ASAP.

So my patch should fix this issue (Patch not live yet. Should be live on Wednesday). It sounds like in later comments, you are describing some additional issues with FlaggedRevs, which sound unrelated to the above issue to me. Just to keep things more clear, can you file separate bugs for any of those issues if they are causing problems.

The default value of stablepages is 'exclude', since the very beginning (f798ae02d418f63301da636d1d21719ead743773).
The main code of DynamicPageList (aka. Intersection) has been unchanged for years, though.

Just as a point of clarity. The default is include if no stablepages property is specified. However if one is specified, and the value for the stablepage property is not recognized, it is treated as exclude. This is super confusing and certainly seems wrong (Like actual wtf?)

However, in terms of this bug, I think ru.wikinews is using a template to call dynamicpagelist, which behind the scenes as stablepages=exclude. So it seems like a default to ruwikinews folks, because the template always adds it.

Bawolff renamed this task from Russian Wikinews has sudden patrolling problem to DynamicPageList (Wikimedia) no longer integrates properly with flagged revisions.Mon, Oct 7, 6:05 AM
Bawolff updated the task description. (Show Details)

...
So my patch should fix this issue (Patch not live yet). It sounds like in later comments, you are describing some additional issues with FlaggedRevs, which sound unrelated to the above issue to me. Just to keep things more clear, can you file separate bugs for any of those issues if they are causing problems.

Above, ssr writes about oddities that appeared a few days ago (weeks?). We associate them with this ticket as side effects. But maybe this is something else. We can find out it when the patch will be connected.

The default value of stablepages is 'exclude', since the very beginning (f798ae02d418f63301da636d1d21719ead743773).
The main code of DynamicPageList (aka. Intersection) has been unchanged for years, though.

Just as a point of clarity. The default is include if no stablepages property is specified. However if one is specified, and the value for the stablepage property is not recognized, it is treated as exclude. This is super confusing and certainly seems wrong (Like actual wtf?)
However, in terms of this bug, I think ru.wikinews is using a template to call dynamicpagelist, which behind the scenes as stablepages=exclude. So it seems like a default to ruwikinews folks, because the template always adds it.

Very strange. Maybe this should be described in the documentation or to make behavior more unambiguous?

In any case, the code of this extension needs to be reworked. All Wikinews and some other project use it. But it looks outdated. It lacks some of the necessary features. For example, DPL does not show the union of categories (now only the intersection), etc. But here it is off-topic, of course, sorry.

Well, we are waiting for the patch to be connected. Thank you!

Change 541140 merged by jenkins-bot:
[mediawiki/extensions/intersection@master] Fix detection of FlaggedRevs

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

I think its just a language barrier issue, and by "patrolling" the user actually means what we usually refer to as flagged revision review, and not RC patrol.

Ah, thank you, Bawolff!

Kaganer added a subscriber: Kaganer.Tue, Oct 8, 4:26 PM
ssr added a comment.Wed, Oct 9, 9:50 AM

It sounds like in later comments, you are describing some additional issues with FlaggedRevs, which sound unrelated to the above issue to me. Just to keep things more clear, can you file separate bugs for any of those issues if they are causing problems.

Thank you for this! We are considering discussing it at our forum and later file some more reports, but they are not urgent, this is just for information.

Should be live now. Please let me know if that fixed the problem

Should be live now. Please let me know if that fixed the problem

All my tests are passed. Thank you!

I suggest to wait a bit. Maybe someone else will check.

Bawolff closed this task as Resolved.Thu, Oct 10, 6:29 PM
Bawolff claimed this task.