Make class WatchAction a real action class and move helper function to own service
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Umherirrender
	Oct 19 2020, 7:48 PM

Description

WatchAction::doWatchOrUnwatch needs some services

https://codesearch.wmcloud.org/search/?q=WatchAction&i=nope&files=&repos=

Details

	Subject	Repo	Branch	Lines +/-
	Hard deprecate WatchAction::getWatchToken	mediawiki/core	master	+7 -0

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Open	None	T259960 Inject services into API modules and special pages
Resolved	Vlad.shapik	T265944 Make class WatchAction a real action class and move helper function to own service
Resolved	CCicalese_WMF	T280750 Hard deprecate watchlist methods in WatchAction
Resolved	Vlad.shapik	T285399 extensions/LiquidThreads: avoid using WatchAction::getWatchToken

Event Timeline

Umherirrender created this task.Oct 19 2020, 7:48 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 19 2020, 7:48 PM

This is not T253078: Create ActionFactory, add dependency injection to actions, because that the non-static part of the class. This is the static part.

If WatchAction::doWatchOrUnwatch moves, so should WatchAction::doWatch and WatchAction::doUnwatch
The following services should be injected to the new service:
WatchedItemStore - currently retrieved from MediaWikiServices
PermissionManager - currently retrieved from MediaWikiServices
HookContainer - currently used via Hooks::runner()

The WatchAction methods use calls to User::addWatch and ::removeWatch, which internally use a PermissionManager and a WatchedItemStore. Thus, I would suggest that the new service avoid using the user methods, and instead the user methods be deprecated in favor of calls to the new service.
Alternatively, the code could all be moved into the existing WatchedItemStore

Restricted Application added a project: Growth-Team. · View Herald TranscriptOct 22 2020, 2:10 AM

DannyS712 awarded a token.Mar 25 2021, 1:33 AM

DannyS712 added projects: Platform Engineering, User-DannyS712.

DannyS712 moved this task from Unsorted to Later on the User-DannyS712 board.

BPirkle triaged this task as Medium priority.Mar 30 2021, 8:24 PM

BPirkle edited projects, added Platform Team Workboards (Clinic Duty Team); removed Platform Engineering.

BPirkle moved this task from Inbox to Later on the Platform Team Workboards (Clinic Duty Team) board.

• Pchelolo closed this task as Resolved.Mar 30 2021, 8:50 PM

• Pchelolo assigned this task to CCicalese_WMF.

• Pchelolo edited projects, added Platform Team Workboards (MW Expedition); removed Platform Team Workboards (Clinic Duty Team).

@Pchelolo WatchAction::doWatchOrUnwatch, doWatch, and doUnwatch still have all of the logic - this doesn't appear to have been done

Oh, I resolved it by mistake. Mostly wanted to CC Cindy since she's working on this area.

MBinder_WMF added a project: Growth-Team-Filtering.Apr 15 2021, 6:48 PM

CCicalese_WMF moved this task from Unsorted pile to Doing on the Platform Team Workboards (MW Expedition) board.Apr 21 2021, 5:07 AM

I should have linked this task from the relevant patches (https://gerrit.wikimedia.org/r/c/mediawiki/core/+/678679 and https://gerrit.wikimedia.org/r/c/mediawiki/core/+/681141). The following were done in those patches:

WatchAction::doWatch has been merged with WatchlistManager::addWatch
WatchAction::doUnwatch has been merged with WatchlistManager::removeWatch
WatchAction::::doWatchOrUnwatch has been moved to WatchlistManager and renamed WatchlistManager::setWatch

I will be working on removing remaining uses of and hard deprecating the methods in WatchAction.

The remaining functions still need to be addressed (I'm not currently working on them, so they're free to be picked up by somebody else if they want):

WatchAction::getExpiryOptions
WatchAction::getWatchToken

CCicalese_WMF removed CCicalese_WMF as the assignee of this task.Apr 26 2021, 6:13 PM

CCicalese_WMF added a subtask: T280750: Hard deprecate watchlist methods in WatchAction.

CCicalese_WMF subscribed.

CCicalese_WMF closed subtask T280750: Hard deprecate watchlist methods in WatchAction as Resolved.May 5 2021, 5:38 PM

I don't think getExpiryOptions necessarily need to be addressed right away, it's just rendering localized names for possible expiry options, so it's fine to live in WatchAction.

WatchAction::getWatchToken though needs to be hard-deprecated. Should be done after https://gerrit.wikimedia.org/r/c/mediawiki/core/+/697639 is merged.

Change 701161 had a related patch set uploaded (by Vlad.shapik; author: Vlad.shapik):

[mediawiki/core@master] Hard deprecate WatchAction::getWatchToken

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

gerritbot added a project: Patch-For-Review.Jun 23 2021, 5:20 PM

Change 701161 merged by jenkins-bot:

[mediawiki/core@master] Hard deprecate WatchAction::getWatchToken

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

ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.12; 2021-06-28).Jun 23 2021, 6:00 PM

Maintenance_bot removed a project: Patch-For-Review.Jun 23 2021, 6:11 PM

Vlad.shapik moved this task from Doing to On the train on the Platform Team Workboards (MW Expedition) board.Jun 24 2021, 7:35 AM

• Pchelolo closed this task as Resolved.Jul 13 2021, 3:17 PM

• Pchelolo closed subtask T285399: extensions/LiquidThreads: avoid using WatchAction::getWatchToken as Resolved.