diff --git a/app/models/message/broadcasts.rb b/app/models/message/broadcasts.rb index b9660c9..6ae7569 100644 --- a/app/models/message/broadcasts.rb +++ b/app/models/message/broadcasts.rb @@ -9,11 +9,13 @@ module Message::Broadcasts end # Fanned out to the room's members rather than published on one global stream, so - # that the timing of activity in a room only reaches people who are in it. + # that the timing of activity in a room only reaches people who are in it. Of those, + # only the ones who still have it unread or are in it now: by the time this runs, + # someone may have opened the room and moved on, and would see it marked unread again. def broadcast_unread_room payload = ActiveSupport::JSON.encode(roomId: room.id) - room.memberships.pluck(:user_id).each do |user_id| + room.memberships.unread.or(room.memberships.connected).pluck(:user_id).each do |user_id| ActionCable.server.broadcast UnreadRoomsChannel.stream_name_for(user_id), payload, coder: nil end end diff --git a/test/controllers/messages_controller_test.rb b/test/controllers/messages_controller_test.rb index 63266cb..83cedda 100644 --- a/test/controllers/messages_controller_test.rb +++ b/test/controllers/messages_controller_test.rb @@ -58,6 +58,8 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest end test "creating a message broadcasts unread room to each member" do + memberships(:david_watercooler).present # the poster is in the room + @room.users.each do |member| assert_broadcasts UnreadRoomsChannel.stream_name_for(member.id), 1 do perform_enqueued_jobs only: Message::BroadcastUnreadRoomJob do @@ -77,6 +79,20 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest assert_enqueued_with job: Message::BroadcastUnreadRoomJob, args: [ Message.last ] end + test "the unread fanout skips a member who has opened the room since" do + membership = memberships(:jason_watercooler) + + post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one", client_message_id: 999 } } + assert membership.reload.unread? + + membership.present + membership.reload.disconnected + + assert_no_broadcasts UnreadRoomsChannel.stream_name_for(membership.user_id) do + perform_enqueued_jobs only: Message::BroadcastUnreadRoomJob + end + end + test "creating a message doesn't broadcast unread room to non-members" do outsiders = User.where.not(id: @room.users.map(&:id)) assert outsiders.any?, "need someone outside the room for this test to mean anything"