Page MenuHomePhabricator

RfC: Notifications in core
Open, HighPublic

Tokens
"Love" token, awarded by MGChecker."Like" token, awarded by Liuxinyu970226."Barnstar" token, awarded by Zppix."Love" token, awarded by matej_suchanek."Barnstar" token, awarded by daniel."Dislike" token, awarded by Ricordisamoa."Like" token, awarded by Luke081515."Like" token, awarded by Bawolff."Like" token, awarded by Anomie."Like" token, awarded by fbstj."Like" token, awarded by Krenair."Like" token, awarded by Florian."Like" token, awarded by Jdforrester-WMF.
Assigned To
None
Authored By
Legoktm, Feb 29 2016

Description

Summary of this RFC

This proposes that we move part of the implementation of the Notification framework RFC currently in the Echo extension into MediaWiki core.

  • Being in an extension, Echo has to use ugly hooks to suppress some core features and has generally weird interactions with some things.
  • extensions that wish to use it have to do conditional checks every time they'd like to trigger a notifications.
  • Echo is a reserved word in PHP, preventing us from using it as the namespace for a PHP class. ;-)

Note: This RFC doesn't answer the questions in RFC: Need to merge Notifications and Watchlist or lack thereof (T128352)

Full details

mw:Requests_for_comment/Notifications_in_core

RFC meetings

Related Objects

View Standalone Graph
This task is connected to more than 200 other tasks. Only direct parents and subtasks are shown here. Use View Standalone Graph to show more of the graph.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
brion added a comment.Mar 2 2016, 9:46 PM

I like this. Echo has become a critical feature of any wiki.

Yay!

One concern I have - And maybe this isn't something that should be related to the RFC. I've noticed that there seems to be some confusion about how echo interacts with anon users, and if the current lack of echo for anons is ideal. Maybe it should be clarified what we want to do in that regard before merging into core (I'm not saying it should be fixed, just that we should know where we want to go with that in the future). Or maybe I'm wrong on that account. I don't really follow echo development very closely.

*nod*... @Bawolff can you file a separate task and we'll discuss further on that, maybe develop it into an RfC or smaller work plan in future.

I like this. Echo has become a critical feature of any wiki.

Yay!

One concern I have - And maybe this isn't something that should be related to the RFC. I've noticed that there seems to be some confusion about how echo interacts with anon users, and if the current lack of echo for anons is ideal. Maybe it should be clarified what we want to do in that regard before merging into core (I'm not saying it should be fixed, just that we should know where we want to go with that in the future). Or maybe I'm wrong on that account. I don't really follow echo development very closely.

*nod*... @Bawolff can you file a separate task and we'll discuss further on that, maybe develop it into an RfC or smaller work plan in future.

Submitted as T128663

brion added a comment.Mar 2 2016, 11:14 PM

Ok the anon issue got duped to T58828 per legoktm; turns out we already have an old task in the system for this topic. :D

Always worth checking. :) Any particular candidates for a split-out to libs?

Off the top of my head, not really. Nearly all notifications have to be aware of the User they're for, and the Title they're about, introducing hard dependencies upon MW. That said, we should make sure we have clean interfaces and proper separation like the difference between EchoEvent (model) and EchoEventMapper (database logic).

Which version of MediaWiki is this targeted for?

None in particular, whenever I or someone else has time to do it :)

I mostly agree with Krenair: I think the general idea is fine, but I'm hesitant about retaining a separate Echo extension after a merge.

My current view is that we should do a full merge of the extension. What's the argument against putting everything in Echo in core? Are there really wikis that want notifications, but don't want user mentions? For "thank-you-edit", maybe put that in the Thanks extension? :-) That leaves "a page you created has been linked to" ("page-linked"), which I'm not sure anybody actually wants to maintain/support, so that feature might just get killed altogether.

