Page MenuHomePhabricator

Use object stash for persisting last-use proprety to control curation toolbar display
Closed, ResolvedPublic

Description

As was reported in Wikipedia talk:Page Curation#New issue?, the curation toolbar is not present on pages that are supposed to have it.

When testing for possible scenarios/causes for the inconsistent behavior of the curation toolbar, I found two cases when the toolbar is not present:

(1) If a user is logged for sometime and has Special:NewPagesFeed open, then clicking on 'Review' will display a page without toolbar.
To work around it - a user may re-login or click on 'Curate this article' link on the left side nav panel.

(2) (seems to be specific to User namespace only)

  • create a page in User namespace and make sure that it shows in NPP feed - click on 'Review', the page will display the curation toolbar.
  • go to either Contributions or to Recent changes - locate the same page - click on a page, the page won't display the curation toolbar.

Per discussion with Aaron, Roan and Stephane, we will rely on a combination of query parameters (always show if clicking "Review" from the main list UI) and WAN cache. It may make sense to keep the localStorage check since the code is already there, so on any given article we would:

  1. Check if query parameter for showcurationtoolbar is set, show the toolbar. If the parameter is not set, go to next step.
  2. Check localStorage and compare the last-use param to current timestamp. If localStorage doesn't have this property set, then go to next step.
  3. Check the WAN cache for last-use for a given user ID. If that's not set, then don't show the toolbar.

On the NewPagesFeed page, when listControlNav finishes initializing we will want to update the cache with the user's last use info and set a 24 hour TTL.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 9 2018, 10:41 PM
kostajh claimed this task.Oct 10 2018, 3:57 PM
kostajh added a subscriber: kostajh.

Some background:

The root of the problem is the set of changes we implemented in this patchset https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageTriage/+/446517 for issue T154719, specifically per the discussion in patchset 4 https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageTriage/+/446517/4#88

The way things used to work was:

  1. User with NPP rights visits Special:NewPagesFeed
  2. On the server side, we set a property for pagetriage-lastuse in the user_properties table with the current timestamp
  3. When viewing articles, on the server side, we had code that would check the user's options to see what the value of pagetriage-lastuse was. If it was not within the last 24 hours, then a JS config variable called $wgPageTriagelastUseExpired was set to true or false
  4. On the client-side, when a user viewed an article, the JS would check the value of wgPageTriagelastUseExpired and then show/hide the curation toolbar.

As discussed in T154719, writing to the user_properties table in the context of a GET request is frowned upon. The first iterations of the patchset to fix this involved updating user_properties via a POST request after listControlNav.jswas initialized. We eventually discarded this idea due to performance concerns.

Instead, we first tried storing the pagetriage-lastuse property in the server-side session, in T203273. This had the appearance of working in local development but did not work reliably across longer periods of time, apparently because we should not be relying on server-side sessions to persist information like this.

