To me it seems like we should at least have the interfaces (aka, the services) an a basic (or empty) implementation of a notification system in core. This mainly means replacing the hook handlers that Echo uses to hook into core with proper service interfaces that all core components can know about and use.

Current state:

Me & Timo are in agreement that we need good enough interfaces in core that other extensions don't have to depend on Echo; also agreed that shipping Echo with core as a default extension makes sense.

We still have to discuss the 'line' of how much of Echo infrastructure needs to be moved to core, which'll be the subject of next week's archcom IRC meeting. :)

@brion @Krinkle do you think you could propose strawman service interfaces to replace Echo hook handlers, for next week's discussion? These would be the "good enough interfaces", I suppose.

In T128351#2500149, @brion wrote:

We still have to discuss the 'line' of how much of Echo infrastructure needs to be moved to core, which'll be the subject of next week's archcom IRC meeting. :)

/me nods in agreement

We have this planned for next week's IRC meeting:
E251: ArchCom RFC Meeting W31: Notifications in core (2016-08-03, #wikimedia-office)

I'm not sure how realistic it is to find an abstraction that is sufficiently non-leaky that it's worth separating from Echo. The current interface is basically EchoEventPresentationModel (ie. getType, getCategory, getIconType, canRender, getHeaderMessage, getCompactHeaderMessage, getSubjectMessage, getBodyMessage getPrimaryLink, getSecondaryLinks, plus something for bundling support if that should be part of core, plus something for finding the target users) and that very much depends on the current Echo flyout design. An alternative implementation with a different design probably would not find the body / primary links / secondary links split that useful.

In T128351#2503928, @Tgr wrote:

I'm not sure how realistic it is to find an abstraction that is sufficiently non-leaky that it's worth separating from Echo. The current interface is basically EchoEventPresentationModel [details on that] An alternative implementation with a different design probably would not find the body / primary links / secondary links split that useful.

Let me see if I understand this based on a naive reading of what you wrote. When a server application wants to present a message to a user, it instantiates an object that implements the EchoEventPresentationModel interface, and puts that object in Echo's message queue somehow. Do I have that basically right?

Currently (when using the non-legacy method), you would

• use the BeforeCreateEchoEvent hook to register event types and set up some other minor details such as icon URLs
• register an EchoEventPresentationModel for each event type, which can convert an EchoEvent into a predefined set of messages (header, body etc)
• when the event happens, instantiate and queue an EchoEvent
• use the EchoGetDefaultNotifiedUsers hook to return a list of users to notify

EchoEvent and EchoGetDefaultNotifiedUsers lend themselves to standardization well; the first is basically just a hashmap of event details, the second is just a service that gets an EchoEvent and has to return a user list. EchoEventPresentationModel seems hard to untangle from implementation details. Or you can just leave it out, in which case notification-sending extensions would have a standard event model but would have to define a presentation model for each implementation they want to be compatible with.

Things might get even more complex if you consider bundling ("your article was linked 10 times!" instead of ten separate "your article was linked by X!" messages), but I never actually used that so not sure.

An alternative implementation with a different design probably would not find the body / primary links / secondary links split that useful.

Even if it's not useful, dealing with that split doesn't seem at all difficult. The split seems fairly generic to me: you have content, a main link, and possibly additional links.

Bundling, on the other hand, might be difficult for some implementations to deal with unless the bundle-event's primary link is to a page that lists the subevents. From the notification-generation side it can probably just ignore bundling, while a consumer that can't support it would probably want to either unbundle or request unbundled events.

The split seems fairly generic to me: you have content, a main link, and possibly additional links.

Echo requires you to not have any links in your content (instead the primary link will wrap the content). A different implementation might allow links inside and not need the primary link. Or it might want a short and full version of the notification, for a dense list with show/hide. Or it might want a guranteed <140 characters message for SMS notifs. Etc. Hard to be generic, and seems like a waste of time to me given how small the chance is that someone would write an Echo alternative.

No links in the body seems like a reasonable restriction to me. A short
(<140 char?) summary is something that could certainly be added, although
yagni may apply.

In T128351#2507030, @Tgr wrote:

Hard to be generic, and seems like a waste of time to me given how small the chance is that someone would write an Echo alternative.

I can respect a desire not to generalize something that seems to be working well enough for the current usage patterns. Just to clarify: do you believe that Echo implements essential notification functionality that belongs in core, or do you think having it as an extension (and implicitly optional) is fine? My reading of your March comment (T128351#2105216) is that you're supportive of moving Echo notifications into core.

My impression is that the functionality implemented by Echo should be defined by core (not necessarily implemented). But the code currently in the Echo extension apparently isn't fit for that. Defining a proper notification interface from scratch may be easier than trying to merge (part of) Echo in.

In T128351#2515396, @RobLa-WMF wrote:

do you believe that Echo implements essential notification functionality that belongs in core

I do believe that. And I am usually a fan of unnecessary generalizations :) I just think that view interfaces are very hard to generalize in a useful way, and more so when trying to generalize from a sample set of 1.
(I am not familiar with the codebase beyond what is exposed to clients of the extension, so I have no opinion on whether it is a bad idea to merge the code as it is.)

+1

In T128351#2515632, @daniel wrote:

Defining a proper notification interface from scratch may be easier than trying to merge (part of) Echo in.

In T128351#2515802, @Tgr wrote:

I just think that view interfaces are very hard to generalize in a useful way, and more so when trying to generalize from a sample set of 1.

@Tgr makes a very powerful point here. Generalizing "from a sample set of 1" seems really dangerous. @daniel, you seem to be proposing something even more dangerous: design something "from scratch". What problem does that solve? Who would do this work? If we don't use Echo functionality, how would we integrate Echo with it?

@RobLa-WMF: I'm not sure what @Tgr means by a view-interface in this context. I agree for UI design - it's very hard to do that in a generic way.

Abstracting from a sample set of one is of course also very hard - but that's not what I propose. Echo is already an abstraction of various kinds of notifications we need. I propose to define an actual service interface for this functionality, instead of relying on hooks, which we would do if we just merge Echo into core as-is.

Generally, hooks are not used within core; so when moving Echo functionality into core, it should be re-defined in terms of an actual service interface. Besides the general advantages of a service interface, such as type-safety, a service interface clearly defines what notification functionality is available, and lets core code interact with it directly.

My point is: core code should not listen e.g. to the LinksUpdateAfterInsert, it should rather be całled directly by the LinksUpdate class. That is much more obvious and safer.

To me, the crucial part of this RFC is defining this interface that core can call to trigger notifications (or, to put the the other way around, the interface core offers to trigger notifications).

As a point-in-case, here are all the hooks that Echo registers:

	// Housekeeping hooks
	$wgHooks['LoadExtensionSchemaUpdates'][] = 'EchoHooks::onLoadExtensionSchemaUpdates';
	$wgHooks['GetPreferences'][] = 'EchoHooks::getPreferences';
	$wgHooks['PersonalUrls'][] = 'EchoHooks::onPersonalUrls';
	$wgHooks['BeforePageDisplay'][] = 'EchoHooks::beforePageDisplay';
	$wgHooks['MakeGlobalVariablesScript'][] = 'EchoHooks::makeGlobalVariablesScript';
	$wgHooks['UnitTestsList'][] = 'EchoHooks::getUnitTests';
	$wgHooks['ResourceLoaderRegisterModules'][] = 'EchoHooks::onResourceLoaderRegisterModules';
	$wgHooks['EventLoggingRegisterSchemas'][] = 'EchoHooks::onEventLoggingRegisterSchemas';
	$wgHooks['ResourceLoaderTestModules'][] = 'EchoHooks::onResourceLoaderTestModules';
	$wgHooks['UserGroupsChanged'][] = 'EchoHooks::onUserGroupsChanged';
	$wgHooks['UserLoadOptions'][] = 'EchoHooks::onUserLoadOptions';
	$wgHooks['UserSaveOptions'][] = 'EchoHooks::onUserSaveOptions';
	$wgHooks['UserClearNewTalkNotification'][] = 'EchoHooks::onUserClearNewTalkNotification';
	$wgHooks['ParserTestTables'][] = 'EchoHooks::onParserTestTables';
	$wgHooks['EmailUserComplete'][] = 'EchoHooks::onEmailUserComplete';
	$wgHooks['LoginFormValidErrorMessages'][] = 'EchoHooks::onLoginFormValidErrorMessages';

	// Extension:UserMerge support
	$wgHooks['UserMergeAccountFields'][] = 'EchoHooks::onUserMergeAccountFields';
	$wgHooks['MergeAccountFromTo'][] = 'EchoHooks::onMergeAccountFromTo';
	$wgHooks['UserMergeAccountDeleteTables'][] = 'EchoHooks::onUserMergeAccountDeleteTables';

	// Extension initialization
	$wgExtensionFunctions[] = 'EchoHooks::initEchoExtension';

	require __DIR__ . '/Resources.php';

	$wgHooks['EchoGetBundleRules'][] = 'EchoHooks::onEchoGetBundleRules';
	$wgHooks['EchoAbortEmailNotification'][] = 'EchoHooks::onEchoAbortEmailNotification';

	// Hook appropriate events
	$wgHooks['ArticleSaveComplete'][] = 'EchoHooks::onArticleSaved';
	$wgHooks['AddNewAccount'][] = 'EchoHooks::onAccountCreated';
	$wgHooks['ArticleRollbackComplete'][] = 'EchoHooks::onRollbackComplete';
	$wgHooks['UserSaveSettings'][] = 'EchoHooks::onUserSaveSettings';

	// Disable ordinary user talk page email notifications
	$wgHooks['AbortTalkPageEmailNotification'][] = 'EchoHooks::onAbortTalkPageEmailNotification';
	$wgHooks['SendWatchlistEmailNotification'][] = 'EchoHooks::onSendWatchlistEmailNotification';
	// Disable the orange bar of death
	$wgHooks['GetNewMessagesAlert'][] = 'EchoHooks::abortNewMessagesAlert';
	$wgHooks['LinksUpdateAfterInsert'][] = 'EchoHooks::onLinksUpdateAfterInsert';

	// Beta features
	$wgHooks['GetBetaFeaturePreferences'][] = 'EchoHooks::getBetaFeaturePreferences';

	// Global config vars
	$wgHooks['ResourceLoaderGetConfigVars'][] = 'EchoHooks::onResourceLoaderGetConfigVars';

Only the hooks in the Hook appropriate events section would need to be covered by the notification service interface.
The functionality of the house keeping hooks would probably be baked into core inline.

So, my point was: it may be simpler to think up a simple notification service interface, and then implement that interface based on existing code, then to do it the other way around, and attemt to model everything Echo does into services.

In T128351#2520383, @daniel wrote:

My point is: core code should not listen e.g. to the LinksUpdateAfterInsert, it should rather be całled directly by the LinksUpdate class. That is much more obvious and safer.

Well yes, no one is proposing to do that.

In T128351#2520391, @Legoktm wrote:

Well yes, no one is proposing to do that.

Excellent. Then let's discuss what to do instead. What interface should core be using for notifications?

Brief clarification from the ArchCom meeting (thanks, @Krinkle):

Merging Echo "as-is" still means getting rid of hooks, which is a non-trivial refactoring task.

Merging "as-is" not really a light weight option. The proposal is rather to integrate Echo's data model and UI into core as-is, rather than re-engineering it. But integration with the rest of core will have to be re-engineered in any case.

The proposal is not to move the extension into a directory in core and hook it up like an extension.

In T128351#2504237, @Tgr wrote:

• use the EchoGetDefaultNotifiedUsers hook to return a list of users to notify

Note, this is also deprecated. The recommend way to do that is now EchoAttributeManager::ATTR_LOCATORS / 'user-locators' , e.g. https://phabricator.wikimedia.org/diffusion/EFLW/browse/master/includes/Notifications/Notifications.php;e90b910f3720d1f932ec83a15e40f3ca94c08068$101 .

Minutes of RFC meeting: https://tools.wmflabs.org/meetbot/wikimedia-office/2016/wikimedia-office.2016-08-03-21.02.html

Full log is linked from there.

Quick brain dump after the IRC meeting:

• there is logic that is specific to each event type - in particular detecting the event, and deciding who should be notified (user location, aka routing). Which event types should be implemented in core is still unclear. Maybe everything that Echo covers, maybe not. * Extensions must be able to define new event types.
• there is logic that is common to all even types ("infrastructure" for registering even types, triggering relevant callbacks)
• delivery/presentation logic (email, popups, whatever) should be orthogonal to event types. Core should provide a decent implementation (probably not just email).
• many delivery mechanisms need some sort of storage. It's unclear if core can provide a generic storage facility for this purpose, or whether the needs of the various delivery mechanisms are too different.
• an essential component will be a service interface for posting (Echo)Event objects for eventual delivery, with user location / routing hidden.
  • a more low level service would take care of delivering a specific message to a specific user
• It's unclear to me how event detection can be generalized. Extensions should probably rely on hook handlers, and core can just have the detection code inlined. I can't imagine a general event detection interface.

So, there are essentially two discussions

• what infrastructure should be in core, what should its API(s) look like.
  • e.g. Buble popup delivery seems uncontroversial, storage is unclear.
• which event types should be supported in core?

IMO it's more helpful to think of it in terms of event triggering instead of event detection. Echo has two parts: a generic event handling framework with interfaces for triggering, routing and presenting events; and event-specific code that hooks into core to trigger events and hooks into the event framework to tell it what to do with them. There is not much point in trying to generalize the second part.

In T128351#2500149, @brion wrote:

Current state:

Me & Timo are in agreement that we need good enough interfaces in core that other extensions don't have to depend on Echo; also agreed that shipping Echo with core as a default extension makes sense.

Let's try to define what a "good enough interface" means. Specifically, what capabilities should be standardised in such a way that core and any extension can express those without depending on Echo.

Basic interface

• Register an event type (by canonical name and perhaps by class).
• Instantiate an event object of a certain type, with a set of standardised required and/or optional parameters associated a given event type. (Accessed via an abstract class interface defining getter methods).
• Getting a hold of the event queue.
• Emitting an event object.
• Listening to events.

This is not unlike Job, JobQueue and JobRunner. $wgJobClasses, $wgJobTypeConf (JobQueueDB), $JobQueueAggregatorNull. It's also somewhat similar to our EventRelayer interface.

Default handler

After the basics, another major decision to make is what the default core behaviour should be if no extensions are installed.

For most internal interfaces we tend to default to no-op (e.g. NullRelayer, NullLogger). This may be acceptable for notifications, but I can also see arguments to the contrary. When an extension queues a Job, it expects that work to be executed. Just like it would execute outside a job. Similarly, when an extension adopts the notifications interface (instead of automating a talk-page message or user-mail), it will expect that message to be delivered regardless of the site's configuration and installed extensions.

Following that logic, I expect we'd implement a very basic handler. Maybe it'd simply enqueue a job that mails the user, perhaps with a digest preference.

It seems a waste to demand more of the default handler as it would be similar to Echo but mostly unused. I think there's definitely room for innovative Echo alternative developed by third parties. And when that happens, people will have a choice. The reason for the default is just to make sure that extensions don't have to think about a scenario where messages are never deliverable.

We also need to decide whether a distinction between notifications to web vs mail should be standardised or left as implementation detail. For most cases I'd say this is an implementation detail. However there is one interesting exception: Notifications for when a user e-mails you. Obviously we'd don't want to notify users by e-mail about another e-mail. Though this could be mitigated by simply having the default preference be turned off for that one.

Built-in events

Just implementing the abstract interface and default handlers will solve the problem that extensions need to hardcode Echo extension references. But it won't change the fact that Echo needs a lot of hooks.

There are two types of core events that we'll probably want to do:

1. Override core behaviour: Right now, MediaWiki does have certain notification abilities already (new talk page message, watchlist etc.). These would now become the first notifications and by default e-mailed through the default event handler (instead of the hard-coded mail sending logic for those, which Echo disables). When Echo is installed, these notifications would naturally be handled by Echo instead (e.g. sent to both web and mail, and a different mail format).
2. New behaviour: Notifications for user group changes, mentions, reverts.

These would presumably by emitted by core instead of via hooks in Echo. Though extensions and site admins should be able to disable them by setting their class handler to false in the event type registry.

In T128351#2521175, @Krinkle wrote:

1. Override core behaviour: Right now, MediaWiki does have certain notification abilities already (new talk page message, watchlist etc.).

Watchlist is a complicated issue. We may not want that to use core-ified Echo. See T128352: RfC: Need to merge Notifications and Watchlist or lack thereof.

Are there any plans to move forward with this?