diff --git a/backend/README.md b/backend/README.md index 1e0d1ab..3e14322 100644 --- a/backend/README.md +++ b/backend/README.md @@ -311,6 +311,40 @@ that point, so nothing online-facing is delayed, and it sidesteps on. An expired/invalid subscription (pywebpush 404/410) is deleted automatically. +## Desktop notifications + +DS Chat Desktop (a separate Electron wrapper, not this repo) has no push +delivery service configured, so it can't receive Web Push. Instead, +`app/services/message_events.py`'s existing offline-member computation +(`room members - Presence.connected_user_ids(room_id) - {sender}` — the +same audience Web Push uses, described above) also broadcasts a +`desktop_notification` WS envelope (`{type, id, room_id, title, body}`, +`id` being the message's own id so the client can dedupe across +reconnects) to every eligible offline member over their already-open +authenticated socket, unconditionally — the server has no notion of which +clients are running inside Electron. It's sent alongside the Web Push +send, not instead of it, so a member with only a browser tab open is +unaffected. + +The client decides whether to act on it: `frontend/src/lib/desktopBridge.ts` +feature-detects `window.dsDesktop` (the bridge Electron's preload script +exposes, per-method rather than via user-agent sniffing — an older wrapper +build may be missing individual methods) and only calls +`showNotification` when the bridge is present and the user's +localStorage-backed preference (`ds-chat-desktop-notifications-enabled`, +default on) allows it. This preference is deliberately a plain client-side +flag rather than reusing `PushSubscription` — desktop notifications need +no server round trip to enable/disable, unlike a push subscription which +has a row to create/delete. `frontend/src/components/DesktopNotificationBridge.tsx` +is mounted once, as a sibling of the routed pages inside the `user.id`-keyed +`ChatSocketProvider`, so it subscribes exactly once per authenticated +session; it also wires `window.dsDesktop.onNotificationClick` to navigate +to the notification's room. + +No `User`/`PushSubscription` schema change was needed for this feature — +the only backend change is the new `desktop_notification` envelope type, +covered by `backend/tests/test_desktop_notifications.py`. + ## Room roles and membership (Phase 2) Rooms can be `open` (anyone can join via `POST /api/rooms/{id}/join`) or diff --git a/backend/app/services/message_events.py b/backend/app/services/message_events.py index 18d4b36..bb27287 100644 --- a/backend/app/services/message_events.py +++ b/backend/app/services/message_events.py @@ -55,6 +55,7 @@ async def _notify_offline_members( ) room = await db.get(Room, room_id) + title = f"#{room.name}" if room else "New message" for user_id in offline_ids: mentioned = user_id in mentioned_ids if message.content: @@ -64,12 +65,28 @@ async def _notify_offline_members( body = f"{sender.username} sent a file" else: body = f"{sender.username} sent an image" - payload = { - "title": f"#{room.name}" if room else "New message", - "body": body, - "room_id": str(room_id), - } + payload = {"title": title, "body": body, "room_id": str(room_id)} await send_push_to_user(db, user_id, payload) + # Desktop notifications (#49): delivered over this same already-open + # authenticated socket rather than Web Push, since Electron has no + # push delivery service configured. Broadcast to every eligible + # offline member regardless of push-subscription status -- the + # client decides whether to act on it (only when window.dsDesktop + # is present), so the server doesn't need to track which clients + # are running inside Electron. `id` is the message's own id + # (stable, not random) so the client can dedupe across socket + # reconnects/replays, the same way Electron's own eventId dedup + # does on its side. + await broadcaster.publish_to_user( + user_id, + { + "type": "desktop_notification", + "id": str(message.id), + "room_id": str(room_id), + "title": title, + "body": body, + }, + ) async def _message_payload(db: AsyncSession, message: Message, username: str) -> dict: diff --git a/backend/tests/test_desktop_notifications.py b/backend/tests/test_desktop_notifications.py new file mode 100644 index 0000000..82381fc --- /dev/null +++ b/backend/tests/test_desktop_notifications.py @@ -0,0 +1,156 @@ +import uuid + + +def _unique(prefix: str) -> str: + return f"{prefix}-{uuid.uuid4().hex[:8]}" + + +def _recv(ws) -> dict: + """Reads the next frame, discarding member_updated presence-change + broadcasts -- same convention as test_mentions.py/test_push.py.""" + while True: + msg = ws.receive_json() + if msg.get("type") != "member_updated": + return msg + + +def _register_ws(ws_client, username: str) -> dict: + from app.schemas.user import UserCreate + from app.services.auth_service import register_user + + async def _seed(): + async with ws_client.session_factory() as session: + await register_user( + session, + UserCreate(username=username, email=f"{username}@example.com", password="password123"), + ) + + ws_client.portal.call(_seed) + resp = ws_client.post( + "/api/auth/login", json={"username_or_email": username, "password": "password123"} + ) + assert resp.status_code == 200, resp.text + return resp.json() + + +def _send_and_sync(ws, room_id: str, content: str) -> dict: + """Sync barrier -- see test_mentions.py's identical helper. Proves the + message frame's full handling (including the offline-notify step this + feature hooks into) has completed before the test checks anything.""" + ws.send_json({"type": "message", "room_id": room_id, "content": content}) + message = ws.receive_json() + ws.send_json({"type": "join", "room_id": room_id}) + assert ws.receive_json()["type"] == "joined" + return message + + +def test_desktop_notification_delivered_to_offline_member(ws_client_factory): + # #49: bob has an open connection (so he can receive his per-user + # channel broadcast) but hasn't joined *this* room's channel -- exactly + # the "app running, room not foregrounded" case _notify_offline_members + # already treats as offline for push, and desktop notifications should + # use the identical audience. + instance1 = ws_client_factory() + instance2 = ws_client_factory() + + alice = _register_ws(instance1, _unique("alice")) + room = instance1.post("/api/rooms", json={"name": _unique("general")}).json() + + _register_ws(instance2, _unique("bob")) + instance2.post(f"/api/rooms/{room['id']}/join") + + with instance2.websocket_connect("/ws/chat") as bob_ws: + with instance1.websocket_connect("/ws/chat") as alice_ws: + alice_ws.send_json({"type": "join", "room_id": room["id"]}) + assert alice_ws.receive_json()["type"] == "joined" + message = _send_and_sync(alice_ws, room["id"], "hey, look at this") + assert message["type"] == "message" + + # bob never joined the room's channel on his connection, so he's + # "offline" for it even though instance2 is connected. + bob_update = _recv(bob_ws) + assert bob_update["type"] == "unread_update" + + desktop_note = _recv(bob_ws) + assert desktop_note == { + "type": "desktop_notification", + "id": message["id"], + "room_id": room["id"], + "title": f"#{room['name']}", + "body": f"{alice['username']}: hey, look at this", + } + + +def test_desktop_notification_uses_mention_wording(ws_client_factory): + instance1 = ws_client_factory() + instance2 = ws_client_factory() + + alice = _register_ws(instance1, _unique("alice")) + room = instance1.post("/api/rooms", json={"name": _unique("general")}).json() + + bob = _register_ws(instance2, _unique("bob")) + instance2.post(f"/api/rooms/{room['id']}/join") + + with instance2.websocket_connect("/ws/chat") as bob_ws: + with instance1.websocket_connect("/ws/chat") as alice_ws: + alice_ws.send_json({"type": "join", "room_id": room["id"]}) + assert alice_ws.receive_json()["type"] == "joined" + _send_and_sync(alice_ws, room["id"], f"@{bob['username']} check this out") + + _recv(bob_ws) # unread_update + desktop_note = _recv(bob_ws) + assert desktop_note["type"] == "desktop_notification" + assert desktop_note["body"] == f"{alice['username']} mentioned you: @{bob['username']} check this out" + + +def test_desktop_notification_not_sent_to_room_member_who_is_present(ws_client_factory): + instance1 = ws_client_factory() + instance2 = ws_client_factory() + + alice = _register_ws(instance1, _unique("alice")) + room = instance1.post("/api/rooms", json={"name": _unique("general")}).json() + + _register_ws(instance2, _unique("bob")) + instance2.post(f"/api/rooms/{room['id']}/join") + + with instance2.websocket_connect("/ws/chat") as bob_ws: + bob_ws.send_json({"type": "join", "room_id": room["id"]}) + assert bob_ws.receive_json()["type"] == "joined" + + with instance1.websocket_connect("/ws/chat") as alice_ws: + alice_ws.send_json({"type": "join", "room_id": room["id"]}) + assert alice_ws.receive_json()["type"] == "joined" + _send_and_sync(alice_ws, room["id"], "hello") + + # bob is actively in the room -- he should only see the live + # "message" broadcast, never an unread_update or desktop_notification. + live_message = _recv(bob_ws) + assert live_message["type"] == "message" + + +def test_desktop_notification_not_sent_to_non_member(ws_client_factory): + instance1 = ws_client_factory() + instance2 = ws_client_factory() + + _register_ws(instance1, _unique("alice")) + room = instance1.post("/api/rooms", json={"name": _unique("general")}).json() + + _register_ws(instance2, _unique("outsider")) + # Deliberately not joining `room`. + + with instance2.websocket_connect("/ws/chat") as outsider_ws: + with instance1.websocket_connect("/ws/chat") as alice_ws: + alice_ws.send_json({"type": "join", "room_id": room["id"]}) + assert alice_ws.receive_json()["type"] == "joined" + _send_and_sync(alice_ws, room["id"], "hello") + + # Nothing should ever arrive on the outsider's own channel for a + # room they aren't a member of. Send a harmless self-targeted + # frame on a *different* room-less action and confirm the socket + # stays quiet: simplest proof is a short, bounded wait via a + # room creation (which touches no broadcast) -- if anything queued + # up for outsider, it would already be sitting in the socket buffer. + room2 = instance2.post("/api/rooms", json={"name": _unique("outsiders-room")}).json() + outsider_ws.send_json({"type": "join", "room_id": room2["id"]}) + joined = outsider_ws.receive_json() + assert joined == {"type": "joined", "room_id": room2["id"]} diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 83fd2c1..9b88a55 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -2,6 +2,7 @@ import { Navigate, Route, Routes } from 'react-router-dom' import { AuthProvider, useAuth } from './context/AuthContext' import { ChatSocketProvider } from './context/ChatSocketContext' import { AdminRoute } from './components/AdminRoute' +import { DesktopNotificationBridge } from './components/DesktopNotificationBridge' import { ProtectedRoute } from './components/ProtectedRoute' import { UpdateBanner } from './components/UpdateBanner' import { LoginPage } from './pages/LoginPage' @@ -55,7 +56,12 @@ function AppRoutes() { // is currently open. const { user } = useAuth() if (!user) return routes - return {routes} + return ( + + + {routes} + + ) } function App() { diff --git a/frontend/src/components/DesktopNotificationBridge.tsx b/frontend/src/components/DesktopNotificationBridge.tsx new file mode 100644 index 0000000..108ca7d --- /dev/null +++ b/frontend/src/components/DesktopNotificationBridge.tsx @@ -0,0 +1,50 @@ +import { useEffect } from 'react' +import { useNavigate } from 'react-router-dom' +import { useChatSocketContext } from '../context/ChatSocketContext' +import { + getDesktopNotificationsEnabled, + isDesktopNotificationsSupported, + onDesktopNotificationClick, + showDesktopNotification, +} from '../lib/desktopBridge' +import type { ServerEnvelope } from '../types' + +// #49: renders nothing -- purely wires the authenticated socket's +// "desktop_notification" envelopes (see backend/app/services/ +// message_events.py's _notify_offline_members) into DS Chat Desktop's +// native notification bridge, when running inside it. A no-op everywhere +// else (isDesktopNotificationsSupported() is false in every real browser). +// +// Mounted once, as a sibling of the routed pages inside ChatSocketProvider +// (App.tsx) -- that provider is already keyed by user.id and untouched by +// route changes, so this subscribes exactly once per authenticated session +// rather than accumulating a listener per navigation. +export function DesktopNotificationBridge() { + const socket = useChatSocketContext() + const navigate = useNavigate() + + useEffect(() => { + return socket.subscribe((envelope: ServerEnvelope) => { + if (envelope.type !== 'desktop_notification') return + if (!isDesktopNotificationsSupported() || !getDesktopNotificationsEnabled()) return + showDesktopNotification({ + eventId: envelope.id, + roomId: envelope.room_id, + title: envelope.title, + body: envelope.body, + }) + }) + }, [socket]) + + useEffect(() => { + const unsubscribe = onDesktopNotificationClick((roomId) => { + // Always an internal room id from our own server, never a URL -- + // constructing the route here (not accepting a URL from the bridge) + // is the point, not an implementation detail. + navigate(`/rooms/${roomId}`) + }) + return unsubscribe + }, [navigate]) + + return null +} diff --git a/frontend/src/components/TopBar.tsx b/frontend/src/components/TopBar.tsx index 0195083..348e120 100644 --- a/frontend/src/components/TopBar.tsx +++ b/frontend/src/components/TopBar.tsx @@ -6,11 +6,22 @@ import { ApiError } from '../api/client' import { getUserAvatarUrl } from '../api/users' import { useAuth } from '../context/AuthContext' import { hashIndex } from '../lib/avatar' +import { + getDesktopNotificationsEnabled, + isDesktopNotificationsSupported, + setDesktopNotificationsEnabled, +} from '../lib/desktopBridge' import { getPushSubscriptionStatus, isPushSupported, subscribeToPush, unsubscribeFromPush } from '../lib/push' import { ProfileModal } from './ProfileModal' import { UserAvatar } from './UserAvatar' import './TopBar.css' +// #49: inside DS Chat Desktop, notifications are delivered over the socket +// bridge instead of Web Push (Electron has no push delivery service +// configured) -- checked once, not re-derived per render, since bridge +// presence can't change over a session's lifetime. +const desktopMode = isDesktopNotificationsSupported() + export function TopBar() { const { user, updateUser, logout } = useAuth() const navigate = useNavigate() @@ -19,13 +30,26 @@ export function TopBar() { const [pushSubscribed, setPushSubscribed] = useState(false) const [pushBusy, setPushBusy] = useState(false) const [pushError, setPushError] = useState(null) + const [desktopNotificationsEnabled, setDesktopNotificationsEnabledState] = useState( + getDesktopNotificationsEnabled, + ) const [presenceBusy, setPresenceBusy] = useState(false) const [presenceError, setPresenceError] = useState(null) useEffect(() => { + // Never touch PushManager at all in desktop mode -- Electron has no + // push service configured, so even the read-only getSubscription() + // check has no reason to run there. + if (desktopMode) return getPushSubscriptionStatus().then(setPushSubscribed) }, []) + function handleToggleDesktopNotifications() { + const next = !desktopNotificationsEnabled + setDesktopNotificationsEnabled(next) + setDesktopNotificationsEnabledState(next) + } + async function handleTogglePush() { setPushBusy(true) setPushError(null) @@ -119,15 +143,21 @@ export function TopBar() { Admin )} - {isPushSupported() && ( - + ) : ( + isPushSupported() && ( + + ) )} {pushError &&
{pushError}
}