The first time I noticed a second notification for the same reply while no link, quote or mention was added in the edit was after the edit on Topics from some categories do not appear on /latest - #36 by JammyDodger. This case is slightly different than my repro steps below, but I think the underlying issue is the same.
The second post where this happened was on Messages section for sidebar - #13 by nathank. It was similar: The edit didn’t add anything that would result in a notification - the quotes both had been there before - and still I was notified a second time.
Here are the steps to reproduce I found that worked [1]
You need 3 users: OP, notifiedUser, spammer
OP creates a topic
notifiedUser replies
OP replies to the post of notified user notifiedUser is notified about the reply (expected)
spammer replies to the post of notifiedUser. The reply contains a link to another post by notifiedUser and a quote from the post you reply to. (optional: you can also @mention notifiedUser) notifiedUser is notified about the reply (expected) [In case you added an @mention the notification is about the @mention (expected)]
notifiedUser reads the new replies to mark the notifications as read and navigates back somewhere else so we won’t miss a notification.
spammer edits the reply and fixes a typo (or adds edit1) notifiedUser is notified about being quoted (unexpected, they were notified about this reply before and the quote has been there before, no need to tell them)
spammer edits the reply again to fix another typo (or adds edit2) notifiedUser is notified about being linked (unexpected, they were notified about this reply before and the link has been there before, no need to tell them)
The video only shows the final steps 5-7. Spammer is on the left, notifiedUser on the right
at least most of the time, sometimes even adding an @mention in the edit doesn’t trigger a new notification ↩︎
Change lines 589-599 from:
# linked, quoted, mentioned, chat_quoted may be suppressed if you already have a reply notification
if [
Notification.types[:quoted],
Notification.types[:linked],
Notification.types[:mentioned],
Notification.types[:chat_quoted],
].include?(type)
if existing_notifications.find { |n| n.notification_type == Notification.types[:replied] }
return
end
end
To:
# linked, quoted, mentioned, chat_quoted may be suppressed if you already have any notification about this
post
if [
Notification.types[:quoted],
Notification.types[:linked],
Notification.types[:mentioned],
Notification.types[:chat_quoted],
].include?(type)
return if existing_notifications.any?
end
This will work but I do worry a bit cause there are other notifications that can slip through here. (plugins notifications for example that are ones we may want to suppress)
@lindsey there is a bit of a product question here, when should we suppress a notification?
I’m not sure. Isn’t that a reply notification in my video? And yet, there is still a notification about the quote and the link afterward. So, extending that for other notification types might not help here. But maybe it covers other edge cases.
I wonder if the problem in my repro is the “2 replies” notification. Does that break the check for existing notifications by that reply when the reply is edited?
In general, I would expect no additional notification because of an edit if all the triggers had already been in the post when I read it before. Fixing a typo unrelated to mention/link/quote shouldn’t result in a new notification.
I think, in case a trigger is replaced, I would rather like the existing, unread notification to be changed (or replaced) than receiving a second one. Removing the @mention in a user’s post to avoid noise shouldn’t result in a second notification about the quote. I would expect the notification about the @mention to disappear.
I think the only case where I’d want a new notification is if an edit adds a more important notification type. So when someone linked my post and later adds an edit which @mentions me, it makes sense to tell me because they seem to be talking no longer about something I wrote but directly to me. Since an edit no longer bumps the topic, @mentions might be a helpful way to alert users about edits.
I think Moin describes the ideal from the user perspective, but I think we’d struggle to get this part right:
Perhaps we could manage that for in-app notifications, but we can’t unsend push notifications or email notifications. (In general I’m hesitant to add more complexity to notifications, so while some might be okay with in-app and email/push for the notification being different, I’d prefer we forego this.)
I agree with this, though:
If a post should notify a user (e.g., post author quotes User A), notify the relevant user(s) (e.g., User A gets a notification that they’ve been quoted).
If an edit to that post does not change who should be notified or why (e.g., post author edits a typo), don’t notify anyone.
If an edit to that post does change who should be notified (e.g., post author mentions User B) or why (e.g., the post author mentions user A), notify the impacted user(s) (e.g., User B gets a mention notification, User A gets a mention notification).
@lindsey would like … new notifications for new information edited into post
Full alignment here requires pretty complex tracking… I am assessing the size of the change, but to be honest just doing nothing for now is probably easiest, cause the change will end up being very fragile.
Extracting which mentions are new and which are old is something that is going to require either a parser (we don’t parse at that point) or fragile regex.
Moving this to feature @Moin and closing my PR for now
Above all, I do not want to receive another notification if nothing has changed in the post that would cause a notification. Correcting a typo should not notify me if I have already been notified just because 2 notifications about the same topic were collapsed before. And I never noticed that for years until it happened in December.
I asked ChatGPT to write a summary of what it told me about this issue, when I tried to find repro steps based on the code
Summary: Why duplicate notifications are normally prevented, and why it fails hereAI
Core issue
Duplicate-prevention relies on finding a reply notification that is still scoped to the edited post, but reply notifications are collapsed, destroyed, or re-attached to a different post.
As a result, edit-time suppression cannot reliably detect that the user was already notified about the reply.
Duplicate notifications are prevented in PostAlerter by:
Tracking users already notified during post creation
notified = []
Suppressing secondary triggers if a reply notification exists
if [:quoted, :linked, :mentioned, :chat_quoted].include?(type)
return if existing_notifications.find { |n|
n.notification_type == Notification.types[:replied]
}
end
This is meant to avoid notifying a user twice about the same post.
Collapsing reply notifications
Reply notifications (:replied) are collapsed per topic:
so the notification no longer represents the post that originally triggered it. As a result, suppression logic that relies on matching post_number cannot reliably detect prior notifications for edited posts.
Why this fails in the repro case
In the repro:
The user is notified about the reply initially
When multiple replies exist, reply notifications are collapsed
The collapsed notification:
may no longer exist for the edited post, or
exists with a different post_number
On edit:
reply logic does not run (new_record == false)
suppression checks only for a post-scoped :replied notification
none is found → suppression fails
Quote/link detection runs again and creates new notifications
From the user’s perspective:
“I was already notified about this reply.”
From the code’s perspective:
“There is no existing notification for this post.”
When I create a topic mentioning my testuser in a public category and move it to a private category after a few minutes, the notification is also removed from the interface - even though they received an email about the @mention.
So in-app notifications that are deleted, even though an email was sent, do already exist.