Page MenuHomePhabricator

PageTriage last use not persisted properly for user session
Closed, ResolvedPublic

Description

As reported here, the "last use" time of PageTriage is not properly stored or accessed, which leads to inconsistent results for loading the page curation toolbar on individual page views.

Event Timeline

Just to record a conversation I had with @kostajh, he said that he will investigate one additional idea here, but that since it is so difficult to reproduce this, and since it is not blocking reviews, we'll need to keep an eye on it and assemble more evidence over time.

This is reproducible on test.wikipedia.org (credit to @Etonkovidova):

  1. As a logged-in user, go to https://test.wikipedia.org/wiki/Special:NewPagesFeed
  2. Click "Review" next to an article, verify that the toolbar appears
  3. Wait 4 hours
  4. Reload the article, toolbar appears inconsistently across page reloads

There appears to be a problem using session data for keeping track of last use (see here).

We could both simplify the code and work around this bug by using LocalStorage for keeping track of a user's last use of PageTriage. In listControlNav.js we'd want to use mw.storage.session.set( 'pagetriage-lastuse', mw.now();, then in modules/ext.pageTriage.views.toolbar/ext.pageTriage.toolbarView.js:L128 calculate whether to show/hide the toolbar based on getting the pagetriage-lastuse value in LocalStorage.

Change 458550 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/PageTriage@master] WIP: Use localStorage for tracking PageTriage last use

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

kostajh changed the task status from Open to Stalled.Sep 6 2018, 6:20 PM
kostajh added a subscriber: aaron.

Per discussion with @MMiller_WMF today, submitting my WIP patch for this and marking the task as stalled for now.

Also adding that per discussion with @aaron, localStorage is the preferred solution as sessions aren't meant for this kind of data any more.

kostajh changed the task status from Stalled to Open.Sep 21 2018, 2:46 PM
kostajh added a subscriber: Catrope.

Per our last triage meeting, @Catrope is going to review this.

@kostajh -- another user may be reporting this behavior. See comments from User:Atsme here.

Thanks for the heads up @MMiller_WMF, this is in the code review now.

Change 458550 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@master] Determine toolbar visibility with query param and localStorage

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

@MMiller_WMF thanks for that. Users will continue to experience the issue until next Thursday's deployment, because the patch above didn't make it into this week's train. If you would like, we could swat the patch so it's in production today or tomorrow. We could also ask users to verify that things are fixed in testwiki this week and wait until next week for the fix to roll out with the next train.

@kostajh -- I think it is fine for users to wait until next Thursday. I will just give them that update. And I kind of think it will cause additional confusion to test in Test Wiki because the issue is sort of silent and intermittent, so it will be hard to tell if it is caused by the change in environment?

Waiting is fine by me, probably good to tell the users that a fix is coming next week then. It is possible to reproduce the bug (see here) if any users want to try.

Etonkovidova claimed this task.