probably checkNewtalk needs to be factored out too

Probably the TalkPageNotificationManager need to be introduced before the WatchlistNotificationManager (T208777) because as I understand it the User::clearNotification is to be replaced by the WatchlistNotificationManager and the method make use of both User::getNewtalk and User:: setNewtalk so it would make sense to have these already moved to the service

It feels like the User::getTalkPage could be also migrated into a new service, but probably not the TalkPageNotificationManger

The service probably could be drafted like below:

class TalkPageNotificationManager {

    private $userNewTalkCache = [];
    
    public function getNewtalk() { ... }
    public function getNewMessageLinks() { ... }
    public function getNewMessageRevisionId() { ... }
    public function setNewtalk() { ... }
    public function updateNewtalk() { ... }
    public function deleteNewtalk() { ... }
    public function checkNewtalk() { ... }

    public function invalidateCache() { ... }

}

Merge with T208777: Factor notification-related methods out of User and Title, introduce WatchlistNotificationManager?

Change 585852 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Create TalkPageNotificationManager service

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

Change 589345 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Cover user_newtalk functionality with tests

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

Change 589345 merged by jenkins-bot:
[mediawiki/core@master] Fix Postgres user_newtalk table

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

Change 585852 merged by jenkins-bot:
[mediawiki/core@master] Create TalkPageNotificationManager service

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

Deployed uses of the now deprecated methods:
https://codesearch.wmflabs.org/deployed/?q=-%3E(getNewMessage(Links%7CRevisionId)%7C(set%7Cupdate%7Cdelete%7Cget)Newtalk)&i=nope&files=&repos=

Change 594812 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] ApiClearHasMsg: Use new TalkPageNotificationManager

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

Now that the TalkPageNotificationManager was introduced, we can start using it. However, here it will not be a one-to-one replacement for calling methods.

Revision parameter in TalkPageNotificationManager::setUserHasNewMessages is supposed to be required now - we need to make it required to make improvements to the user_newtalk table. This has to be taken into account when replacing usages of setNewtalk.

Also, User::getNewMessageLinks was somewhat replaced by TalkPageNotificationManager::getLatestSeenMessageTimestamp, but there's still quite a bunch of code left in User:: getNewMessageLinks. Not all of that code was ported into TalkPageNotificationManager cause of its crazy overcomplicated return type contract - it attempts to be compatible with crosswiki talk page notifications, but that was never implemented and I do not know if that compatibility is required. We need to figure out if we want that complexity.

After usages are changed and we ensure Revision is passed to setUserHasNewMessages, we can remove the fallback to null there

Change 594812 merged by jenkins-bot:
[mediawiki/core@master] ApiClearHasMsg: Use new TalkPageNotificationManager

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

Given that the end goal is not just decoupling, but also moving us closer to fixing the problems with the table described by @Krinkle in T146585#4233276 we need to change the API to perhaps eventually disallow NULL user_last_timestamp so that we're able to switch to upsert and remove duplicated rows, so I have deprecated passing null revision into setNewtalk. However, maybe this requires course correction.

Currently, TalkPageNotificationManager::setUserHasNewMessages has an optional $revision parameter, that is the 'new, not yet seen revision of a user talk page'. We then use RevisionStore to fetch a previous revision, and use it's timestamp. If RevisionRecord is not passed, or it's the first revision of the page, or the previous was not found for some reason, we insert NULL. The exact value of the field is then used in a query with a MIN function, and is ignored in that result. Additionally, simply the presence of the row in the database with NULL timestamp indicates that there are some new messages.

So, NULL timestamp in the database actually means several different things:

• We could not find a revision prior to the new unseen revision due to replication lag or whatever
• There is not revision prior to the new unseen revision yet - the user page was just created.
• Someone didn't include the known new revision into the method call. That happens in Echo and LiquidThreads.

So, TLDR: we can probably try to eliminate the third issue - not passing a revision due to coding error, but we can't really eliminate the first and the second source of NULLs without a major refactoring of the whole feature, which is vastly out of scope for this decoupling effort. So, I propose to undeprecate passing null into the method, I was too cavalier with it.

Change 596501 had a related patch set uploaded (by Clarakosi; owner: Clarakosi):
[mediawiki/core@master] TalkPageNotificationManager: Undeprecate passing null to setUserHasNewMessages

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

Change 596501 merged by jenkins-bot:
[mediawiki/core@master] TalkPageNotificationManager: Undeprecate passing null to setUserHasNewMessages

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

Change 596685 had a related patch set uploaded (by Clarakosi; owner: Clarakosi):
[mediawiki/core@master] Use new TalkPageNotificationManager

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

Change 596695 had a related patch set uploaded (by Clarakosi; owner: Clarakosi):
[mediawiki/extensions/Echo@master] Use new TalkPageNotificationManager

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

Change 596727 had a related patch set uploaded (by Clarakosi; owner: Clarakosi):
[mediawiki/extensions/LiquidThreads@master] Use TalkPageNotificationManager

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

Change 596732 had a related patch set uploaded (by Clarakosi; owner: Clarakosi):
[mediawiki/skins/BlueSky@master] Use TalkPageNotificationManager

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

Change 596733 had a related patch set uploaded (by Clarakosi; owner: Clarakosi):
[mediawiki/skins/Nostalgia@master] Use TalkPageNotificationManager

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

Change 596742 had a related patch set uploaded (by Clarakosi; owner: Clarakosi):
[mediawiki/skins/Tempo@master] Use TalkPageNotificationManager

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

Change 596743 had a related patch set uploaded (by Clarakosi; owner: Clarakosi):
[mediawiki/extensions/MediaWikiAuth@master] Use TalkPageNotificationManager

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

Change 596695 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Use new TalkPageNotificationManager

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

Change 596727 merged by jenkins-bot:
[mediawiki/extensions/LiquidThreads@master] Use TalkPageNotificationManager

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

Change 596742 merged by jenkins-bot:
[mediawiki/skins/Tempo@master] Use TalkPageNotificationManager

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

Change 596743 merged by jenkins-bot:
[mediawiki/extensions/MediaWikiAuth@master] Use TalkPageNotificationManager

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

Change 596733 merged by jenkins-bot:
[mediawiki/skins/Nostalgia@master] Use TalkPageNotificationManager

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

Change 596732 merged by jenkins-bot:
[mediawiki/skins/BlueSky@master] Use TalkPageNotificationManager

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

Change 596685 merged by jenkins-bot:
[mediawiki/core@master] Use new TalkPageNotificationManager

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

Change 607592 had a related patch set uploaded (by Clarakosi; owner: Clarakosi):
[mediawiki/core@master] Hard deprecate User::getNewtalk

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

Change 607592 merged by jenkins-bot:
[mediawiki/core@master] Hard deprecate User::getNewtalk and User::setNewtalk

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

Change 710554 had a related patch set uploaded (by Zabe; author: Clarakosi):

[mediawiki/skins/Nostalgia@REL1_35] Use TalkPageNotificationManager

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

Change 710554 abandoned by Zabe:

[mediawiki/skins/Nostalgia@REL1_35] Use TalkPageNotificationManager

Reason:

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