Next, we implemented the storage of pagetriage-lastuse in localStorage (also in T203273). LocalStorage will persist the pagetriage-lastuse information in a single browser window (not new tabs/windows), so this led to problems where a user clicks "Review" on Special:NewPagesFeed and they're taken to a new window where pagetraige-lastuse is not set in localStorage, so the toolbar doesn't load. Again in T203273 (patchset https://gerrit.wikimedia.org/r/458550) we made a further fix to set a query parameter so that clicking "Review" from Special:NewPagesFeed would take you to /wiki/SomeTitle?showcurationtoolbar=1.


Overall, it seems like users expect to be able to navigate to articles in any window/tab and have the toolbar show up if they've used Special:NewPagesFeed within the last 24 hours, and our combination of query parameter and localStorage isn't accomplishing this.

AFAICT, after the changes we've cycled through, it seems like the most reliable is to once again store pagetriage-lastuse in the user_properties table, update this value via a POST on listControlNav.js, and conditionally set $wgPageTriagelastUseExpired based on a server-side calculation in the article footer view hook.

There were concerns expressed in 446517/4#88 about performance implications of POSTing this value on each initialization of the listControlNav, but IMO it would be better to do it this way as it will be the most reliable for end users.

[...]
AFAICT, after the changes we've cycled through, it seems like the most reliable is to once again store pagetriage-lastuse in the user_properties table, update this value via a POST on listControlNav.js, and conditionally set $wgPageTriagelastUseExpired based on a server-side calculation in the article footer view hook.

That sounds reasonable to me.

There were concerns expressed in 446517/4#88 about performance implications of POSTing this value on each initialization of the listControlNav, but IMO it would be better to do it this way as it will be the most reliable for end users.

So this would be adding 1 API call every time Special:NewPagesFeed is accessed. That doesn't seem excessive.

Was pagetriage-lastuse refreshed when the toolbar was being used to review articles but the special page was not revisited?

MMiller_WMF moved this task from Inbox to Upcoming Work on the Growth-Team board.Oct 10 2018, 5:25 PM
MMiller_WMF moved this task from Upcoming Work to Needs Discussion/Analysis on the Growth-Team board.

@aaron - Any thoughts about the course of action suggested above in T206580#4655234?

aaron added a comment.Oct 10 2018, 6:38 PM

@aaron - Any thoughts about the course of action suggested above in T206580#4655234?

If you update via POST, why not use the main object stash (which is persistent/DC-replicated). That would be faster and would avoid increasing user row CAS exceptions.

Roan, Stephane and I agree with trying the WAN cache approach.

kostajh renamed this task from Curation toolbar issues to Use WAN cache for persisting last-use proprety to control curation toolbar display.Oct 15 2018, 1:50 PM
kostajh updated the task description. (Show Details)

Change 467426 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/PageTriage@master] Use WAN Object Cache for keeping track of PageTriage last use

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

kostajh renamed this task from Use WAN cache for persisting last-use proprety to control curation toolbar display to Use object stash for persisting last-use proprety to control curation toolbar display.Oct 17 2018, 2:04 PM

Change 467426 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@master] Use Main Object Stash for keeping track of PageTriage last use

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

Change 468380 had a related patch set uploaded (by Catrope; owner: Kosta Harlan):
[mediawiki/extensions/PageTriage@wmf/1.32.0-wmf.26] Use Main Object Stash for keeping track of PageTriage last use

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

Change 468384 had a related patch set uploaded (by Catrope; owner: Kosta Harlan):
[mediawiki/extensions/PageTriage@wmf/1.32.0-wmf.26] Use Main Object Stash for keeping track of PageTriage last use

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

Change 468380 abandoned by Catrope:
Use Main Object Stash for keeping track of PageTriage last use

Reason:
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageTriage/ /468384

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

Change 468384 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@wmf/1.32.0-wmf.26] Use Main Object Stash for keeping track of PageTriage last use

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

At https://en.wikipedia.org/w/index.php?title=Wikipedia:Village_pump_(technical)&oldid=864788191#How_can_I_permanently_hide_the_page_curation_rightside_toolbar we have at least one user reporting that they are no longer able to persistently DISMISS the triage toolbar, in that it keeps reappearing when unwanted.

@Xaosflux I just posted a reply there: https://en.wikipedia.org/w/index.php?title=Wikipedia:Village_pump_(technical)&type=revision&diff=864789837&oldid=864789492&diffmode=source, if they report that ?showcurationtoolbar=1 is not in their URL, then we will investigate further.

@kostajh looks like so far the issue has been people following those links with the ?show argument, checking before closing this.

That can be canceled with some css, maybe we will make a gadget to hold that.

<syntaxhighlight lang=css>
/* Hide the page triage toolbar */
#mwe-pt-toolbar{display: none !important;}
</syntaxhighlight>

Etonkovidova closed this task as Resolved.Oct 23 2018, 12:09 AM

Checked in testwiki (and betalabs). To summarize the behavior of the curation toolbar

(1) Curation toolbar will be always present when a user navigates to the article form NPP (via clicking on 'Review' or or on the article link).

(2) Navigating from RC or Watchlist, Contribution - the Curation toolbar will be present depending on a user's last selection.

(3) Curation toolbar is not present when a user just searches an article that in NPP feed.

(4) 'Curate this article' will be present if the curation toolbar is dismissed.

There is no discrepancy in displaying the curation toolbar for User namespace vs Article namespace.

Also, the following is resolved too:

(1) If a user is logged for sometime and has Special:NewPagesFeed open, then clicking on 'Review' will display a page without toolbar.