| kaldari | |
| Aug 5 2018, 3:23 AM |
| F24905753: Screenshot from 2018-08-13 12-47-49.png | |
| Aug 13 2018, 7:50 PM |
| F24643952: Screen Shot 2018-08-07 at 12.55.05.png | |
| Aug 7 2018, 5:13 PM |
Since drafts use a different system for triaging, we should not remove drafts from Special:NewPagesFeed if PageTriage considers them "reviewed" (as this typically means someone just clicked on the patrol link). Instead, Special:NewPagesFeed should allow access to all drafts until they are deleted or published to main namespace.
This should be possible by simply adding showreviewed=1 to the API queries that Special:NewPagesFeed sends for requesting lists of drafts.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | MMiller_WMF | T193782 [EPIC] Prioritization tools for AfC and NPP | |||
| Resolved | MMiller_WMF | T196177 New Pages Feed: draft addition | |||
| Resolved | SBisson | T201253 Special:NewPagesFeed should ignore ptrp_reviewed status for Drafts |
I was just talking with @MusikAnimal who was saying that this code was written to ignore patrolled status in the first place. @MusikAnimal, could you comment on that?
Well first off, there are two things we're talking about -- the MediaWiki patrolled status, and ptrp_reviewed within PageTriage. As I recall both are supposed to be ignored within the draft queue. I think this is what I'm remembering:
But that only sets the reviewed status to 0 for drafts on page creation (regardless if they are autopatrolled). I'm guessing the issue is that if you "mark this page as patrolled", PageTriage also sets ptrp_reviewed to non-zero, and hence it disappears from the queue. If that's the case, it's definitely a bug, and something we overlooked. Sorry! A possible solution is to check if the page is a draft in PageTriage::setTriageStatus().
We can easily always specify showreviewed=1 and showunreviewed=1 for AfC as @kaldari suggested in the task description. It works but it shows the patrolled pages with the green icon. It stands out a lot for a concept that may be irrelevant for AfC.
The other option, suggested by @MusikAnimal, is to force ptrp_reviewed to stay 0 for draft. We would have to make sure all code paths that set this field are covered and old drafts be migrated.
@SBisson - Good point, I didn't think about the icons. Maybe MusikAnimal's idea is better.
Change 451182 had a related patch set uploaded (by Sbisson; owner: Sbisson):
[mediawiki/extensions/PageTriage@master] Keep patrolled drafts in the feed
Change 451183 had a related patch set uploaded (by Sbisson; owner: Sbisson):
[mediawiki/extensions/PageTriage@master] Keep patrolled drafts in the feed
I've implemented both approaches in the patches above. I don't think any of them is great since we keep special-casing drafts everywhere. I'm curious what other people think, which one, if any, can be merged.
I think I prefer the front-end approach to the back-end approach, but I agree neither is especially great. What I don't like about the back-end approach is that I think that keeping the data in the DB "correct" (i.e. make sure drafts never become reviewed) when there are many code paths writing to it feels futile, and it feels safer to instead ignore bad things at render time (ignoring the reviewed field for drafts).
I agree. Did you look at the patch? https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/PageTriage/+/451182/
Yes, I said that having just read both patches :) . Any objection to me merging the front-end one?
Change 451182 abandoned by Catrope:
Keep patrolled drafts in the feed
Reason:
Superseded by https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageTriage/ /451183
Change 451183 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@master] Keep patrolled drafts in the feed
Checked in betalabs - old drafts that had 'Mark as patrolled' link do not disappear anymore from AfC feed.
However, newly created drafts (created after the fix was merged) will not appear in AfC feed until 'Mark as patrolled' link is clicked.
@SBisson - is it desired behavior?
@Etonkovidova it is NOT the desired behavior. I can't reproduce it locally but I can in betalabs. Also, according to the version page, PageTriage is stuck in the past, running a version from 7 days ago.
@Catrope it looks like you merged the backend option and abandoned the frontend one. Was it intentional?
Change 451182 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@master] Keep patrolled drafts in the feed
I can't reproduce this now. I just created two drafts on beta labs, one as me and one as an anon (logged out), and they both showed up immediately:
I guess it's possible that me reverting Stephane's backend patch and merging the frontend one fixed it?
Yes, all seems ok now - this issue "Special:NewPagesFeed should ignore ptrp_reviewed status for Drafts" and T201661 are Resolved.
@SBisson, @Catrope: There's one more loose end I didn't notice until now: We need to update PageTriage/cron/updatePageTriageQueue.php to not automatically remove drafts older than 30 days (regardless of their ptrp_reviewed state). This is a cron script that is run daily in production to prune the pagetriage_page table. I'm reopening this ticket to handle it since it probably needs to be addressed immediately (since the new branch will be cut tomorrow).
I guess the obvious follow-up question is what should updatePageTriageQueue.php do with drafts? I guess nothing since even old drafts (that are still drafts) can potentially be re-reviewed.
Change 452665 had a related patch set uploaded (by Sbisson; owner: Sbisson):
[mediawiki/extensions/PageTriage@master] Don't cleanup old drafts, unless they are redirects
Change 452857 had a related patch set uploaded (by Catrope; owner: Sbisson):
[mediawiki/extensions/PageTriage@wmf/1.32.0-wmf.16] Keep patrolled drafts in the feed
Change 452857 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@wmf/1.32.0-wmf.16] Keep patrolled drafts in the feed
Change 452665 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@master] Exclude Drafts from daily cleanup of PageTriage queue