Page MenuHomePhabricator

Implement functionality for RC page 'Review status' filters
Closed, ResolvedPublic

Description

Provides filtering and/or highlighting of editor contributions based on whether the contribution has been reviewed or not. Find the authoritative interface text (filter names and descriptions) for these at T149385. There are two filters in the group:

Patrolled*
This finds contributions that have been automatically patrolled or marked as Patrolled by qualified users.

Unpatrolled*
This finds everything that is not in the above category (including logged actions, category edits, etc., which users can screen out using other filters if desired).

About the functionality of these filters

  • *Names and descriptions for these filters are pending discussion in T149385
  • The current RC page shows users with patrol rights the ability to hide/show patrolled edits. This filter should work similarly, in that it will appear only for editors who have the appropriate rights.
  • The current hide/show patrolled functionality can (probably) be used as the basis for these filters, though any hidden functions should be removed.
  • Like all filters in the enhanced RC page filter UI, these filters conform to a set of rules that are, in some ways, very different from the existing RC page filters. The existing filters are designed primarily to EXCLUDE selected properties (in this case, my edits). The new filters are designed to INCLUDE those properties; logically, each new group is a set of OR filters (with each group of ORs being connected to other groups by ANDS). Which means the filters in this group follow these rules:
    • To INCLUDE property A, users check the box for property A.
    • To EXCLUDE property A, the user must uncheck A and check it's complement, property B.
    • If NEITHER A nor B are checked, then BOTH are included.
    • If BOTH A and B are checked, then the result is the same: BOTH are included.
  • As per T146076, searches on the RC page are meant to be bookmarkable. Please make sure your search adds query strings to the URL.
  • PLEASE NOTE: because English is unique in using the word "patrol" for the patrol flag, please add language to the qqq file explaining that translators should be sure that they use the right word to name these filters, instead of mechanically translating "patrol." (E.g., if Portugese calls the patrol flag the "revisão bandeira"--I don't know what they call it, but supposing they might-- then the filters should be called something that would conform with that.)

Event Timeline

Change 324784 had a related patch set uploaded (by Sbisson):
Special:RC filter: hideNonPatrolled

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

jmatazzoni updated the task description. (Show Details)
jmatazzoni updated the task description. (Show Details)

Change 324784 merged by jenkins-bot:
Special:RC filter: hideunpatrolled

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

Unpatrolled*
This finds everything that is not in the above category (including logged actions, category edits, etc., which users can screen out using other filters if desired).

In general, it's the opposite (they *do* start patrolled) for log entries, but It's a little more complicated. Some log entries are patrollable (apparently just the upload log, but there is general support). Logged entries start out:

  • Patrolled - If either the user has 'autopatrol' (they patrol their own edits and log entries), or the log entry type is not patrollable.
  • Unpatrolled - Otherwise.

So almost all log entries (non-upload) are automatically marked patrolled, because actual patrolling is not supported for them.

Category changes (RC_CATEGORIZE) always start patrolled, no exceptions (presumably because the underlying edit, separate in the RC feed, is generally patrollable)

QA recommendation: Product should weigh in.

Existing filter, hidepatrolled defines an edit as patrolled, if the edit needs to be reviewed( patrolled). Thus, the edits are marked with ! and hidepatrolled works simply as a toggle, i.e. hidepatrolled=1 hides all edits or other entries that do not have ! mark.

So, hideunpatrolled filter works as a reverse filter to hidepatrolled=1
hideunpatrolled, in fact, will display either all RC records (hideunpatrolled=0) or it will filter out edits that are marked with ! (hideunpatrolled=1).

Comparing with the already existing filter filter hidepatrolled - tested on cawiki in betalabs and in production.

Filter nameRecords with '!' are displayed?Non-edits displayed?Edits without '!' are displayed?
hidepatrolled=0YesYesYes
hidepatrolled=1YesNoNo
hideunpatrolled=0YesYesYes
hideunpatrolled=1NoYesYes

@jmatazzoni - are we going to keep hidepatrolled filter along with hideunpatrolled filter? If so, we should clarify in documentaion that hideunpatrolled filter just filters out edits that have !, not the action that is described in https://gerrit.wikimedia.org/r/#/c/324784/

Allows hiding edits that have not been patrolled.
In other words, showing only edits that have been patrolled.

@jmatazzoni - are we going to keep hidepatrolled filter along with hideunpatrolled filter? If so, we should clarify in documentaion that hideunpatrolled filter just filters out edits that have !, not the action that is described in https://gerrit.wikimedia.org/r/#/c/324784/

Allows hiding edits that have not been patrolled.
In other words, showing only edits that have been patrolled.

Both filters should be kept. Patrolled is a boolean field. We need one parameter to hide the entry where it is true, and another parameter to hide the entries where it's false. Only with both we can show exactly what we want (patrolled, unpatrolled, all, none). I'm not sure how it contradicts the two lines above from the patch commit message but with all those negations it might short-circuit my brain ;)

