Page MenuHomePhabricator

Watchlist missing logic is broken and hits the API multiple times unnecessarily on load
Closed, ResolvedPublic5 Story Points

Description

Use testing criteria from https://phabricator.wikimedia.org/T189066
Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:EditWatchlist

Open the network tab - many API requests are made (one for each missing page). o_O
Not only that.. but for pages that do not exist (red links) the watchstar is unfilled and it's impossible to remove the item from the watchlist.

Expected: No API call should be necessary on this page.

Acceptance criteria

  • On the Watchlist, for pages loaded via PHP, no API call for watch status should be necessary. They are on the watchlist so we know they are watched!
  • Infinite scroll should trigger 1 API request (as it does currently)
  • Watch stars for missing links should be filled
  • Hygiene patches should be merged.

Testing criteria

Use testing criteria from https://phabricator.wikimedia.org/T189066
Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:EditWatchlist

  • Verify that you can add/remove items from your watchlist (including red links)
  • Verify that you watch articles on pages via the top page actions menu.
  • Verify that you can watch articles on Special:Nearby and that refreshing the page continues to show the pages as watched
  • Verify that you can watch/unwatch articles while making a search

Event Timeline

Jdlrobson created this task.Apr 4 2018, 7:01 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 4 2018, 7:01 PM
Jdlrobson triaged this task as High priority.Apr 4 2018, 7:01 PM
Jdlrobson updated the task description. (Show Details)Apr 10 2018, 4:19 PM
Jdlrobson renamed this task from Watchlist hits the API multiple times unnecessarily on load to Watchlist missing logic is broken and hits the API multiple times unnecessarily on load.Apr 10 2018, 4:23 PM

In short: don't make a watchlist request for pages that are known to be missing.

Jdlrobson updated the task description. (Show Details)Apr 10 2018, 4:24 PM
ovasileva set the point value for this task to 5.Apr 10 2018, 4:27 PM
ovasileva moved this task from Upcoming to 2017-18 Q4 on the Readers-Web-Backlog board.

Change 426974 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Fix: TypeError at end of watchlist

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

Change 426974 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Fix: TypeError at end of watchlist

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

Change 427531 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] WIP: Fix: don't request watch status with unknown IDs

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

Change 427394 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Hygiene: trim unused PageListItem template tag

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

Change 427747 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Hygiene: remove unused styles in Watchlist

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

Change 427394 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Hygiene: trim unused PageListItem template tag

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

Change 427747 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Hygiene: remove unused styles in Watchlist

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

Niedzielski removed Niedzielski as the assignee of this task.Apr 20 2018, 4:38 AM

Change 428846 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Hygiene: upgrade WatchstarGateway to formatversion v2

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

Change 428847 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/MinervaNeue@master] Hygiene: replace calledOnce / Twice w/ callCount

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

Change 428848 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/Popups@master] Hygiene: replace calledOnce / Twice w/ callCount

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

Change 428986 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Hygiene: rename and symbolize InfiniteScroll event

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

Change 429226 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Hygiene: rename InfiniteScroll to ScrollEndEmitter

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

Change 428846 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Hygiene: upgrade WatchstarGateway to formatversion v2

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

I'd like to get https://gerrit.wikimedia.org/r/427531 which fixes the bug out into the next train. I suggest we get that wrapped up and then focus on the hygiene patches.

Change 430015 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Fix: limit WatchlistGateway to 50 not 51 results

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

Niedzielski removed Niedzielski as the assignee of this task.May 1 2018, 4:59 AM

Change 427531 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Fix: bugs in watchstars

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

Jdlrobson updated the task description. (Show Details)
Jdlrobson reassigned this task from Jdlrobson to ABorbaWMF.May 1 2018, 11:10 PM

Skipping design review as a bug.
Anthony can you take a look at the testing steps on the beta cluster?

Side note: There are some hygiene patches - I'll look at those in parallel.

Looking good to me on a variety of devices/browsers





Jdlrobson updated the task description. (Show Details)May 2 2018, 6:18 PM

Change 430015 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Fix: limit WatchlistGateway to 50 not 51 results

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

Looks good from my side. @Jdlrobson - one more checkbox in the acceptance criteria. Could you take a look and sign off?

Jdlrobson closed this task as Resolved.May 3 2018, 10:21 PM
Jdlrobson updated the task description. (Show Details)

I think we reached a good place. There are more hygiene patches but only loosely tied to this change.

Change 428986 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Hygiene: rename and symbolize InfiniteScroll event

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

Change 429226 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Hygiene: rename InfiniteScroll to ScrollEndEmitter

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