What about resolve/lock? I'm assuming we don't want to hide those topics (lock is a moderation action), just confirming.

Confirmed. Just for hide/delete/suppress.

One of the challenges here is when to filter out notifications.

Filtering at rendering time with canRender()

• Relatively easy to implement inside each presentation model.
• Notifications still get included in the counters (read and unread). The user could see bright icons with X unread notifications, click on them and see nothing new. They would also never be marked as read and the discrepancy would persist until they fall off the 2000-limit cliff. It may sound like an edge case but vandalism on popular pages would create that situation for a lot of users.
• canRender() may have to execute queries. That leads to a lot of queries that are executed every single time the notifications are fetched. On the other hand, they would show up again if the topic is unmoderated.

Removing notifications when they become irrelevant (i.e. when a topic is moderated)

• The challenge here is to locate all notifications that are about a certain object that can be moderated. This kind of information is currently stored as json in echo_event.extra and often contains serialized UUID instances, which is hard to query.

Some notes from IRC about this (I removed unrelated conversations that were interspersed).

[Monday, August 31, 2015] [4:50:47 PM] <matt_flaschen> legoktm, we were talking in the meeting about potentially marking the echo_event with a flag (here be dragons) for hide/delete/suppression. This would be done at hide/delete/suppress time, and if the flag is 1 it would have to do special handling at display-time.
[Monday, August 31, 2015] [4:50:50 PM] <matt_flaschen> In theory, we wouldn't have to clear the flag on undelete/unsuppress, as long as the formatter handled the current state correctly. This would save time on the 99.9.. of events with the flag 0.
[Monday, August 31, 2015] [4:53:13 PM] <legoktm> matt_flaschen: sounds like a good idea. except we have problems with doing this already (https://phabricator.wikimedia.org/T106219) where _deleted columns aren't updated properly...
[Monday, August 31, 2015] [4:53:40 PM] <legoktm> matt_flaschen: I think we need to have better batching instead of lots of individual look ups, and I have a few ideas for that
[Monday, August 31, 2015] [4:54:15 PM] <legoktm> $formatter->preload( EchoEvent[] );
[Monday, August 31, 2015] [4:54:23 PM] <legoktm> $formatter->format( EchoEvent )...
[Monday, August 31, 2015] [4:55:11 PM] <matt_flaschen> legoktm, yeah, makes sense, that's what we do for Flow integration in various places (e.g. onChangesListInitRows)
[Monday, August 31, 2015] [4:55:38 PM] <matt_flaschen> I don't have access to that bug, but yeah, it would definitely require being very good about setting the flag to 1 when required.

That would create the problem you listed at the end, unless we reimplement moderation (which we talked about at one point; there should be a task somewhere)

I don't think we should focus on moderation too much: there may also be other reasons why we would not want to render a notification, other than "the thing" having been moderated.

Some server/service may not be available or data may not have persisted correctly. The thing the notification was about may have been revoked/undone. The notification has changed since it was created & now needs more/other data to render. I'm sure plenty more things could happen.

We should probably focus on more than just suppression.
Or maybe we dont have to do anything and having the occasional "miscalc" is okay (it probably wouldn't happen too often, and if it does, it's likely for people getting a lot of notifications already who may not even notice) - do we have any numbers here?
And if we do fix something, at the very least we should make sure that programming mistakes don't cause things like that: maybe we should run canRender() prior to inserting the event?

In T93673#1997810, @matthiasmullie wrote:

[...] The thing the notification was about may have been revoked/undone.

Notifications represent "events" that happen and not the current state of the system. If you are added to a group and immediately removed we show those 2 notifications. Similarly, if someone writes on your talk page and reverts their own edit, the 'edit-user-talk' is still there and links to the proper revision.

We should probably focus on more than just suppression.

Moderation is the only case I know of that is worthy of re-writing history. I would like to see more examples to get a better understanding of the problem.

Or maybe we dont have to do anything and having the occasional "miscalc" is okay (it probably wouldn't happen too often, and if it does, it's likely for people getting a lot of notifications already who may not even notice) - do we have any numbers here?

I would also like to see those number. I expect this to happen by bursts when there is vandalism.

Someone mentioned the idea of showing an indicator that there is something new or unseen instead of the exact number of new notifications. That could minimize the occasional "miscalc".

And if we do fix something, at the very least we should make sure that programming mistakes don't cause things like that: maybe we should run canRender() prior to inserting the event?

Formatters can try/catch presentation models and make sure to log and discard those who don't play nice. We don't have to rely on, or trust, canRender() for that.

Notes from a conversation with @Catrope on March 1st, 2016:

We should try to moderate notifications as the same time as moderating content.

Inspired by the log_search table, we could have an Echo table that maintains the relationships between events and pages. The relationship we're talking about here is 'moderated-with', like in the sentence: "Notifications about event X are moderated with page Y".

Unlike the Flow permission system, this solution would remove events associated with a moderated Flow object (topic, post) regardless of if you have the permission to still see it or not. Looping on individual notifications and checking if the associated user can still see it is possible but probably not viable.

Notifications could be marked as deleted instead of removed completely so they can be unmoderated.

The relationship table could also hold the 'read-on' relationship to specify on which page(s) a notification can be read and mark it as read when the page is visited. This is currently stored in the echo_target_page table but @Catrope expressed that this table is not great and a replacement would be welcome.

For the record, things I don't like about the echo_target_page table:

• It should have a unique index on (event, page, user)
• That unique index should be the primary key, there shouldn't be an auto-increment ID field
• I'm not quite sure whether the user field is useful. On the one hand, I don't see why the set of pages for an event would differ by user; on the other hand, this allows us to only keep this data around for unread notifications (when a notification is marked as read, its echo_target_page entries are deleted).

Change 275049 had a related patch set uploaded (by Sbisson):
[WIP] [POC] Moderate notifications

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

Change 275052 had a related patch set uploaded (by Sbisson):
[WIP] [POC] Moderate notifications

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

Change 275049 merged by jenkins-bot:
Dynamic bundles

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

Change 275052 merged by jenkins-bot:
Moderate notifications

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

Related bugs were filed as T140836: [betalabs-Regression] cross-wiki bundle displays only wiki names - no notifications and T140327: [betalabs-Regression] Incorrect count when only cross-wiki notifications are present.

Test cases to check after the fix
(1) Topic deletion

• user A creates a new topic
• other users do the following actions: Reply, Thank - 2 times, Summary, Description, Topic title, Resolve, Re-open
• user A successfully receives all notifications that those events triggered
• another user deleted the topic

Expected result: the notifications are purged from the flyout and from Special:Notifications page
Actual result:

• the flyout counter gets updated only after repeated opening of the flyout
• the bundled Thanks get unbundled and not purged
• both - the flyout and Special:Notifications page display 'Thanks'; clicking on them redirects to "This topic has been deleted." page.

(1a) Deleted topic restored
Expected result All notifications are restored.
Actual result All notifications are restored

(2) Post deletion

• user A creates a post
• other users: Edit post - 2 times; Thank - 2 times; Reply to post - 2 times.
• another user deletes the post

Expected result All notifications are purged.
Actual result

• 'Thanks' are not purged
• Special:Notifications counter (mw-echo-ui-paginationWidget-label) does not take into account that some notifications are purged

(2a) Post restored
Expected result All notifications are restored.
Actual result All notifications are restored

(3) Flow board deletion

• user A has a Flow board on his watchlist
• user A receives several notifications from the event/actions on the watched Flow board
• another user deletes the Flow board

Expected result All notifications from the Flow board should be purged
Actual result

• the notifications from deleted Flow board are purged
• Special:Notifications page does not show previous notifications that are unrelated to deleted Flow board - specifically Alerts; the flyout displays them.

(3a) Flow board restored
Expected result All notifications are restored.
Actual result All notifications are restored

Change 300898 had a related patch set uploaded (by Sbisson):
Delete 'flow-thank' on moderated topic/post

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

Moderation of flow-thank notifications should be fixed by https://gerrit.wikimedia.org/r/300898

I believe the other problems have the same cause (canRender() { return false; }) as T140327 and T140836

Change 300898 merged by jenkins-bot:
Delete 'flow-thank' on moderated topic/post

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

Checked in betalabs

From UI - the 'Thank' notifications are not purged, and overall, the set of deleted notifications does not show thanks notifications as deleted.

mysql> select distinct(event_type) from echo_event where event_deleted=1;
+---------------------+
| event_type          |
+---------------------+
| flow-new-topic      |
| flow-post-edited    |
| flow-post-reply     |
| flow-summary-edited |
| flow-topic-resolved |
+---------------------+
5 rows in set (0.75 sec)

Documentation updated.

Change 301617 had a related patch set uploaded (by Sbisson):
flow-thank: add a target-page entry for the topic page

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

Change 301618 had a related patch set uploaded (by Sbisson):
Moderate events that don't have target-page entries

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

I'm not sure what the sub-tasks are used for but while they are related there is absolutely no cause-consequence relationship between all of those.

Change 301617 merged by jenkins-bot:
flow-thank: add a target-page entry for the topic page

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

(3) Flow board deletion , (1) Topic deletion - checked; All notifications associated with the deleted board were purged from the flyout and from Special:Notification page.

The following events were checked:
flow-new-topic
flow-post-edited
flow-post-reply
flow-summary-added
flow-topic-resolved
flow-topic-renamed
flow-topic was re-opened
summary was updated
mention
thanks
flow-description-edited

The same notification events were checked for flowusertalk page.

(2) Post deletion scenario - the notifications are not purged. Filed as a separate task - T142758: Notifications from moderated post are not purged

T142758: Notifications from moderated post are not purged was checked - all notifications generated from deleted/suppressed posts are successfully purged (flow board posts were checked including flowusertalk pages).

Change 301618 merged by jenkins-bot:
Locate events using EventMapper instead of TargetPageMapper

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

Two fresh reports of editors seeing notifications for topics that had been moderated

• https://www.mediawiki.org/wiki/Topic:Tauymllg06e18ero
• https://www.mediawiki.org/wiki/Topic:Tavk2hlnv3ytwor1

Does this task need to be re-opened?

In T93673#2607119, @Quiddity wrote:

Two fresh reports of editors seeing notifications for topics that had been moderated

• https://www.mediawiki.org/wiki/Topic:Tauymllg06e18ero
• https://www.mediawiki.org/wiki/Topic:Tavk2hlnv3ytwor1

Does this task need to be re-opened?

It seems like in both cases the notifications were not visible but the count was incorrect.

According to the db the notifications were deleted correctly when the topics were moderated.

I tried reproducing the problem locally with 3 users on 2 wikis but I couldn't. Here's what I did:

1. User1 creates garbage topics on wiki1
2. User2 sees notifications and increased count on wiki1 and wiki2
3. User3 (admin) deletes all User1 contributions using Special:Nuke
4. User2 doesn't see the notifications count has decreased on wiki1 and wiki2

@Etonkovidova did you ever see this problem when testing moderation?

@SBisson

• looking at the snapshot https://snag.gy/M9s0AI.jpg - the problem seems that the opening cross-wiki bundled notification does not display anything
• after intensive fixing, the counters when moderated topics/posts behaved correctly. Although, log in/log out is required (refreshing a page often was not enough)
• filed a bug about somewhat misleading count of multiple edits when those edits are not explicitly bundled - T144860: Not bundled multiple edits are counted as separated notices in badge counter
• when testing, I did not use Special:Nuke - might be some discrepancies there
• should the moderated topics/pages be deleted from Watchlist?

I will be testing moderated notifications more and add comments to that ticket.

Re-tested in betalabs (unfortunately there were couple of hiccups in betalabs env today):
@Quiddity: the main question: do the counters (the badge counter and the counter on Special:Notifications page) show correct numbers after a user log out/logs in? If an incorrect count persists, it'd be a more severe problem.

If pages/topics were deleted during a user session, the counters would still display not-updated count - which does not get updated with page refreshing. Alerts notifications seem to be particularly stubborn - no page refreshing or navigating away can change their incorrect count.

Below are the screenshots that illustrate some possible confusing cases:

• Special:Notifications page left panel states that there must be one unread notifications, but filtering for 'Unread' brings zero results cause the page/topic were deleted

• the page that triggered alert notification was deleted - Alerts are displayed red, but 'Failed to fetch notifications' is displayed

• Alerts badge displays that there is one unread notification, but opening the panel shows that all notifications have been read

• it seems there is no difference in how page/topic/post were deleted via page/topic/post drop-down menu option or via Special:Nuke

In T93673#2607119, @Quiddity wrote:

Two fresh reports of editors seeing notifications for topics that had been moderated

• https://www.mediawiki.org/wiki/Topic:Tauymllg06e18ero
• https://www.mediawiki.org/wiki/Topic:Tavk2hlnv3ytwor1

Does this task need to be re-opened?

Very likely fixed by https://gerrit.wikimedia.org/r/#/c/309465

Change 310063 had a related patch set uploaded (by Sbisson):
resetNotificationCount() from replica with no lag

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

Change 310063 merged by jenkins-bot:
resetNotificationCount() from replica with no lag

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