Keep an unread notice older than the latest read from marking the room

The job picks the room's members once and then publishes to them one at a
time, as the request did before it. A member who opens the room during that
loop gets the read event in their other tabs and then the older notice,
which marked the room unread again. Both events now carry the server time,
and the sidebar ignores a notice dated before the room's latest read. The
notice still moves a direct room to the top.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0142qgjggdJ2KDdGk7RF9Xm9

app/channels/presence_channel.rb

@@ -22,6 +22,6 @@ class PresenceChannel < RoomChannel
     end
 
     def broadcast_read_room
-      ActionCable.server.broadcast "user_#{current_user.id}_reads", { room_id: membership.room_id }
+      ActionCable.server.broadcast "user_#{current_user.id}_reads", { room_id: membership.room_id, at: Time.current.to_fs(:epoch) }
     end
 end

app/javascript/controllers/read_rooms_controller.js

@@ -16,7 +16,7 @@ export default class extends Controller {
     })
   }
 
-  #read = ({ room_id }) => {
-    this.dispatch("read", { detail: { roomId: room_id } })
+  #read = ({ room_id, at }) => {
+    this.dispatch("read", { detail: { roomId: room_id, at } })
   }
 }

app/javascript/controllers/rooms_list_controller.js

@@ -7,6 +7,7 @@ export default class extends Controller {
   static classes = [ "unread" ]
 
   #disconnected = true
+  #readAt = new Map()
 
   async connect() {
     this.channel ??= await cable.subscribeTo({ channel: "UnreadRoomsChannel" }, {
@@ -27,9 +28,13 @@ export default class extends Controller {
     this.read({ detail: { roomId: Current.room.id } })
   }
 
-  read({ detail: { roomId } }) {
+  read({ detail: { roomId, at } }) {
     const room = this.#findRoomTarget(roomId)
 
+    if (at) {
+      this.#readAt.set(Number(roomId), Math.max(Number(at), this.#readAt.get(Number(roomId)) ?? 0))
+    }
+
     if (room) {
       room.classList.remove(this.unreadClass)
       this.dispatch("read", { detail: { targetId: roomId } })
@@ -47,11 +52,11 @@ export default class extends Controller {
     this.#disconnected = true
   }
 
-  #unread({ roomId }) {
+  #unread({ roomId, at }) {
     const unreadRoom = this.#findRoomTarget(roomId)
 
     if (unreadRoom) {
-      if (Current.room.id != roomId) {
+      if (Current.room.id != roomId && !this.#readSince(roomId, at)) {
         unreadRoom.classList.add(this.unreadClass)
       }
 
@@ -59,6 +64,12 @@ export default class extends Controller {
     }
   }
 
+  // Notices fan out one member at a time, so one can arrive after the member has already
+  // read the room in another tab. It still reorders the room, but doesn't mark it unread.
+  #readSince(roomId, at) {
+    return Number(at) <= this.#readAt.get(Number(roomId))
+  }
+
   #findRoomTarget(roomId) {
     return this.roomTargets.find(roomTarget => roomTarget.dataset.roomId == roomId)
   }

app/models/message/broadcasts.rb

@@ -13,7 +13,7 @@ module Message::Broadcasts
   # 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)
+    payload = ActiveSupport::JSON.encode(roomId: room.id, at: created_at.to_fs(:epoch))
 
     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

test/channels/presence_channel_test.rb

@@ -40,6 +40,16 @@ class PresenceChannelTest < ActionCable::Channel::TestCase
     end
   end
 
+  test "subscribing tells the user's other tabs when they read the room" do
+    membership = users(:david).memberships.first
+
+    freeze_time do
+      assert_broadcast_on "user_#{users(:david).id}_reads", { room_id: membership.room_id, at: Time.current.to_fs(:epoch) } do
+        subscribe room_id: membership.room_id
+      end
+    end
+  end
+
   test "unsubscribing marks the membership as disconnected" do
     membership = users(:david).memberships.first
     subscribe room_id: membership.room_id

test/channels/unread_rooms_channel_test.rb

@@ -26,14 +26,16 @@ class UnreadRoomsChannelTest < ActionCable::Channel::TestCase
 
   test "a member is told about activity in their own room" do
     direct = rooms(:bender_and_kevin)
+    message = nil
 
     broadcasts = capture_unread_broadcasts_for(users(:kevin)) do
       perform_enqueued_jobs only: Message::BroadcastUnreadRoomJob do
-        direct.messages.create!(body: "Private", creator: users(:bender), client_message_id: "member").broadcast_create
+        message = direct.messages.create!(body: "Private", creator: users(:bender), client_message_id: "member")
+        message.broadcast_create
       end
     end
 
-    assert_equal [ direct.id ], broadcasts.collect { |broadcast| broadcast["roomId"] }
+    assert_equal [ { "roomId" => direct.id, "at" => message.created_at.to_fs(:epoch) } ], broadcasts
   end
 
   private

test/system/unread_rooms_test.rb

@@ -27,4 +27,36 @@ class UnreadRoomsTest < ApplicationSystemTestCase
     join_room designers_room
     assert_room_read designers_room
   end
+
+  test "a notice about a message from before the room was read in another tab doesn't mark it unread again" do
+    using_session("Kevin in HQ") do
+      sign_in "kevin@37signals.com"
+      join_room rooms(:hq)
+      broadcast_unread_notice rooms(:designers), to: users(:kevin), at: Time.current
+      assert_room_unread rooms(:designers)
+    end
+
+    sent_before_reading = Time.current
+    using_session("Kevin in Designers") do
+      sign_in "kevin@37signals.com"
+      join_room rooms(:designers)
+    end
+
+    using_session("Kevin in HQ") do
+      assert_room_read rooms(:designers)
+
+      broadcast_unread_notice rooms(:designers), to: users(:kevin), at: sent_before_reading
+      broadcast_unread_notice rooms(:bender_and_kevin), to: users(:kevin), at: Time.current
+      assert_selector "#" + dom_id(rooms(:bender_and_kevin), :list) + ".unread"
+      assert_room_read rooms(:designers)
+
+      broadcast_unread_notice rooms(:designers), to: users(:kevin), at: Time.current
+      assert_room_unread rooms(:designers)
+    end
+  end
+
+  private
+    def broadcast_unread_notice(room, to:, at:)
+      ActionCable.server.broadcast UnreadRoomsChannel.stream_name_for(to.id), { roomId: room.id, at: at.to_fs(:epoch) }
+    end
 end