Private
Public Access
Also email room subscribers on the first unread message, not just mentions (#67)
The previous pass only emailed on a mention. Corrected scope: the room's first unread message triggers one debounced email (same shape as #66), and every mention additionally emails regardless of that debounce, since a mention shouldn't get silently absorbed by an earlier plain message's already-sent notification. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -42,10 +42,11 @@ class RoomMembership(Base):
|
|||||||
# arrives in the room, or when find_or_create_dm resolves back to it --
|
# arrives in the room, or when find_or_create_dm resolves back to it --
|
||||||
# both count as the conversation being active again.
|
# both count as the conversation being active again.
|
||||||
hidden_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True))
|
hidden_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True))
|
||||||
# #67: opt-in, per-member -- email when a message in this room mentions
|
# #67: opt-in, per-member -- email while offline on the room's first
|
||||||
# them while they're offline. Deliberately separate from #66's DM
|
# unread message, plus every mention regardless of that debounce (see
|
||||||
# emails (always-on, no toggle) rather than a shared flag, since DMs
|
# message_events.py's _maybe_email_room_notifications). Deliberately
|
||||||
# are explicitly out of scope for this setting (see message_events.py).
|
# separate from #66's DM emails (always-on, no toggle) rather than a
|
||||||
|
# shared flag, since DMs are explicitly out of scope for this setting.
|
||||||
email_notifications: Mapped[bool] = mapped_column(
|
email_notifications: Mapped[bool] = mapped_column(
|
||||||
Boolean, default=False, server_default="false", nullable=False
|
Boolean, default=False, server_default="false", nullable=False
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -63,9 +63,11 @@ class MyRoomItem(RoomRead):
|
|||||||
# avatar, not this room's internal `name`) without a second fetch per
|
# avatar, not this room's internal `name`) without a second fetch per
|
||||||
# row. None for a regular room.
|
# row. None for a regular room.
|
||||||
dm_partner: DmPartnerInfo | None = None
|
dm_partner: DmPartnerInfo | None = None
|
||||||
# #67: this viewer's own opt-in for "email me when mentioned here while
|
# #67: this viewer's own opt-in for email while offline -- the room's
|
||||||
# offline" -- always false for a DM (see message_events.py, deliberately
|
# first unread message plus every mention (see message_events.py's
|
||||||
# out of scope; DMs already get #66's automatic offline email).
|
# _maybe_email_room_notifications for the exact debounce shape). Always
|
||||||
|
# false for a DM, deliberately out of scope -- DMs already get #66's
|
||||||
|
# automatic offline email.
|
||||||
email_notifications: bool = False
|
email_notifications: bool = False
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -249,7 +249,7 @@ async def _maybe_email_dm_notification(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
async def _maybe_email_room_mention_notifications(
|
async def _maybe_email_room_notifications(
|
||||||
db: AsyncSession,
|
db: AsyncSession,
|
||||||
global_presence: GlobalPresence,
|
global_presence: GlobalPresence,
|
||||||
base_url: str,
|
base_url: str,
|
||||||
@@ -257,11 +257,16 @@ async def _maybe_email_room_mention_notifications(
|
|||||||
sender: User,
|
sender: User,
|
||||||
message: Message,
|
message: Message,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""#67: opt-in, per-room email on mention -- deliberately scoped to
|
"""#67: opt-in, per-room email -- deliberately scoped to regular rooms
|
||||||
regular rooms only (see set_room_email_notifications: DMs already get
|
only (see set_room_email_notifications: DMs already get #66's
|
||||||
#66's always-on offline email, no separate toggle). Debounced the same
|
always-on offline email, no separate toggle).
|
||||||
shape as #66 -- only the first unread mention in this room emails, not
|
|
||||||
one per mention while the recipient is away.
|
Two triggers, not one: the room's first unread message debounces the
|
||||||
|
same way #66 does (one email per unread burst, not one per message),
|
||||||
|
but a mention always emails regardless of that debounce -- a mention
|
||||||
|
is a stronger, individually-addressed signal that shouldn't get
|
||||||
|
silently swallowed just because an earlier plain message in the same
|
||||||
|
burst already used up the "first unread" email.
|
||||||
"""
|
"""
|
||||||
room = await db.get(Room, room_id)
|
room = await db.get(Room, room_id)
|
||||||
if room is None or room.is_dm:
|
if room is None or room.is_dm:
|
||||||
@@ -270,20 +275,21 @@ async def _maybe_email_room_mention_notifications(
|
|||||||
result = await db.execute(
|
result = await db.execute(
|
||||||
select(MessageMention.user_id).where(MessageMention.message_id == message.id)
|
select(MessageMention.user_id).where(MessageMention.message_id == message.id)
|
||||||
)
|
)
|
||||||
mentioned_ids = {row[0] for row in result.all()} - {sender.id}
|
mentioned_ids = {row[0] for row in result.all()}
|
||||||
if not mentioned_ids:
|
|
||||||
|
result = await db.execute(
|
||||||
|
select(RoomMembership).where(
|
||||||
|
RoomMembership.room_id == room_id,
|
||||||
|
RoomMembership.user_id != sender.id,
|
||||||
|
RoomMembership.email_notifications.is_(True),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
subscribed_memberships = result.scalars().all()
|
||||||
|
if not subscribed_memberships:
|
||||||
return
|
return
|
||||||
|
|
||||||
for user_id in mentioned_ids:
|
for membership in subscribed_memberships:
|
||||||
membership_result = await db.execute(
|
user_id = membership.user_id
|
||||||
select(RoomMembership).where(
|
|
||||||
RoomMembership.room_id == room_id, RoomMembership.user_id == user_id
|
|
||||||
)
|
|
||||||
)
|
|
||||||
membership = membership_result.scalar_one_or_none()
|
|
||||||
if membership is None or not membership.email_notifications:
|
|
||||||
continue
|
|
||||||
|
|
||||||
recipient = await db.get(User, user_id)
|
recipient = await db.get(User, user_id)
|
||||||
if recipient is None:
|
if recipient is None:
|
||||||
continue
|
continue
|
||||||
@@ -291,30 +297,41 @@ async def _maybe_email_room_mention_notifications(
|
|||||||
if not recipient.appear_offline and await global_presence.is_online(recipient.id):
|
if not recipient.appear_offline and await global_presence.is_online(recipient.id):
|
||||||
continue
|
continue
|
||||||
|
|
||||||
already_unread_mention = await db.execute(
|
mentioned = user_id in mentioned_ids
|
||||||
select(Message.id)
|
if not mentioned:
|
||||||
.join(MessageMention, MessageMention.message_id == Message.id)
|
already_unread = await db.execute(
|
||||||
.where(
|
select(Message.id)
|
||||||
MessageMention.user_id == user_id,
|
.where(
|
||||||
Message.room_id == room_id,
|
Message.room_id == room_id,
|
||||||
Message.id != message.id,
|
Message.id != message.id,
|
||||||
Message.created_at > membership.last_read_at,
|
Message.created_at > membership.last_read_at,
|
||||||
|
)
|
||||||
|
.limit(1)
|
||||||
)
|
)
|
||||||
.limit(1)
|
if already_unread.scalar_one_or_none() is not None:
|
||||||
)
|
continue
|
||||||
if already_unread_mention.scalar_one_or_none() is not None:
|
|
||||||
continue
|
if mentioned:
|
||||||
|
subject = f"New mention in #{room.name}"
|
||||||
|
body_line = (
|
||||||
|
f"{sender.username} mentioned you: {message.content[:200]}"
|
||||||
|
if message.content
|
||||||
|
else f"{sender.username} mentioned you"
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
subject = f"New message in #{room.name}"
|
||||||
|
if message.content:
|
||||||
|
body_line = f"{sender.username}: {message.content[:200]}"
|
||||||
|
elif message.file_id:
|
||||||
|
body_line = f"{sender.username} sent a file"
|
||||||
|
else:
|
||||||
|
body_line = f"{sender.username} sent an image"
|
||||||
|
|
||||||
body_line = (
|
|
||||||
f"{sender.username} mentioned you: {message.content[:200]}"
|
|
||||||
if message.content
|
|
||||||
else f"{sender.username} mentioned you"
|
|
||||||
)
|
|
||||||
link = f"{base_url.rstrip('/')}/rooms/{room_id}"
|
link = f"{base_url.rstrip('/')}/rooms/{room_id}"
|
||||||
await send_email(
|
await send_email(
|
||||||
db,
|
db,
|
||||||
recipient.email,
|
recipient.email,
|
||||||
f"New mention in #{room.name}",
|
subject,
|
||||||
[body_line],
|
[body_line],
|
||||||
cta_label="Open room",
|
cta_label="Open room",
|
||||||
cta_url=link,
|
cta_url=link,
|
||||||
@@ -361,7 +378,7 @@ async def broadcast_new_message(
|
|||||||
)
|
)
|
||||||
await _notify_offline_members(db, broadcaster, presence, focus_presence, room_id, sender, message)
|
await _notify_offline_members(db, broadcaster, presence, focus_presence, room_id, sender, message)
|
||||||
await _maybe_email_dm_notification(db, global_presence, base_url, room_id, sender, message)
|
await _maybe_email_dm_notification(db, global_presence, base_url, room_id, sender, message)
|
||||||
await _maybe_email_room_mention_notifications(db, global_presence, base_url, room_id, sender, message)
|
await _maybe_email_room_notifications(db, global_presence, base_url, room_id, sender, message)
|
||||||
await dispatch_event(db, "message.created", room_id, payload)
|
await dispatch_event(db, "message.created", room_id, payload)
|
||||||
_maybe_fetch_link_preview(broadcaster, room_id, message)
|
_maybe_fetch_link_preview(broadcaster, room_id, message)
|
||||||
|
|
||||||
|
|||||||
@@ -73,7 +73,7 @@ def _subscribe(ws_client, room_id: str, username: str, password: str = "password
|
|||||||
assert resp.status_code == 204, resp.text
|
assert resp.status_code == 204, resp.text
|
||||||
|
|
||||||
|
|
||||||
def test_room_mention_emails_offline_subscribed_member(ws_client_factory, monkeypatch):
|
def test_room_first_message_emails_offline_subscribed_member(ws_client_factory, monkeypatch):
|
||||||
calls = _fake_smtp(monkeypatch)
|
calls = _fake_smtp(monkeypatch)
|
||||||
instance1 = ws_client_factory()
|
instance1 = ws_client_factory()
|
||||||
instance2 = ws_client_factory()
|
instance2 = ws_client_factory()
|
||||||
@@ -88,18 +88,18 @@ def test_room_mention_emails_offline_subscribed_member(ws_client_factory, monkey
|
|||||||
with instance1.websocket_connect("/ws/chat") as alice_ws:
|
with instance1.websocket_connect("/ws/chat") as alice_ws:
|
||||||
alice_ws.send_json({"type": "join", "room_id": room["id"]})
|
alice_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||||
assert alice_ws.receive_json()["type"] == "joined"
|
assert alice_ws.receive_json()["type"] == "joined"
|
||||||
_send_and_sync(alice_ws, room["id"], f"hey @{bob['username']}, look at this")
|
_send_and_sync(alice_ws, room["id"], "no mention here, just a plain message")
|
||||||
|
|
||||||
assert len(calls) == 1
|
assert len(calls) == 1
|
||||||
email = calls[0]["message"]
|
email = calls[0]["message"]
|
||||||
assert email["To"] == bob["email"]
|
assert email["To"] == bob["email"]
|
||||||
assert f"New mention in #{room['name']}" in email["Subject"]
|
assert f"New message in #{room['name']}" in email["Subject"]
|
||||||
body = email.get_body(preferencelist=("plain",)).get_content()
|
body = email.get_body(preferencelist=("plain",)).get_content()
|
||||||
assert alice["username"] in body
|
assert alice["username"] in body
|
||||||
assert f"/rooms/{room['id']}" in body
|
assert f"/rooms/{room['id']}" in body
|
||||||
|
|
||||||
|
|
||||||
def test_room_message_without_mention_does_not_email_subscribed_member(ws_client_factory, monkeypatch):
|
def test_room_second_plain_message_does_not_reemail_before_read(ws_client_factory, monkeypatch):
|
||||||
calls = _fake_smtp(monkeypatch)
|
calls = _fake_smtp(monkeypatch)
|
||||||
instance1 = ws_client_factory()
|
instance1 = ws_client_factory()
|
||||||
instance2 = ws_client_factory()
|
instance2 = ws_client_factory()
|
||||||
@@ -113,9 +113,45 @@ def test_room_message_without_mention_does_not_email_subscribed_member(ws_client
|
|||||||
with instance1.websocket_connect("/ws/chat") as alice_ws:
|
with instance1.websocket_connect("/ws/chat") as alice_ws:
|
||||||
alice_ws.send_json({"type": "join", "room_id": room["id"]})
|
alice_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||||
assert alice_ws.receive_json()["type"] == "joined"
|
assert alice_ws.receive_json()["type"] == "joined"
|
||||||
_send_and_sync(alice_ws, room["id"], "hello room, no mention here")
|
|
||||||
|
|
||||||
assert calls == []
|
_send_and_sync(alice_ws, room["id"], "message one")
|
||||||
|
assert len(calls) == 1
|
||||||
|
|
||||||
|
# A second plain message while bob still hasn't read the first --
|
||||||
|
# no second email for the same unread burst.
|
||||||
|
_send_and_sync(alice_ws, room["id"], "message two")
|
||||||
|
assert len(calls) == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_room_mention_always_emails_even_mid_unread_burst(ws_client_factory, monkeypatch):
|
||||||
|
calls = _fake_smtp(monkeypatch)
|
||||||
|
instance1 = ws_client_factory()
|
||||||
|
instance2 = ws_client_factory()
|
||||||
|
|
||||||
|
alice = _register_ws(instance1, _unique("alice"))
|
||||||
|
bob = _register_ws(instance2, _unique("bob"))
|
||||||
|
room = instance1.post("/api/rooms", json={"name": _unique("general")}).json()
|
||||||
|
instance2.post(f"/api/rooms/{room['id']}/join")
|
||||||
|
_subscribe(instance2, room["id"], bob["username"])
|
||||||
|
|
||||||
|
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"
|
||||||
|
|
||||||
|
# First unread message (plain) -- emails once, uses up the debounce.
|
||||||
|
_send_and_sync(alice_ws, room["id"], "hey everyone")
|
||||||
|
assert len(calls) == 1
|
||||||
|
|
||||||
|
# A mention arriving while that first message is still unread --
|
||||||
|
# must email anyway, unlike a second plain message.
|
||||||
|
_send_and_sync(alice_ws, room["id"], f"@{bob['username']} specifically you")
|
||||||
|
assert len(calls) == 2
|
||||||
|
|
||||||
|
mention_email = calls[1]["message"]
|
||||||
|
assert mention_email["To"] == bob["email"]
|
||||||
|
assert f"New mention in #{room['name']}" in mention_email["Subject"]
|
||||||
|
body = mention_email.get_body(preferencelist=("plain",)).get_content()
|
||||||
|
assert f"{alice['username']} mentioned you" in body
|
||||||
|
|
||||||
|
|
||||||
def test_room_mention_does_not_email_unsubscribed_member(ws_client_factory, monkeypatch):
|
def test_room_mention_does_not_email_unsubscribed_member(ws_client_factory, monkeypatch):
|
||||||
@@ -123,7 +159,7 @@ def test_room_mention_does_not_email_unsubscribed_member(ws_client_factory, monk
|
|||||||
instance1 = ws_client_factory()
|
instance1 = ws_client_factory()
|
||||||
instance2 = ws_client_factory()
|
instance2 = ws_client_factory()
|
||||||
|
|
||||||
alice = _register_ws(instance1, _unique("alice"))
|
_register_ws(instance1, _unique("alice"))
|
||||||
bob = _register_ws(instance2, _unique("bob"))
|
bob = _register_ws(instance2, _unique("bob"))
|
||||||
room = instance1.post("/api/rooms", json={"name": _unique("general")}).json()
|
room = instance1.post("/api/rooms", json={"name": _unique("general")}).json()
|
||||||
instance2.post(f"/api/rooms/{room['id']}/join")
|
instance2.post(f"/api/rooms/{room['id']}/join")
|
||||||
@@ -137,7 +173,7 @@ def test_room_mention_does_not_email_unsubscribed_member(ws_client_factory, monk
|
|||||||
assert calls == []
|
assert calls == []
|
||||||
|
|
||||||
|
|
||||||
def test_room_mention_does_not_email_online_subscribed_member(ws_client_factory, monkeypatch):
|
def test_room_message_does_not_email_online_subscribed_member(ws_client_factory, monkeypatch):
|
||||||
calls = _fake_smtp(monkeypatch)
|
calls = _fake_smtp(monkeypatch)
|
||||||
instance1 = ws_client_factory()
|
instance1 = ws_client_factory()
|
||||||
instance2 = ws_client_factory()
|
instance2 = ws_client_factory()
|
||||||
@@ -159,7 +195,7 @@ def test_room_mention_does_not_email_online_subscribed_member(ws_client_factory,
|
|||||||
assert calls == []
|
assert calls == []
|
||||||
|
|
||||||
|
|
||||||
def test_room_mention_email_debounced_to_first_unread_then_resets_after_read(ws_client_factory, monkeypatch):
|
def test_room_notifications_reset_after_read(ws_client_factory, monkeypatch):
|
||||||
calls = _fake_smtp(monkeypatch)
|
calls = _fake_smtp(monkeypatch)
|
||||||
instance1 = ws_client_factory()
|
instance1 = ws_client_factory()
|
||||||
instance2 = ws_client_factory()
|
instance2 = ws_client_factory()
|
||||||
@@ -173,13 +209,7 @@ def test_room_mention_email_debounced_to_first_unread_then_resets_after_read(ws_
|
|||||||
with instance1.websocket_connect("/ws/chat") as alice_ws:
|
with instance1.websocket_connect("/ws/chat") as alice_ws:
|
||||||
alice_ws.send_json({"type": "join", "room_id": room["id"]})
|
alice_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||||
assert alice_ws.receive_json()["type"] == "joined"
|
assert alice_ws.receive_json()["type"] == "joined"
|
||||||
|
_send_and_sync(alice_ws, room["id"], "message one")
|
||||||
_send_and_sync(alice_ws, room["id"], f"@{bob['username']} first mention")
|
|
||||||
assert len(calls) == 1
|
|
||||||
|
|
||||||
# A second mention while bob still hasn't read the first -- no
|
|
||||||
# second email for the same burst.
|
|
||||||
_send_and_sync(alice_ws, room["id"], f"@{bob['username']} second mention")
|
|
||||||
assert len(calls) == 1
|
assert len(calls) == 1
|
||||||
|
|
||||||
instance2.post(
|
instance2.post(
|
||||||
@@ -191,7 +221,7 @@ def test_room_mention_email_debounced_to_first_unread_then_resets_after_read(ws_
|
|||||||
with instance1.websocket_connect("/ws/chat") as alice_ws:
|
with instance1.websocket_connect("/ws/chat") as alice_ws:
|
||||||
alice_ws.send_json({"type": "join", "room_id": room["id"]})
|
alice_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||||
assert alice_ws.receive_json()["type"] == "joined"
|
assert alice_ws.receive_json()["type"] == "joined"
|
||||||
_send_and_sync(alice_ws, room["id"], f"@{bob['username']} third mention")
|
_send_and_sync(alice_ws, room["id"], "message two")
|
||||||
|
|
||||||
assert len(calls) == 2
|
assert len(calls) == 2
|
||||||
|
|
||||||
|
|||||||
@@ -68,9 +68,10 @@ export function hideDm(roomId: string): Promise<void> {
|
|||||||
return apiFetch<void>(`/api/rooms/${roomId}/hide`, { method: 'POST' })
|
return apiFetch<void>(`/api/rooms/${roomId}/hide`, { method: 'POST' })
|
||||||
}
|
}
|
||||||
|
|
||||||
// #67: per-viewer opt-in for email-on-mention in this room. 400s for a DM
|
// #67: per-viewer opt-in for email on this room's first unread message
|
||||||
// (backend rejects it -- see set_room_email_notifications), so callers
|
// and every mention. 400s for a DM (backend rejects it -- see
|
||||||
// should only expose the toggle for a non-DM room.
|
// set_room_email_notifications), so callers should only expose the
|
||||||
|
// toggle for a non-DM room.
|
||||||
export function updateRoomNotifications(roomId: string, emailNotifications: boolean): Promise<void> {
|
export function updateRoomNotifications(roomId: string, emailNotifications: boolean): Promise<void> {
|
||||||
return apiFetch<void>(`/api/rooms/${roomId}/notifications`, {
|
return apiFetch<void>(`/api/rooms/${roomId}/notifications`, {
|
||||||
method: 'PATCH',
|
method: 'PATCH',
|
||||||
|
|||||||
@@ -398,8 +398,8 @@ export function RoomInfoPanel({
|
|||||||
<div className="room-info-section">
|
<div className="room-info-section">
|
||||||
<div className="toggle-row">
|
<div className="toggle-row">
|
||||||
<div className="toggle-label">
|
<div className="toggle-label">
|
||||||
<span className="t">Email me on mentions</span>
|
<span className="t">Email notifications</span>
|
||||||
<span className="d">Sent only while you're offline</span>
|
<span className="d">New messages and mentions, while you're offline</span>
|
||||||
</div>
|
</div>
|
||||||
<label className="switch">
|
<label className="switch">
|
||||||
<input
|
<input
|
||||||
|
|||||||
@@ -87,8 +87,9 @@ export interface MyRoomItem extends Room {
|
|||||||
// #52: the other participant, only for is_dm rooms -- see backend
|
// #52: the other participant, only for is_dm rooms -- see backend
|
||||||
// schemas/room.py's MyRoomItem for why this is precomputed server-side.
|
// schemas/room.py's MyRoomItem for why this is precomputed server-side.
|
||||||
dm_partner: DmPartnerInfo | null
|
dm_partner: DmPartnerInfo | null
|
||||||
// #67: this viewer's own opt-in for "email me when mentioned here while
|
// #67: this viewer's own opt-in for email while offline -- the room's
|
||||||
// offline" -- always false for a DM (see backend's message_events.py).
|
// first unread message plus every mention (see backend's
|
||||||
|
// message_events.py). Always false for a DM.
|
||||||
email_notifications: boolean
|
email_notifications: boolean
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user