Page MenuHomePhabricator

Watchlist Expiry: update watch status when editing via dropdown [[LARGE]]
Closed, ResolvedPublic

Description

As a Watchlist Expiry user, I want the star to be automatically updated when I make a watch period selection via edit, so that I can immediately see the relevant watch period in the star (shading & tooltip).

NOTE: This issue is most likely related to T256654. It is also sometimes reproducible and sometimes not on various environments (i.e., there is not necessarily a consistent history of this bug).

Acceptance Criteria:

  • If the user watches via legacy source editor (temporary or permanent), the star and tooltip should be automatically updated upon saving changes
  • If the user edits a temporary watch period via source editing, the star and tooltip should be automatically updated upon saving changes
  • If the user unwatches via source editing, the star and tooltip should be automatically updated upon saving changes
  • Investigate/fix the issue in T256654

Steps to reproduce:

  1. Use the legacy wikitext editor
  2. On a page not on your watchlist (new page, or existing page):
    1. Edit the page, include any change in your edit
    2. Select "watch" using the 'watchthis' control check box
    3. Publish the page
  3. The page now opens for reading, however the watchlink or star still reports the page is not watched

Desired behavior: when the page re-enters reading mode, the now-current watched status should be used to display the appropriate tab controls

Note: the page does actually get added to the watchlist, and manually reloading the page will correct the display

Originally reported on enwiki by BlackcurrantTea (c.f. https://en.wikipedia.org/w/index.php?title=Wikipedia:Village_pump_(technical)&oldid=965128101#New_talk_pages_not_getting_watched)

Event Timeline

Restricted Application added a subscriber: Aklapper. ยท View Herald TranscriptAug 3 2020, 9:57 PM
ifried renamed this task from Watchlist Expiry: fix star issue when source editing [placeholder] to Watchlist Expiry: update watch status when editing via dropdown [placeholder].Aug 3 2020, 10:40 PM
ifried updated the task description. (Show Details)
ifried updated the task description. (Show Details)
ifried renamed this task from Watchlist Expiry: update watch status when editing via dropdown [placeholder] to Watchlist Expiry: update watch status when editing via dropdown.Aug 6 2020, 11:21 PM
ifried updated the task description. (Show Details)
ARamirez_WMF renamed this task from Watchlist Expiry: update watch status when editing via dropdown to Watchlist Expiry: update watch status when editing via dropdown [[LARGE]].Aug 6 2020, 11:48 PM
ifried updated the task description. (Show Details)

There seems to be a race condition between the redirect to the newly edited page after the edit operation is completed and the callback to modify the watch state added as a DeferredUpdate during the edit. See here.

What's happening

  1. When editing a page the callback to update the watchlist enters the queue at around position 94 and the redirect happens before the callback is executed
  2. When editing a page if the edit is null (no changes to the article) but the watchlist state changes, the callback to update the watchlist enters the queue at around position 6 and everything works as expected

Below are some simplified logs of the order of execution of the different methods involved in editing the page, adding the callback to deferred updates, changing the watchlist state, and fetching the watched item. Look closely the request ID (3rd column) and timestamp at the end of the line. I've also added the deferred updates count right before we push WatchAction::doWatchOrUnwatch to the queue.


Editing a page and changing the watch status (current issue)
The third line is the new GET request to see the article after edit succeeds and runs before the callback is executed (4th line)

2020-08-12 22:02:47 [6045b840-be9c-4c0a-9242-f6292ee68d73] ... DEBUG: EditPage::edit: enter 1597269767.9306  
2020-08-12 22:02:48 [6045b840-be9c-4c0a-9242-f6292ee68d73] ... DEBUG: watchlistexpiry: deferred updates count: 93
2020-08-12 22:02:49 [bb508fca-9dc9-4f92-8fee-74b981f8871c] ... DEBUG: watchlistexpiry: WatchedItemStore::getWatchedItem enter 1597269769.4544 
2020-08-12 22:02:49 [6045b840-be9c-4c0a-9242-f6292ee68d73] ... DEBUG: watchlistexpiry: {closure} adding call to WatchAction::doWatchOrUnwatch 1597269769.5007
2020-08-12 22:02:49 [6045b840-be9c-4c0a-9242-f6292ee68d73] ... DEBUG: watchlistexpiry: WatchAction::doWatchOrUnwatch enter 1597269769.5015

Editing a page without changing the article and modifying the watch status (this works just fine)
Here everything runs in the expected order

2020-08-12 22:03:49 [a3a45848-5408-4897-a233-47f9693f72b0] ... DEBUG: EditPage::edit: enter 1597269829.7101  
2020-08-12 22:03:49 [a3a45848-5408-4897-a233-47f9693f72b0] ... DEBUG: watchlistexpiry: deferred updates count: 5  
2020-08-12 22:03:50 [a3a45848-5408-4897-a233-47f9693f72b0] ... DEBUG: watchlistexpiry: {closure} adding call to WatchAction::doWatchOrUnwatch 1597269830.0085  
2020-08-12 22:03:50 [a3a45848-5408-4897-a233-47f9693f72b0] ... DEBUG: watchlistexpiry: WatchAction::doWatchOrUnwatch enter 1597269830.0092  
2020-08-12 22:03:50 [29c1c491-aeed-421f-9b7c-e36b62e9c93b] ... DEBUG: watchlistexpiry: WatchedItemStore::getWatchedItem enter 1597269830.1748

Change 619899 had a related patch set uploaded (by Dmaza; owner: Dmaza):
[mediawiki/core@master] Fix race condition on edit page

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

Change 619899 merged by jenkins-bot:
[mediawiki/core@master] Fix race condition on edit page

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

Similar bug, but I think unrelated in terms of code T260434.

Tested all the possible (I think) transitions from one watch state to another while submitting an edit.

I did so for Source Editor (for an empty and non-empty edit) and VisualEditor.

After submitting, the watch star and tooltip were always in the correct state.

The only exception was T260434, but I think that is a different part of the code.

After each submit, I also refreshed the page to check that the correct watch state and expiry was actually saved to the database.

I also briefly tested with JavaScript disabled. It behaved the same, which is not surprising as this is not a JS-related issue.

Test environment: https://en.wikipedia.beta.wmflabs.org MediaWiki 1.36.0-alpha (473361f) 2020-08-17T06:40:15

N.B. I will make a note for myself to test this on production (probably testwiki) when it goes live.

I have tested this on testwiki. The star is now properly shaded and the tooltip is working upon selecting a watch period in the legacy source editor. For this reason, I'm marking it as Done.

Change 620980 had a related patch set uploaded (by Dmaza; owner: Dmaza):
[mediawiki/core@REL1_35] Fix race condition on edit page

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

Change 620980 merged by jenkins-bot:
[mediawiki/core@REL1_35] Fix race condition on edit page

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

N.B. I will make a note for myself to test this on production (probably testwiki) when it goes live.

I have retested this very briefly on testwiki (1.36.0-wmf.5 (rMW753f685df568) 04:37, 18 August 2020) on Source Editor and VisualEditor.