Page MenuHomePhabricator

Watchlist Expiry: fix WatchItem cache when automatically adding a page to your watchlist via rollback [[MEDIUM]]
Closed, ResolvedPublic

Description

As a Watchlist Expiry user, when rolling back a page that I'm temporarily watching, I want the "watch status" to be accurately represented after the action is completed.

Problem
A WatchedItem object is being cached with no expiry when a page is being temporarily watched resulting in an incorrect representation of the watch status (full star) after the rollback is completed

Possible Solutions

Acceptance Criteria:

  • If a page is rolled back and automatically watched, and if a temporary watch period was already selected, the temporary selection should be accurately represented after the action is completed.

Visual examples:

How users specify automatic rollback in Preferences > Watchlist

Example of how user can rollback an edit

Example of what user sees after rollback:

Event Timeline

ifried created this task.Jul 31 2020, 6:28 PM
Restricted Application added a subscriber: Aklapper. ยท View Herald TranscriptJul 31 2020, 6:28 PM
ifried renamed this task from Watchlist Expiry: Fix inconsistencies when page deleted and then restored to Watchlist Expiry: Watchlist Expiry: make page watched permanently in cases where only 'watch this page' checked.Aug 4 2020, 5:44 PM
ifried renamed this task from Watchlist Expiry: Watchlist Expiry: make page watched permanently in cases where only 'watch this page' checked to Watchlist Expiry: make page watched permanently in cases where only 'watch this page' checked.
ifried updated the task description. (Show Details)Aug 4 2020, 6:00 PM
ifried renamed this task from Watchlist Expiry: make page watched permanently in cases where only 'watch this page' checked to Watchlist Expiry: update rollback status so that watch status preserved.Aug 6 2020, 3:25 PM
ifried updated the task description. (Show Details)
ifried updated the task description. (Show Details)Aug 6 2020, 3:32 PM
ifried updated the task description. (Show Details)
ifried updated the task description. (Show Details)Aug 6 2020, 3:45 PM
ifried updated the task description. (Show Details)Aug 6 2020, 4:04 PM
ifried updated the task description. (Show Details)Aug 7 2020, 12:04 AM
ARamirez_WMF renamed this task from Watchlist Expiry: update rollback status so that watch status preserved to Watchlist Expiry: update rollback status so that watch status preserved [[MEDIUM]].Aug 7 2020, 12:08 AM
ARamirez_WMF moved this task from To Be Estimated/Discussed to Estimated on the Community-Tech board.
ifried updated the task description. (Show Details)Aug 7 2020, 12:09 AM
ifried moved this task from Estimated to Kanban-2020-21-Q1 on the Community-Tech board.

The expiry apparently is preserved (which is good, because it was designed that way), but the star sometimes becomes full apparently due to the same caching issue we thought we fixed with T253937.

HMonroy removed HMonroy as the assignee of this task.Aug 11 2020, 11:14 PM
HMonroy added a subscriber: HMonroy.
dmaza renamed this task from Watchlist Expiry: update rollback status so that watch status preserved [[MEDIUM]] to Watchlist Expiry: fix WatchItem cache when automatically adding a page to your watchlist via rollback [[MEDIUM]].Aug 12 2020, 6:27 PM
dmaza updated the task description. (Show Details)
dmaza added a subscriber: dmaza.Aug 12 2020, 6:46 PM

The expiry apparently is preserved (which is good, because it was designed that way), but the star sometimes becomes full apparently due to the same caching issue we thought we fixed with T253937.

Task description updated to reflect the actual problem

Change 619854 had a related patch set uploaded (by MusikAnimal; owner: MusikAnimal):
[mediawiki/core@master] Cache WatchedItems with preexisting expiry

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

Change 619854 merged by jenkins-bot:
[mediawiki/core@master] WatchedItemStore: Cache single WatchedItems with preexisting expiry

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

Change 621500 had a related patch set uploaded (by MusikAnimal; owner: MusikAnimal):
[mediawiki/core@REL1_35] WatchedItemStore: Cache single WatchedItems with preexisting expiry

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

Change 621500 merged by jenkins-bot:
[mediawiki/core@REL1_35] WatchedItemStore: Cache single WatchedItems with preexisting expiry

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

dom_walden added a subscriber: dom_walden.

Cannot reproduce the issue on beta.

I also could not reproduce the other similar caching issues T259564 and T253937 (just in case this fix affects those).

Briefly tested a few operations that use the functions this fix changed (addWatch and addWatchBatchForUser), including action=watch and Special:EditWatchlist. Seemed to work fine.

In case of regression, on my local vagrant I tested rollback with watchlist expiry disabled. Worked fine, no errors in logs.

Test environments:

ifried updated the task description. (Show Details)Sep 18 2020, 11:16 PM
ifried closed this task as Resolved.Sep 18 2020, 11:21 PM

I have tested this on beta, and it works as expected. When I performed a rollback (with automatic watch upon rollback selected in Preferences) on a page that I had already watched temporarily, the temporary watch status was preserved. For this reason, I'm marking this as Done.