Private
Public Access
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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"]}
|
||||
|
||||
|
||||
|
||||
@@ -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"]}
|
||||
|
||||
Reference in New Issue
Block a user