Page MenuHomePhabricator

Pass RecentChange object down to Watchlist notifications
Closed, ResolvedPublic

Description

Echo watchlist notifications require more properties than the Core notification.

Core uses:

  • RC::mAttribs['rc_timestamp']
  • RC::mAttribs['rc_comment']
  • RC::mAttribs['rc_minor']
  • RC::mAttribs['rc_last_oldid']
  • RC::mExtra['pageStatus']

Echo uses:

  • RC::mAttribs['rc_timestamp']
  • RC::mAttribs['rc_minor']
  • RC::mAttribs['rc_title']
  • RC::mAttribs['rc_namespace']
  • RC::mAttribs['rc_this_oldid']
  • RC::mAttribs['rc_logid']
  • RC::mExtra['pageStatus']

The easiest way to tackle this problem is to pass the RecentChange directly to the notification. When serialization is required, we can use the rc_id and to unserialize RecentChange::newFromId

Event Timeline

Change #1126997 had a related patch set uploaded (by Pmiazga; author: Pmiazga):

[mediawiki/core@master] WIP: Pass the RecentChange to Notifications

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

After a quick research, it looks like this is easy change. In EnotifNotifyJob we can just pass the rc_id. And inside the job when we want to pass the RecentChange object back, we can use RecentChange::newFromId( $this->params['rc_id'] ).

This approach will simplify newly created Core notifications as the Notification will have to handle a RecentChange object instead of multiple params ($timestamp, $summary, $minorEdit, $oldid and $pageStatus). Furthermore it will provide feature parity with Echo - which listens to RecentChange_save hook and builds notifications based on RecentChange object.

Also a note - looks like having the RecentChange will be required to handle cases like this https://gerrit.wikimedia.org/g/mediawiki/extensions/Wikibase/+/c882b34bd26f29b7f754093c199e5aeec942d92f/client/includes/Hooks/TrivialHookHandler.php#51

If we start passing RecentChange within the notification - then Wikibase could use the Middleware to stop event from happening.

Overall the flow is bit complex, as it looks like that Notification can be stopped in two ways:

  1. by $hookRunner->onAbortEmailNotification( $editor, $title, $this ) -> which is triggered in RecentChange class
  2. by $hookRunner->onSendWatchlistEmailNotification( $watchingUser, $title, $this ) hook inside the EmailNotification

I think that earlier we focused only on the latter - but we didn't investigate who is using onAbortEmailNotification

pmiazga triaged this task as Low priority.

Change #1130112 had a related patch set uploaded (by Pmiazga; author: Pmiazga):

[mediawiki/core@master] enotif: Retrieve performer and title from RecentChange

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

Change #1126997 merged by jenkins-bot:

[mediawiki/core@master] Pass the RecentChange to Notifications

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

Change #1130112 merged by jenkins-bot:

[mediawiki/core@master] enotif: Retrieve performer and title from RecentChange

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

pmiazga renamed this task from Pass RecentChange object down to Watchlist notifcations to Pass RecentChange object down to Watchlist notifications.Mar 31 2025, 1:05 PM

Change #1135726 had a related patch set uploaded (by Pmiazga; author: Pmiazga):

[mediawiki/core@master] Enotif: Keep ENotif business logic within ENotif classes

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

Change #1135726 merged by jenkins-bot:

[mediawiki/core@master] Enotif: Keep ENotif business logic within ENotif classes

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

Change #1136420 had a related patch set uploaded (by Pmiazga; author: Pmiazga):

[mediawiki/core@master] WIP: Pass RecentChange back to EmailNotif

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

Change #1136431 had a related patch set uploaded (by Pmiazga; author: Pmiazga):

[mediawiki/core@master] POC: Idea: Refactor logic whether to send notifications

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

Change #1136760 had a related patch set uploaded (by Pmiazga; author: Pmiazga):

[mediawiki/core@master] Enotif: Pass RecentChange down to Notifications

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

Change #1136420 merged by jenkins-bot:

[mediawiki/core@master] Pass RecentChange back to EmailNotif

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

Change #1136760 merged by jenkins-bot:

[mediawiki/core@master] Enotif: Pass RecentChange down to Notification and MailComposer

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

Change #1137363 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/core@master] Enotif: Document some EmailNotification methods as @internal

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

Change #1137363 merged by jenkins-bot:

[mediawiki/core@master] Enotif: Document some EmailNotification methods as @internal

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

Change #1136431 merged by jenkins-bot:

[mediawiki/core@master] recentchanges: Refactor logic on whether to send notifications

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