So, if I understand the question correctly, the issue is that when I say I want to see "unpatrolled," the new system will show me a mix of stuff that has the exclamation point and stuff that doesn't. This is because mediawiki now is making a decision about what types of stuff I may or may not want to patrol, and is marking only the former with the ! mark. (And, furthermore, in the old system, when I said "hide patrolled," I was actually hiding a) everything that was patrolled and b) everything mediawiki decided I didn't need to patrol.) @SBisson, can you tell me what that stuff is? I.e., what classes of content that are NOT patrolled are being screened out automatically when a current RC page user asks to "hide patrolled"?

As to whether we want to adjust our filter functionality to a) make it work as before or b) add a separate filter/class called "no patrol needed," I suspect the answer is no. I will be interested to know what all the "No patrol needed" material is. But I believe we will want to leave this function as we've currently defined it.

In the new system, I suspect that we have provided filters that will enable users to isolate what they want -- probably in the Type of Change section. And I suspect @Pginer-WMF will agree that we are in the business here of removing hidden and duplicate functions, rather than adding them in—even though it's a change that arguably makes life temporarily more complicated for some users. (Although, having said that, I could imagine a tripartite system of "Patrolled, No patrol needed, Needs patrol_--if we agree with what goes in the second bucket.)

@SBisson - yes, all those negations and toggle actions are not easy :) I guess I misunderstood the definition of "edits that have been patrolled" - I'd prefer to call them the edits with "!". Anyway, after reviewing the table above, it seems that hidepatrolled and hideunpatrolled filters work logically correct.

It may be important to note here that the interface language for these has been scaled back here so that we no longer label the unpatrolled items "Pending review." The approved language makes only a more modest and strictly accurate claim, as below:

Review status

Patrolled
Edits marked as patrolled.

Unpatrolled
Edits not marked as patrolled.

So, if I understand the question correctly, the issue is that when I say I want to see "unpatrolled," the new system will show me a mix of stuff that has the exclamation point and stuff that doesn't. This is because mediawiki now is making a decision about what types of stuff I may or may not want to patrol, and is marking only the former with the ! mark. (And, furthermore, in the old system, when I said "hide patrolled," I was actually hiding a) everything that was patrolled and b) everything mediawiki decided I didn't need to patrol.) @SBisson, can you tell me what that stuff is? I.e., what classes of content are being screened out automatically when a current RC page user asks to "hide patrolled"?

It's simpler than that.

Every row has rc_patrolled that can only have the value True or False. The filters correspond to those two values so by combining them, you can show only true, only false, both, or neither. there is no "old vs. new" system. We used to only have hidepatrolled, now we've added hideunpatrolled, which is the flip-side. We've done the same thing for all other binary fields (bot, minor, ...).

Now, a row with rc_patrolled=True can be an edit that was later patrolled, an edit create by a user with auto-patrolled right, or something that cannot be patrolled. There's no way to know by looking at the data.

As to whether we want to adjust our filter functionality to a) make it work as before or b) add a separate filter/class called "no patrol needed," I suspect the answer is no. I will be interested to know what all the "No patrol needed" material is. But I believe we will want to leave this function as we've currently defined it.

Probably log entries. I don't know what else.

@SBisson, I'm sorry, but I don't understand your answer. To put the questions more simply:

  • Do the filters Patrolled and Unpatrolled together account for all edits on RC page?
  • If not—if there is some "no need to patrol" category that the RC page, in its wisdom, is hiding from us—what types of edits are in it?
  • And finally, if there is some third category in the current code, can we eliminate it so that every edit is either Patrolled and Unpatrolled, as intended?