This seems fine to me as well. My main objection is the code that implements mentions kind of sucks (has an alternative parser and all that), and a little less so for page-linked notifs.

  • We have to maintain a backward-compatibility layer of some kind in the extension.
    • Would we have to do this in core as well? What are we talking about here? Deprecation warnings on a few functions/entry points?

When moving it to core, I was planning to rename all the "Echo" references to "Notifications", so EchoEvent -> MediaWiki\Notifications\Event or something. Plus all of the global settings and more stuff. We would want some kind of back-compat layer for all of that, and it could go in core yes.

(more responses to come later)

This seems fine to me as well. My main objection is the code that implements mentions kind of sucks (has an alternative parser and all that), and a little less so for page-linked notifs.

I see no reason to get rid of page-linked. They are a motivating factor for article creation (which is important to counteract the demotivating factors). However, it would be useful to have a way to control which pages it triggered on.

Izno added a subscriber: Izno.Mar 4 2016, 4:34 PM
Krinkle added a subscriber: Krinkle.Mar 4 2016, 5:55 PM

Let me express some concerns here.
As a user, I'd be very disappointed if the Echo system were made part of core as it is now. In my opinion, Echo was a good stopgap to improve collaboration but the whole system is in need of a redesign.

brion added a comment.Mar 5 2016, 8:13 PM

Let me express some concerns here.
As a user, I'd be very disappointed if the Echo system were made part of core as it is now. In my opinion, Echo was a good stopgap to improve collaboration but the whole system is in need of a redesign.

Can you elaborate?

Krinkle added a comment.EditedMar 9 2016, 9:24 PM

TL;DR:

  • Provide base classes and interfaces in MediaWiki core: PHP interfaces for event creation, notification backend and notification handler. As well as an API module.
  • Bundle Echo extension with tarball releases.

I agree with @Ricordisamoa that Echo's current user interface and database model should not become a mandatory user-facing feature of MediaWiki core.

The PHP interfaces for creating and capturing event should be in MediaWiki core so that extensions can cleanly interact with it. However I think it would be beneficial in the long-term if they are a no-op by default (e.g. $wgNotificationHandler=false, or $wgNotificationBackend=false). Similar to RCFeedEngine perhaps.

The AP module I to retrieve notifications could also be in core (perhaps auto-enabled if there is backend it set, or returns a a warning with empty results if no backend set).

As a next step we can also start bundling Echo with core for tarball releases. Remember that right now Echo is not bundled with tarballs. As such it is in my opinion still relatively unexposed to the wider MediaWIki development community. This will help uncover any further issues in the next release cycle whilst still allowing sysadmins to easily opt-out by unregistering the extension. Or they could work on porting any existing systems they have to use these interfaces (I believe WikiHow and Wikia may have their own extensions already).

Echo would continue to exist as an extension and act as event creator (e.g. for mentions), as well as notification backend (db table) and handler (user interface).

The event creation part could probably also be moved into MediaWiki core in a generic way.

TL;DR:

  • Provide base classes and interfaces in MediaWiki core: PHP interfaces for event creation, notification backend and notification handler. As well as an API module.
  • Bundle Echo extension with tarball releases.

    I agree with @Ricordisamoa that Echo's current user interface and database model should not become a mandatory user-facing feature of MediaWiki core.

    The PHP interfaces for creating and capturing event should be in MediaWiki core so that extensions can cleanly interact with it. However I think it would be beneficial in the long-term if they are a no-op by default (e.g. $wgNotificationHandler=false, or $wgNotificationBackend=false). Similar to RCFeedEngine perhaps.

    The AP module I to retrieve notifications could also be in core (perhaps auto-enabled if there is backend it set, or returns a a warning with empty results if no backend set).

    As a next step we can also start bundling Echo with core for tarball releases. Remember that right now Echo is not bundled with tarballs. As such it is in my opinion still relatively unexposed to the wider MediaWIki development community. This will help uncover any further issues in the next release cycle whilst still allowing sysadmins to easily opt-out by unregistering the extension. Or they could work on porting any existing systems they have to use these interfaces (I believe WikiHow and Wikia may have their own extensions already).

    Echo would continue to exist as an extension and act as event creator (e.g. for mentions), as well as notification backend (db table) and handler (user interface).

