Proposal:

• Keep the EmailNotification as a service class. Plenty of config variables/hooks refer to enotif and renaming this class can bring more confusion than good. For clarity, we could rename the notifyOnPageChange to listenToPageChange and this would allow us to listen to an DomainEvent in the future.
• Implement new RecentChangeNotification as a base class for ENotif notifications. Then have two additional subclasses for each specific notifications WatchlistNotification, TalkPageNotification.
• For now, I recommend keeping existing hooks to limit the amount of changes.
• on actuallyNotifyOnPageChange use the NotificationService to trigger notifications:
  • When change happens on Talk page - trigger TalkPageNotification
  • When config MainConfigNames::UsersNotifiedOnAllChanges is set, use this values from this config and trigger RecentChangeNotification
  • iterate over $watchers and trigger WatchlistNotification in batches of 500 each.
• Move the Email rendering logic (compose, composeCommonMailtext) to a Presentation service
  • Implement RecentChangeNotificationHandler that would listen to Notifications mentioned above (RecentChage, Watchlist, TalkPageNotification). This handler will use EmailComposer to compose emails and send then

Next steps

• onAbortTalkPageEmailNotification - is used only by Echo, this hook could be deprecated and Echo could just handle the TalkPageNotification.
• same applies to onSendWatchlistEmailNotification, if Echo Provider could handle the `
• We should keep most of the how and when to trigger RecentChangeNotification out of the MediaWiki notification system.
• As we keep most of should I send this email logic in the EmailNotification class, after work on RecentChangeNotificationHandler is done - we can reconsider moving some parts to Handler (see https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/fb32ca22651e5bfce0c36ee8b236f1268b9d841d/includes/mail/EmailNotification.php#282, checks like isEmailConfirmed() )

Questions, pain points:

• Currently only mails from Watchlist have a header from List-Help='https://www.mediawiki.org/wiki/Special:MyLanguage/Help:Watchlist. Can we add this header to all Watchlist related notifications? (including talk page and
• onSendWatchlistEmailNotification is used by Echo and Bluespice. Bluespice is using is only to filter out entity notifications. We can deprecate this hook if we provide a mechanism to abort sending a notification.
• can we drop support for impersonal messages? Doesn't look like it is used.

Findings

ENotif can send multiple notifications to same User. For example if I'm watching an article, and I'm on the UsersNotifiedOnAllChanges list - I still was getting two emails even there is a code preventing it.

Steps

1. Implement WatchlistEmailComposer class, and move the mail composing logic there. For now this class would be used in EmailNotification
2. Implement the RecentChangeNotification and RecentChangeNotificationHandler.
3. Refactor the EmailNotification class to trigger RecentChangeNotification for users listed in UsersNotifiedOnAllChanges config variable
4. Refactor the EmailNotification class to trigger TalkPageNotification when a change happens on Talk page.
5. Test the system and validate the proposed solution
6. Migrate the remaining part, eg introduce WatchlistNotification.

This approach will allow us to gradually move notification/email rendering logic out of EmailNotification class, keeping the existing code mostly intact (configs/hooks). It will also allow to validate the idea on step 3 and 4 before moving the entire watchlist into Notifications system.

cc @Mooeypoo @matmarex @Catrope

During one of the collab sessions we talked through the proposed solution and agreed that this seems a reasonable approach. As a result of this ticket, I created tickets and assigned them to the parent task.

I'm calling this part of work done. @JTweed-WMF is there anything else you expect before I close this ticket?