@SBisson, I'm sorry, but I don't understand your answer. To put the questions more simply:

  • Do the filters Patrolled and Unpatrolled together account for all edits on RC page?

Yes, together they account for all rc entries. There is no third state and nothing is hidden. You can stop reading here.

  • If not—if there is some "no need to patrol" category that the RC page, in its wisdom, is hiding from us—what types of edits are in it?

Conceptually, log entries do not need to be patrolled so they are always flagged with patrolled=True. In RC world, we cannot tell if something is patrolled or cannot be patrolled but nothing is hidden. If you ask for everything that is patrolled, you also receive stuff that cannot be patrolled.

  • And finally, if there is some third category in the current code, can we eliminate it so that every edit is either Patrolled and Unpatrolled, as intended?

Every single RC entry, not just edits, is either patrolled or unpatrolled.

@SBisson I was doing additional comparison checking for patrolled/unpatrolled for enwiki and cawiki.
cawiki has 'Hide/Show patrolled edits' options displayed on RC. cawiki (betalabs) displays the exclamation mark for recentchanges table records that have rc_patrolled=0
(1) patrolled=1 -- all records with rc_patrolled=0 (for the selected date range, of course) are displayed.
(2) unpatrolled =1 displays all records with rc_patrolled=1

Now, enwiki (betalabs) does not have 'Hide/Show patrolled edits' options displayed on RC(?). Although, the same description for the mark ! is displayed in the legend, the exclamation mark is applied only to the new pages created by non-autopatrolled users, not to the edits as in cawiki. Hence, many records have rc_patrolled=0 but the exclamation mark is not displayed for them in UI. Then, the displayed results with patrolled=1 and unpatrolled =1 are not so easily understood.

Questions:

  • Will patrolled/unpatrolled filters take into account custom ways that different wikis use the exclamation mark to mark actions that needs to be patrolled?
  • Will patrolled/unpatrolled filters be displayed to users who do not have rights to see patrolled edits or to anon? Presently anons and users without special privileges do not see ! marks.
  • Or all the differences between cawiki and enwiki are just limitations of betalabs and the real wikis have more logic in patrolling?

Questions:

  • Will patrolled/unpatrolled filters take into account custom ways that different wikis use the exclamation mark to mark actions that needs to be patrolled?

Like you've noted in your recent test results, the filters define what data is returned, not when or why the "!" is displayed.

  • Will patrolled/unpatrolled filters be displayed to users who do not have rights to see patrolled edits or to anon? Presently anons and users without special privileges do not see ! marks.

We talked about showing the filters to everyone, not just those with the power to patrol as is currently the case, but I don't know if we've officially made that decision. Better to check with Roan/Joe or maybe one of the UI ticket would specify this.

  • Or all the differences between cawiki and enwiki are just limitations of betalabs and the real wikis have more logic in patrolling?

@Catrope can tell you how (il)logical patrolling is in various wikis.

Thx @SBisson!

@jmatazzoni: please review @SBisson answers above.

QA recommendation: Product should weigh in.

  • Will patrolled/unpatrolled filters be displayed to users who do not have rights to see patrolled edits or to anon? Presently anons and users without special privileges do not see ! marks.

We talked about showing the filters to everyone, not just those with the power to patrol as is currently the case, but I don't know if we've officially made that decision. Better to check with Roan/Joe or maybe one of the UI ticket would specify this.

.In answer to Stephane's question above, no, we decided NOT to change the way the patrol flag and filter work or on respective wikis. Which means, I believe, that all the behavior Elena describes is correct. I tried to capture the desired functionality in the spec above when I wrote:

The current RC page shows users with patrol rights the ability to hide/show patrolled edits. This filter should work similarly, in that it will appear only for editors who have the appropriate rights.

But it appears I didn't fully understand the complexity of the current wiki-specific behaviors (e.g., re. visibility of the !). If you'd like to suggest who this spec could be rewritten to be more accurate, please do. But bottom line, we're not at this time changing how any wiki treats the patrol flag.

@Etonkovidov, does that mean I can Resolve this ticket?