If Echo continues to act as event creator, how is the issue of Echo replacing core notifications addressed? E.g. Echo has its own notifications (regular and email) it creates for user talk edits. If Echo is turned on and 'edit-user-talk' is enabled, it takes over the user talk email notification with AbortTalkPageEmailNotification. I think this one of the hooks @Legoktm is referring to in the description.

If Echo is not part of core, I can think of three options:

  1. Core doesn't send this notification at all if neither Echo nor another $wgNotificationHandler backend is installed.
  2. Core has its own simple $wgNotificationHandler for such cases.
  3. It basically works as it does now, and extensions like Echo have to keep doing this hooking. That removes what I think is the primary motivation for this RFC (a way to clean up this duplicate functionality). Checking for Echo being installed is easy to centralize in each extension, so it's not a big issue.

We'll have to discuss the pros and cons of making it pluggable.

TL;DR:

  • Provide base classes and interfaces in MediaWiki core: PHP interfaces for event creation, notification backend and notification handler. As well as an API module.
  • Bundle Echo extension with tarball releases.

    I agree with @Ricordisamoa that Echo's current user interface and database model should not become a mandatory user-facing feature of MediaWiki core.

    The PHP interfaces for creating and capturing event should be in MediaWiki core so that extensions can cleanly interact with it. However I think it would be beneficial in the long-term if they are a no-op by default (e.g. $wgNotificationHandler=false, or $wgNotificationBackend=false). Similar to RCFeedEngine perhaps.

That seems like abstraction for the sake of abstraction, and I don't see it as an advantage. Why do you think it would be beneficial? RCFeedEngine comes with actual implementations in core that are usable and configurable with just MediaWiki core. As other people have expressed on this ticket, notifications have become an integral part of how MW works, and making individual wikis install extra things or need extra config is a step in the wrong direction IMO.

As a next step we can also start bundling Echo with core for tarball releases. Remember that right now Echo is not bundled with tarballs. As such it is in my opinion still relatively unexposed to the wider MediaWIki development community. This will help uncover any further issues in the next release cycle whilst still allowing sysadmins to easily opt-out by unregistering the extension. Or they could work on porting any existing systems they have to use these interfaces (I believe WikiHow and Wikia may have their own extensions already).

Per https://wikiapiary.com/wiki/Extension:Echo, about 40% of the wikis using the extension are not Wikimedia wikis. wikiHow uses a modified version of Echo, and I wouldn't call what Wikia runs MediaWiki anymore. I have no objections to bundling it in the tarball prior to merging it into core though.

Let me express some concerns here.
As a user, I'd be very disappointed if the Echo system were made part of core as it is now. In my opinion, Echo was a good stopgap to improve collaboration but the whole system is in need of a redesign.

Can you elaborate?

As I wrote at T93109#1156473, I think notifications are bound to have different priorities. Echo's flyout is good to draw the user's immediate attention, but sometimes one couldn't care less about a supposedly important item. On the other hand, the watchlist only works by page (not by user, etc.) and interesting events can get easily missed. Users should be able to choose better the events they want to be notified about, perhaps with both manual control and AI. Again, this is from a user's perspective and I don't know anything about feasibility. Echo might get better within core but I doubt it.

On the other hand, the watchlist only works by page (not by user, etc.) and interesting events can get easily missed. Users should be able to choose better the events they want to be notified about, perhaps with both manual control and AI.

Unifying watchlist and Echo would be much easier if Echo were in core.

brion added a comment.Mar 9 2016, 10:43 PM

Quick note on my position after IRC discussion:

Having the interfaces available in core is I think very important for establishing server->user notifications as a standard part of the MediaWiki platform which can be reliably used by features both in core and extensions. For this reason I tend to think that decent email-based and on-web notification view implementations are also required in core, or the system can't be relied upon to actually work.

