From 4cc3823adf0e6b926e57897730468a89c6ce513a Mon Sep 17 00:00:00 2001 From: Keith Smith Date: Thu, 27 Aug 2026 20:41:57 -0600 Subject: [PATCH] Fix DM presence indicator never updating live (#63) The sidebar shows every DM's online/offline dot at once, but the only existing signal for a presence change (member_updated) is broadcast to a room's own channel, which Presence only delivers to a connection that currently has that specific room joined -- never true for a DM sitting unopened in the sidebar. Add a dedicated per-user broadcast (dm_presence_update) sent to each of a user's DM partners on their own per-user channel whenever their global online/offline state changes, so the sidebar dot updates without needing that DM to be the open room. Co-Authored-By: Claude Sonnet 5 --- backend/app/services/message_events.py | 24 ++++++++++ backend/app/services/room_service.py | 21 +++++++++ backend/app/ws/chat.py | 3 ++ backend/tests/test_broadcast.py | 14 ++++-- backend/tests/test_direct_messages.py | 65 ++++++++++++++++++++++++++ frontend/src/pages/ChatShellPage.tsx | 8 ++++ frontend/src/types.ts | 12 +++++ 7 files changed, 142 insertions(+), 5 deletions(-) diff --git a/backend/app/services/message_events.py b/backend/app/services/message_events.py index 109a303..671cd66 100644 --- a/backend/app/services/message_events.py +++ b/backend/app/services/message_events.py @@ -8,6 +8,7 @@ from app.models import Message, MessageFile, MessageMention, Room, RoomMembershi from app.schemas.message import ReactionSummary from app.services.link_preview_service import fetch_and_broadcast_link_preview from app.services.push_service import send_push_to_user +from app.services.room_service import list_dm_partner_ids from app.services.webhook_service import dispatch_event from app.ws.broadcaster import Broadcaster from app.ws.focus_presence import FocusPresence @@ -237,6 +238,29 @@ async def broadcast_member_updated(db: AsyncSession, broadcaster: Broadcaster, u ) +async def broadcast_dm_presence_update( + db: AsyncSession, broadcaster: Broadcaster, user_id: uuid.UUID, online: bool +) -> None: + """Tells every one of user_id's DM partners that their online/offline + status just changed (#63) -- on each partner's own per-user channel, + not the DM room's channel. The room channel alone doesn't reach the + sidebar: Presence gates room-channel delivery on actually having that + specific room's channel joined right now, which is only ever the one + room currently open in the UI -- so a DM sitting unopened in the + sidebar (which is the normal case; the sidebar shows every DM's status + at once) never saw its partner's status change until something else + forced a full room-list refetch.""" + for partner_id in await list_dm_partner_ids(db, user_id): + await broadcaster.publish_to_user( + partner_id, + { + "type": "dm_presence_update", + "user_id": str(user_id), + "status": "online" if online else "offline", + }, + ) + + async def broadcast_room_added(broadcaster: Broadcaster, user_id: uuid.UUID, room: Room) -> None: """The only signal a user's open client gets that they were just added to a room -- without it, GET /rooms/mine is only ever fetched once at diff --git a/backend/app/services/room_service.py b/backend/app/services/room_service.py index 59e4367..f4efb27 100644 --- a/backend/app/services/room_service.py +++ b/backend/app/services/room_service.py @@ -244,6 +244,27 @@ async def list_member_rooms( ] +async def list_dm_partner_ids(db: AsyncSession, user_id: uuid.UUID) -> list[uuid.UUID]: + """Every user this user_id shares a DM with (#63) -- used to know who + needs telling about a global online/offline transition, since Presence + gates room-channel delivery on actually having that specific room + joined right now (only ever the one room currently open in the UI), so + a DM sitting unopened in the sidebar would otherwise never hear about + its partner's status changing at all.""" + result = await db.execute( + select(RoomMembership.user_id) + .join(Room, Room.id == RoomMembership.room_id) + .where( + Room.is_dm.is_(True), + RoomMembership.user_id != user_id, + RoomMembership.room_id.in_( + select(RoomMembership.room_id).where(RoomMembership.user_id == user_id) + ), + ) + ) + return [row[0] for row in result.all()] + + async def get_room(db: AsyncSession, room_id: uuid.UUID) -> Room: room = await db.get(Room, room_id) if room is None: diff --git a/backend/app/ws/chat.py b/backend/app/ws/chat.py index c739c06..a009222 100644 --- a/backend/app/ws/chat.py +++ b/backend/app/ws/chat.py @@ -9,6 +9,7 @@ from app.database import get_db from app.models import ApiToken, Message, MessageFile, MessageImage, RoomMembership, User from app.services.bot_service import resolve_token from app.services.message_events import ( + broadcast_dm_presence_update, broadcast_member_updated, broadcast_message_update, broadcast_new_message, @@ -96,6 +97,7 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db) # visible. if await global_presence.connect(user.id): await broadcast_member_updated(db, broadcaster, user.id) + await broadcast_dm_presence_update(db, broadcaster, user.id, online=True) # This session is shared for the connection's entire lifetime (which can # be hours) -- SQLAlchemy opens a transaction implicitly on first use, # and every read above (the auth lookup, broadcast_member_updated's own @@ -300,3 +302,4 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db) await focus_presence.mark_focused(user.id) if await global_presence.disconnect(user.id): await broadcast_member_updated(db, broadcaster, user.id) + await broadcast_dm_presence_update(db, broadcaster, user.id, online=False) diff --git a/backend/tests/test_broadcast.py b/backend/tests/test_broadcast.py index 87fdaf1..21ecf8f 100644 --- a/backend/tests/test_broadcast.py +++ b/backend/tests/test_broadcast.py @@ -8,13 +8,17 @@ def _unique(prefix: str) -> str: return f"{prefix}-{uuid.uuid4().hex[:8]}" +_NOISE_TYPES = {"member_updated", "dm_presence_update"} + + def _recv(ws) -> dict: - """Reads the next frame, transparently discarding member_updated - presence-change broadcasts -- another connection in the same room going - online/offline is real, expected noise these tests aren't about.""" + """Reads the next frame, transparently discarding presence-change + broadcasts (member_updated, and #63's dm_presence_update) -- another + connection sharing a room or a DM going online/offline is real, + expected noise these tests aren't about.""" while True: msg = ws.receive_json() - if msg.get("type") != "member_updated": + if msg.get("type") not in _NOISE_TYPES: return msg @@ -201,7 +205,7 @@ def test_new_message_notifies_recipient_who_hid_the_dm_via_websocket(ws_client_f alice_ws.send_json({"type": "join", "room_id": room["id"]}) assert alice_ws.receive_json()["type"] == "joined" - received = bob_ws.receive_json() + received = _recv(bob_ws) assert received == {"type": "room_added", "room_id": room["id"]} diff --git a/backend/tests/test_direct_messages.py b/backend/tests/test_direct_messages.py index ca08450..370f749 100644 --- a/backend/tests/test_direct_messages.py +++ b/backend/tests/test_direct_messages.py @@ -275,3 +275,68 @@ def test_new_message_unhides_dm_for_both_participants(ws_client): "/api/auth/login", json={"username_or_email": bob["username"], "password": "password123"} ) assert any(r["id"] == dm["id"] for r in ws_client.get("/api/rooms/mine").json()) + + +def test_dm_partner_presence_update_delivered_without_room_open(ws_client_factory): + # #63: alice never joins the DM's own room channel anywhere in this + # test -- exactly the normal state for a DM sitting in the sidebar that + # isn't the currently open room. member_updated's room-channel broadcast + # would never reach her in that state (Presence gates it on having that + # specific room joined); this signal has to arrive on her own per-user + # channel instead, same as room_added. + # + # Only the connect ("online") side is exercised here, not disconnect -- + # see test_presence.py's module docstring for why the offline half + # isn't reliably testable via a `with websocket_connect(...)` block + # closing (TestClient cancels the server task rather than delivering a + # real disconnect, which can interrupt a `finally` block's own awaits + # in tests only, never in production). + instance1 = ws_client_factory() + instance2 = ws_client_factory() + + alice = _register_ws(instance1, _unique("alice")) + bob = _register_ws(instance2, _unique("bob")) + instance1.post("/api/rooms/dm", json={"other_user_id": bob["id"]}) + + with instance1.websocket_connect("/ws/chat") as alice_ws: + # Sync barrier: alice's own websocket_connect() returning only + # proves the handshake completed, not that chat.py's connection + # setup (register_user, in particular -- required before bob's + # connect can reach her per-user channel at all) has finished. + # Any reply -- even an error -- proves the connection has reached + # its main frame loop, which setup always completes before. + alice_ws.send_json({"type": "__sync_barrier__"}) + assert alice_ws.receive_json()["type"] == "error" + + with instance2.websocket_connect("/ws/chat"): + online_update = alice_ws.receive_json() + assert online_update == { + "type": "dm_presence_update", + "user_id": bob["id"], + "status": "online", + } + + +def test_dm_presence_update_not_sent_to_non_partner(ws_client_factory): + instance1 = ws_client_factory() + instance2 = ws_client_factory() + instance3 = ws_client_factory() + + alice = _register_ws(instance1, _unique("alice")) + bob = _register_ws(instance2, _unique("bob")) + _register_ws(instance3, _unique("outsider")) + instance1.post("/api/rooms/dm", json={"other_user_id": bob["id"]}) + + with instance3.websocket_connect("/ws/chat") as outsider_ws: + with instance2.websocket_connect("/ws/chat"): + pass + + # Nothing should ever arrive for an outsider who shares no DM with + # bob. Prove the socket stayed quiet the same way + # test_desktop_notifications.py's non-member test does: a harmless + # self-targeted join, whose prompt "joined" ack proves nothing else + # was already queued ahead of it. + room = instance3.post("/api/rooms", json={"name": _unique("outsiders-room")}).json() + outsider_ws.send_json({"type": "join", "room_id": room["id"]}) + joined = outsider_ws.receive_json() + assert joined == {"type": "joined", "room_id": room["id"]} diff --git a/frontend/src/pages/ChatShellPage.tsx b/frontend/src/pages/ChatShellPage.tsx index ca54144..5bf4aa3 100644 --- a/frontend/src/pages/ChatShellPage.tsx +++ b/frontend/src/pages/ChatShellPage.tsx @@ -77,6 +77,14 @@ export function ChatShellPage() { : r, ), ) + } else if (envelope.type === 'dm_presence_update') { + setRooms((prev) => + prev.map((r) => + r.dm_partner && r.dm_partner.user_id === envelope.user_id + ? { ...r, dm_partner: { ...r.dm_partner, status: envelope.status } } + : r, + ), + ) } }), [socket, refreshRooms, refreshMembers, roomId], diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 48af9e8..a0be121 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -218,6 +218,17 @@ export interface ChatUnreadUpdateEnvelope { mentioned: boolean } +// #63: sent on the recipient's own per-user channel, one per DM partner, +// whenever that partner's global online/offline state changes -- lets the +// sidebar's presence dot (MyRoomItem.dm_partner.status) stay live without +// needing that DM to be the currently open room (member_updated's +// room-channel delivery doesn't reach an unopened DM at all). +export interface ChatDmPresenceUpdateEnvelope { + type: 'dm_presence_update' + user_id: string + status: 'online' | 'offline' +} + // #49: delivered over this same socket, alongside the existing Web Push // send, to every eligible offline member regardless of push-subscription // status -- see backend/app/services/message_events.py's @@ -243,6 +254,7 @@ export type ServerEnvelope = | ChatMemberUpdatedEnvelope | ChatUnreadUpdateEnvelope | ChatDesktopNotificationEnvelope + | ChatDmPresenceUpdateEnvelope export interface AdminUser { id: string