Editing posts cause notification

Sorry fort he lack of clarity.
For some reason, editing a post that I created would notify other users of the edit. It appears that it occurred in topics that used the tagging of a group and that group would be notified (even if the tag was not a new one).

  1. Create a post with multiple tags of groups of members
  2. Let it propagate and send notifications. Wait whatever number of minutes is needed for it to send out to any current watch lists/etc.
  3. Come back to the post and edit it.
  4. Add something useless, like the word unicorn and note that the right hand side of the edit window alerts that you will notify the groups in the post despite not having added or edited anything to do with those tags.
  5. Save
  6. Check the logs and note that all members in those groups are now notified of the edit (though the notifications appear as a New Topic notification.
3 Likes

Interesting so this is specific to group mentions? Perhaps you can try to repro this when you have time @jomaxro

It appears to be on my end at least.

I have not been able to repro this. I tested it on try, with 3 users. In one post I mentioned all of them individually, in the other I mentioned a group all 3 were in. They all received the initial mention notification, they did not receive further notifications on edit.




I’m unsure about the type of notification here. Are you referring to a notification that appears on your user profile, i.e., as an extra entry in the list that appears when you click on your avatar up right?

Or is it a small blue dot next to the topic title in the topic list?

1 Like

I am observing this behavior on one of our sites.

When editing a post that mentions a group, I get the following warning:

And can also confirm that the people in that group do receive a new notification when the edit is saved.

2 Likes

Can we repro this @tshenry ?

1 Like

Hmm @Grayden_Shand I am assuming the post had already mentioned the group prior to being edited? I know if you add a new mention to a post through an edit, that user/group will be notified.

I tested two scenarios:

  1. If you make a “ninja edit” to a post that already mentioned a group, the warning will display while editing, but no notification is sent after the ninja edit is made
  2. If you make an edit outside of the edit grace period, the warning will display while editing, but no new notification is sent out to the group

Can you help with any additional reproduction steps?


As an aside I could see it being helpful to have a toggle that controls whether a mention triggers a notification at the post level. It would provide more control in edit scenarios, and also when you just want to include a magic usercard/profile link in the post for other users to make use of.

2 Likes

Yes, the group mention was already in the post before being edited. Also, the user making the edit was a different user than the one who owned the topic.

I will try to repro on my dev setup today and report back if I’m successful.

1 Like

Figured it out…

The notification is being sent to users who joined the group after the post was originally created.

So, for the users who are receiving the notification, it is the first time that THEY received the mention notification, but not the first time that ANYONE got the notification.

But people who were in the group to begin with (and did receive the original mention notification) do not receive an additional notification from the edit.

Steps to reproduce (requires two users)

  1. Create a new group
  2. Create a new post or topic that mentions that group
  3. Create a new user
  4. Add that user to the group
  5. Edit the post – At this point the new user will receive a notification.
  6. Edit the post again – The new user does not receive additional notifications for additional edits.

Not sure what the correct behavior should be here… I guess I would lean towards not sending the mention notifications on edits at all, but I’m not sure how tricky this is with the implementation.

Also, one other note – the “You’re about to notify X people in the group” message shows up when you make an edit regardless of whether or not anyone will actually be notified.

Is it possible to link this warning message to the number of people who have not been notified previously about this post (in order to calculate an accurate count of the number of people who will be notified).

4 Likes

Ah ha, thanks for that! I definitely see the same behavior.

Sending mention notifications through a post edit is an important function that I know I and others use from time to time, so we want that to stay, but the way it’s working with groups feels off.

My opinion at this point would be to only notify a group once per post. Users added to a group after an initial group mention was made should not receive a group mention notification as a result of subsequent post edits. The JIT education banner about notifying the group should not display during the editing of a post if there are no new group mentions since the post was last saved.

If a group that was not previously mentioned is mentioned in an edit, they should be notified. That’s how it works with user mentions. We should also see the relevant educational banner if there is a newly mentioned group.

I’m thinking we can just add some conditional logic based on the existence of a record in the group_mentions table. We create a record at the time of saving a post.

What do you think @codinghorror?

2 Likes

I follow, but should we really consider this a bug we want to fix?

If we completely suppress all @group notifications on edit, editing a new @group will cause no mention notification which is a large change to how notifications worked in the past and very surprising.

If we start being fancy and only notifying new @groups on edit we will be stuck with complex code that has to walk through diffs and lots more bookkeeping.

I can certainly not think of any easy solution here.

4 Likes

So, imagine a post with 3 different group mentions. It’s edited and a fourth mention is added.

When saving the edit, we can use a query like this to find the current list of group mentions for that post.

-- [params]
-- text :post_id

select gm.group_id, g.name
from group_mentions gm
join groups g on (gm.group_id = g.id)
where post_id = :post_id

Then, by comparing the results of the query against the list of groups being mentioned in the post, we can find the new mention(s).

That doesn’t seem like too big of a calculation, but maybe I’m missing something obvious. This also seems like the sort of thing that could be done in a background task.

As far as the JIT banner about notifying the group(s), maybe this logic could be exposed by an API endpoint (/post/<id>/mentions.json) to get this data asynchronously.

Wouldn’t it be simpler to create a new post and notify people that way? I am a bit scared of building a giant tower of complexity here to support what is a fairly narrow edge case.

1 Like

I don’t want to make anything too complex.

I was looking through some of the code for this (thanks @tshenry for the link), and it doesn’t seem like it needs to be that complicated.

The group mentions are currently being triggered here (L78-80). The expand_group_mentions() function is a generator responsible for spitting out the users who should get group mention notifications (while respecting policies like the SiteSetting.max_users_notified_per_group_mention).

That seems like a natural place to put this logic.

In the create_notification function, we suppress notifications for anyone who has already received a notification of the same type. This is why editing a post with a group mention triggers a notification for new group members – a notification is actually generated for all users in the group, but it is then suppressed for people who already got the notification.

I’m suggesting something similar, by updating the expand_group_mentions() function to also filter out groups that have a record in the group_mentions table.

Something like (please excuse my lack of knowledge about ruby syntax)

  def expand_group_mentions(groups, post)
    return unless post.user && groups

    Group.mentionable(post.user, include_public: false).where(id: groups.map(&:id)).each do |group|
      next if group.user_count >= SiteSetting.max_users_notified_per_group_mention

      # Add check for groups already mentioned
      next if GroupMentions().where(group_id: group.id, post_id: post.id).length > 0

      yield group, group.users
    end

  end

This does not do anything for the JIT warning though.

Trying to help solve this, let me know if I’m just being annoying… :slight_smile:

2 Likes

I was about to post a new support question but found this topic.

It seems related.

If I post up a message and realise I should have notified some people that I didn’t - what is my best option?

  1. Edit the post and specifically @ mention a group or a number of people?

  2. Create a new message to @ notify them? I then have to repeat a lot of the information - or add in a link to the original post.

Option 1 would be my preferred (cleaner) method if possible?

Please can anyone confirm - would specifically @ mentioning individuals/groups on edit notify them if they haven’t been notified already? If not it sounds like that might be useful if not too hard to do?

PS if I forget I sometimes use the configurable time window to edit the post before the notifications go out. By the time you post that though you are never quite sure whether you are going to be able to do it in time. You only find out whether it appears as an edit when the edit is posted.

I’ll use this edit to test whether I receive a notification @Rhidian @codinghorror

It appears not :confused:

2 Likes

Yes this is already the case.

2 Likes

This is probably a different matter, but I often get notifications for edits to other people’s posts - I think because they are “wiki” posts that I’m watching, eg Troubleshooting email on a new Discourse install - and I find it very helpful.

3 Likes

Thank you. Has this been tested? I didn’t receive a notification for the test post I edited above - where I @ mentioned myself @Rhidian Is that because it tis set not to send notifications for your own posts?

1 Like

You can’t notify yourself in any case, of anything.

2 Likes