Thus I tend to advocate for a fairly feature-complete merge over an internal-API-only merge with all implementation done separately in the extension (even if it's bundled in the tarball, if it's not always enabled it can't be relied on as part of the platform).

Mattflaschen-WMF added a comment.EditedMar 9 2016, 10:52 PM

I think the simplest option is to:

  • Bundle Echo in the tarball for a release (it's a popular extension already, but it doesn't hurt to do this to demonstrate that make sure sysadmins widely use it before it's in core).
  • Merge Echo into core piece by piece.

If I understand correctly, Krinkle's interface proposal is roughly:

  1. Move some Echo email logic to core.
  2. Refactor core's existing enotif code to provide a wgNotificationHandler.
  3. Have Echo's email class subclass core's email one. Echo would provide its own wgNotificationHandler with more sophisticated functionality, including a web UI (popup/flyout and special page), plus bundling and better email.

For Echo to really reuse core's email notification functionality (to not have two implementations), core would have to change to use Echo's email notification formatting. At that point (Echo email's been brought into core), why not also bring the Echo web notifications to core?

Tgr added a comment.Mar 9 2016, 11:39 PM

It would be nice if MediaWiki was built as a set of loosely-coupled components (such as, say, Symfony2 where messages, resource loading, pages etc. are separate components), and in such a world making notifications a separate component would make perfect sense. We are not in that world, and extensions are not components: they exist to add or replace functionality, not to package default functionality needed by everyone. There is a maintenance burden to extensions which replace core functionality (patches must be split up and are more complicated to review and deploy; relevant code is harder to find; CI setup must be duplicated); I don't see what we would gain here by paying that burden.

MW core is expected to be marginally useful even without extensions; I wouldn't consider it reasonable to have a core where e.g. you ask for a password reminder and nothing happens because the notification went to /dev/null. So email notification support should be in core, and at least the concepts for web notifications need to be in core (things like read/unread and bundling and message expiration need to be part of the interface); it would be possible to put the interface for web notifications in core, and leave the implementation in Echo, but again that seems like maintenance cost (every change had to be coordinated in two places) with unclear benefits.

RobLa-WMF mentioned this in Unknown Object (Event).May 11 2016, 12:09 AM
RobLa-WMF triaged this task as High priority.May 18 2016, 6:20 PM

This RFC seems closely related to T102476, which we're discussing today at E184: RFC Meeting: RFC: Requirements for change propagation (2016-05-18, #wikimedia-office)

daniel added a subscriber: daniel.Jul 27 2016, 8:51 PM

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.

brion added a comment.Jul 27 2016, 8:52 PM

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.

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)

Tgr added a comment.Jul 28 2016, 11:18 PM

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.

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?

Tgr added a comment.Jul 29 2016, 1:54 AM

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.

Tgr added a comment.Jul 29 2016, 9:45 PM
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.

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.

daniel added a comment.Aug 2 2016, 3:57 PM

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.

Tgr added a comment.Aug 2 2016, 4:47 PM

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.)

Zppix awarded a token.Aug 2 2016, 4:50 PM
Zppix added a subscriber: Zppix.

+1

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

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?

daniel added a comment.EditedAug 3 2016, 7:48 PM

@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.

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.

daniel added a comment.Aug 3 2016, 7:55 PM

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?

daniel added a comment.Aug 3 2016, 8:25 PM

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.

Mattflaschen-WMF added a comment.EditedAug 3 2016, 8:39 PM
  • 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 .

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?
Tgr added a comment.Aug 3 2016, 10:59 PM

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.

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.

  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.

Liuxinyu970226 awarded a token.
brion removed brion as the assignee of this task.Jan 29 2018, 7:42 PM

de-assigning, not active on this

Are there any plans to move forward with this?

fbstj added a subscriber: fbstj.Jun 16 2018, 3:51 PM
Restricted Application added a project: Growth-Team. · View Herald TranscriptApr 23 2019, 1:38 PM