"X new / updated topics" gets triggered for muted topics

Bug
1

I had been getting the “X new / updated topics” notification when there hadn’t actually been a new topic occasionally, but as it turns out there were new topics – they were just muted. They didn’t appear in Latest because they were muted, but they still set off the notification. This probably shouldn’t happen.

8 Likes
2

I just found it it also happens for unlisted threads too – not just muted ones.

3

Is this happening only while you are logged in or are you also getting Notifications for posts that were created while you were logged out?

4

It’s only happened while I’ve been logged in so far, but that’s mostly because I’m never logged out – I always have a window for the forum up on my secondary monitor.

5

Is there a way to improve on this for 1.8 that is not risky @eviltrout?

6

I’ll take a look and see

8

This is nasty hard, cause the message bus message we ship goes to “everyone” it does not filter out users that have the topic muted.

To fix this we would need

  • Either have the client be aware of topic ids for muted topics, which could be tens of thousands of ids (so this is a non starter)

  • Or, Have the client issue an extra AJAX call to double check if any of the topics in the batch of refreshed topics is muted. Which also is nasty.

I do not think anything easy can be done here.

2 Likes
9

What about the server doing the filtering?

10

How can it do that?

  • Post is created

  • We trigger a message bus message to all groups with access to the topic informing that a new post is created

We would have hint that message to filter out all users that have the topic muted, that sort that out.

So, now what we have is:

 def self.publish_latest(topic, staff_only=false)
    return unless topic.archetype == "regular"

    message = {
      topic_id: topic.id,
      message_type: "latest",
      payload: {
        bumped_at: topic.bumped_at,
        topic_id: topic.id,
        category_id: topic.category_id,
        archetype: topic.archetype
      }
    }

    group_ids =
      if staff_only
        [Group::AUTO_GROUPS[:staff]]
      else
        topic.category && topic.category.secure_group_ids
      end
    MessageBus.publish("/latest", message.as_json, group_ids: group_ids)
  end

What we kind of want is:

MessageBus.publish("/latest", message.as_json, group_ids: group_ids, server_filter: lambda do |user_id|
   return topic.not_muted?(user_id)
end
)

It has caveats cause it means that when people ask for the message we would have to issue a query, nothing in message bus works this way at the moment.

The trivial implementation does not work, cause the user_id list can be gigantic

MessageBus.publish("/latest", message.as_json, group_ids: group_ids, exclude_user_ids: user_ids_muting_topic)

This one may sort of work,

MessageBus.publish("/latest", message.as_json, group_ids: group_ids, exclude_user_ids: user_ids_muting_topic_seen_in_last_24_hours)

It is a :volcano: of worms.

2 Likes
11

https://github.com/discourse/discourse/blob/master/app/models/topic_tracking_state.rb#L93-L94

Here we already having user-wise filtered MessageBus. Can we do something like above here?

Currently all users are subscribed to /latest MessageBus. Additionally all logged in users should be subscribed to /latest/muted/:USERID MessageBus. Then in server we should publish latest action on both MessageBuses like below

TopicUser.each do # Should select only muted topic users
  MessageBus.publish("/latest/muted/#{tu.user_id}", message.as_json, group_ids: group_ids)
end
MessageBus.publish("/latest", message.as_json, group_ids: group_ids)

In client side if a topic is received on both MessageBuses then we shouldn’t display it on updated topics.

Ignore me if I am wrong. I don’t have deep knowledge on how MessageBuses working.

1 Like
12

The issue I have with that is that a single post creation could create thousands of messages if thousands of people are muting a topic.

13

Yes. I noticed that issue. We are having the same problem on /unread MessageBus. If thousands of people tracking a topic then a single post creation will send thousands of messages in /unread too. Somehow we have to fix both.

Is there any chance to send messages only to the people who online (like last_seen_at > 1.hour.ago) ?

2 Likes
14

Our users are picking up on this issue too.

An uneducated suggestion - If update messages contain topic ids, could these be filtered on client-side based on a user’s mute list? Caching the mute-list client-side would probably be okay as I expect most users probably only mute a small handful of topics (and in any case, we’d use a bounded cache of, say, 1000 most-recently-muted topic ids). No changes to message bus infrastructure required - it would be downstream of that.

2 Likes
15

Yeah that is doable, does increase initial payload, we could defer loading the muted list till we get the first message something that helps

8 Likes
16

Some of our users have been raising this bug with us too lately.

It’s been around for quite some time by the looks of it, any more thoughts on a fix for this one?

17

We should eventually fix this @sam, I feel a final safety check here (wait, before I show this banner to this user, are all the topics I am notifying the user about muted by this user?) could make sense.

3 Likes
18

If we ship to the client the list of muted categories / tags we may be able to filter these out on the client with radical and complex changes to the server.

19

If it’s super hard it can wait – I was thinking more of a “saving throw” that the client does when it gets that message, if the user has muted stuff.

Ironically when the user clicks on the “new replies / topics” notice … this happens, yes? At the time the user clicks, if no new topics appear then you know by definition it was all muted stuff.

It would be a lot of extra work though, you’d basically have to pre-emptively do all the actual work of the user clicking the notice, then compare the results, then choose not to show the banner because the topic list is unchanged.

So yes, I can see why this would be quite painful.

1 Like
20

It think it is doable there are just a few changes that we need in place

  • We need to ensure that muted tags / muted categories are included in the “current user serializer”, this adds 2 queries unconditionally to every initial page load

  • We need to amend the message bus message to include tags cause now it only has categories

  • We need to wire in the business logic of where to apply this filter (clearly you want the bar to show up if you are parked on a muted category vs latest)

1 Like
21

That’s not enough though because users can mute individual topics.

I think I am fine deferring on this quite a bit longer.

2 Likes