Private
Public Access
Add direct messages (#52)
A DM is a Room with a new is_dm flag, not a separate model -- reuses all the membership/message/WS plumbing Room already has instead of duplicating it. The room's `name` (still required + globally unique) is an internal, never-displayed token derived deterministically from the two participants' sorted user IDs (dm_room_name), which makes find-or-create a single indexed lookup and gets free race-condition safety from the existing unique constraint -- a concurrent double- start from both people just hits the same IntegrityError->retry-as- lookup path create_room already established. Both participants get the plain 'member' role (no owner/admin distinction makes sense for a 1:1 DM), which incidentally reuses every existing role gate to block add-member, room-settings edits, and join-via-browse on a DM for free. update_room also gets an explicit is_dm guard independent of that, since renaming a DM isn't just a privacy concern -- it would silently corrupt the find-or-create invariant. DMs are excluded from both Browse Rooms and the admin portal's room listing (fully private, per scope). GET /api/rooms/mine precomputes each DM's other participant (name, avatar, presence) as dm_partner in one batched query, so the sidebar can render a DM row without a fetch per row. Frontend: a new "Direct Messages" sidebar section (searchable by partner name, not the internal room name), clicking someone in the People list starts or resumes a DM, and the chat header/composer/RoomInfoPanel all render the partner's identity instead of a room name where it's a DM. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,35 @@
|
||||
"""room is_dm flag for direct messages
|
||||
|
||||
Revision ID: 9ca717f837c2
|
||||
Revises: 9484fbd1cb3a
|
||||
Create Date: 2026-08-19 15:50:25.781735
|
||||
|
||||
"""
|
||||
from typing import Sequence, Union
|
||||
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision: str = '9ca717f837c2'
|
||||
down_revision: Union[str, Sequence[str], None] = '9484fbd1cb3a'
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
"""Upgrade schema."""
|
||||
# server_default backfills existing rows; dropped right after since the
|
||||
# model itself only sets a Python-side default (see is_archived above).
|
||||
op.add_column(
|
||||
'rooms', sa.Column('is_dm', sa.Boolean(), nullable=False, server_default=sa.false())
|
||||
)
|
||||
op.alter_column('rooms', 'is_dm', server_default=None)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
"""Downgrade schema."""
|
||||
# ### commands auto generated by Alembic - please adjust! ###
|
||||
op.drop_column('rooms', 'is_dm')
|
||||
# ### end Alembic commands ###
|
||||
@@ -15,6 +15,10 @@ class Room(Base):
|
||||
description: Mapped[str | None] = mapped_column(Text)
|
||||
is_private: Mapped[bool] = mapped_column(Boolean, default=False, nullable=False)
|
||||
is_archived: Mapped[bool] = mapped_column(Boolean, default=False, nullable=False)
|
||||
# #52: a DM is a Room whose `name` is an internal, never-displayed
|
||||
# deterministic token (see room_service.dm_room_name) rather than a
|
||||
# user-chosen name -- see that function's docstring for the scheme.
|
||||
is_dm: Mapped[bool] = mapped_column(Boolean, default=False, nullable=False)
|
||||
owner_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("users.id"), nullable=False)
|
||||
created_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True), server_default=func.now(), nullable=False
|
||||
|
||||
@@ -17,6 +17,7 @@ from app.schemas.message import LinkPreviewInfo, MessageFileInfo, MessageRead
|
||||
from app.schemas.message_file import MessageFileCreated
|
||||
from app.schemas.message_image import MessageImageCreated
|
||||
from app.schemas.room import (
|
||||
DmPartnerInfo,
|
||||
MyRoomItem,
|
||||
RoomAttachmentRead,
|
||||
RoomCreate,
|
||||
@@ -26,6 +27,7 @@ from app.schemas.room import (
|
||||
RoomMemberRoleUpdate,
|
||||
RoomRead,
|
||||
RoomUpdate,
|
||||
StartDmRequest,
|
||||
TransferOwnershipRequest,
|
||||
)
|
||||
from app.schemas.webhook import (
|
||||
@@ -45,6 +47,8 @@ from app.services.message_service import (
|
||||
from app.services.upload_settings_service import format_mb, get_upload_settings
|
||||
from app.services.room_service import (
|
||||
AlreadyMemberError,
|
||||
CannotDmSelfError,
|
||||
CannotModifyDmError,
|
||||
CannotRemoveOwnerError,
|
||||
DuplicateRoomError,
|
||||
InsufficientRoleError,
|
||||
@@ -57,6 +61,7 @@ from app.services.room_service import (
|
||||
change_member_role,
|
||||
create_room,
|
||||
delete_room,
|
||||
find_or_create_dm,
|
||||
get_room,
|
||||
join_room,
|
||||
leave_room,
|
||||
@@ -105,6 +110,20 @@ async def create_room_endpoint(
|
||||
raise HTTPException(status_code=409, detail="A room with this name already exists")
|
||||
|
||||
|
||||
@router.post("/dm", response_model=RoomRead, status_code=201)
|
||||
async def start_dm_endpoint(
|
||||
data: StartDmRequest,
|
||||
current_user: User = Depends(get_current_user),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
try:
|
||||
return await find_or_create_dm(db, current_user.id, data.other_user_id)
|
||||
except CannotDmSelfError:
|
||||
raise HTTPException(status_code=400, detail="Cannot start a DM with yourself")
|
||||
except TargetUserNotFoundError:
|
||||
raise HTTPException(status_code=404, detail="No user with that ID")
|
||||
|
||||
|
||||
@router.get("", response_model=list[RoomListItem])
|
||||
async def list_rooms_endpoint(
|
||||
current_user: User = Depends(get_current_user),
|
||||
@@ -117,6 +136,7 @@ async def list_rooms_endpoint(
|
||||
name=room.name,
|
||||
description=room.description,
|
||||
is_private=room.is_private,
|
||||
is_dm=room.is_dm,
|
||||
owner_id=room.owner_id,
|
||||
created_at=room.created_at,
|
||||
is_member=is_member,
|
||||
@@ -127,23 +147,38 @@ async def list_rooms_endpoint(
|
||||
|
||||
@router.get("/mine", response_model=list[MyRoomItem])
|
||||
async def list_my_rooms_endpoint(
|
||||
request: Request,
|
||||
current_user: User = Depends(get_current_user),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
rooms = await list_member_rooms(db, current_user.id)
|
||||
partner_ids = [partner.id for *_, partner in rooms if partner is not None]
|
||||
online_ids = await request.app.state.global_presence.online_user_ids(partner_ids)
|
||||
return [
|
||||
MyRoomItem(
|
||||
id=room.id,
|
||||
name=room.name,
|
||||
description=room.description,
|
||||
is_private=room.is_private,
|
||||
is_dm=room.is_dm,
|
||||
owner_id=room.owner_id,
|
||||
created_at=room.created_at,
|
||||
role=role,
|
||||
has_unread=has_unread,
|
||||
has_mention=has_mention,
|
||||
dm_partner=(
|
||||
DmPartnerInfo(
|
||||
user_id=partner.id,
|
||||
username=partner.username,
|
||||
display_name=partner.display_name,
|
||||
avatar_filename=partner.avatar_filename,
|
||||
status=_member_status(partner, online_ids),
|
||||
)
|
||||
for room, role, has_unread, has_mention in rooms
|
||||
if partner is not None
|
||||
else None
|
||||
),
|
||||
)
|
||||
for room, role, has_unread, has_mention, partner in rooms
|
||||
]
|
||||
|
||||
|
||||
@@ -167,6 +202,8 @@ async def update_room_endpoint(
|
||||
raise HTTPException(status_code=404, detail="Room not found")
|
||||
except DuplicateRoomError:
|
||||
raise HTTPException(status_code=409, detail="A room with this name already exists")
|
||||
except CannotModifyDmError:
|
||||
raise HTTPException(status_code=400, detail="DMs can't be edited")
|
||||
|
||||
|
||||
@router.delete("/{room_id}", status_code=204)
|
||||
|
||||
@@ -26,6 +26,7 @@ class RoomRead(BaseModel):
|
||||
name: str
|
||||
description: str | None
|
||||
is_private: bool
|
||||
is_dm: bool
|
||||
owner_id: uuid.UUID
|
||||
created_at: datetime
|
||||
|
||||
@@ -34,6 +35,14 @@ class RoomListItem(RoomRead):
|
||||
is_member: bool
|
||||
|
||||
|
||||
class DmPartnerInfo(BaseModel):
|
||||
user_id: uuid.UUID
|
||||
username: str
|
||||
display_name: str | None
|
||||
avatar_filename: str | None
|
||||
status: Literal["online", "offline"]
|
||||
|
||||
|
||||
class MyRoomItem(RoomRead):
|
||||
role: RoomRole
|
||||
# Whether this room has a message newer than the caller's last_read_at --
|
||||
@@ -44,6 +53,15 @@ class MyRoomItem(RoomRead):
|
||||
# over has_unread in the sidebar (see RoomRow.tsx), not shown alongside
|
||||
# it.
|
||||
has_mention: bool
|
||||
# #52: populated only when is_dm is true -- the *other* participant,
|
||||
# precomputed here so the sidebar can render a DM row (their name +
|
||||
# avatar, not this room's internal `name`) without a second fetch per
|
||||
# row. None for a regular room.
|
||||
dm_partner: DmPartnerInfo | None = None
|
||||
|
||||
|
||||
class StartDmRequest(BaseModel):
|
||||
other_user_id: uuid.UUID
|
||||
|
||||
|
||||
class RoomMemberRead(BaseModel):
|
||||
|
||||
@@ -93,9 +93,14 @@ async def set_user_site_admin(
|
||||
|
||||
|
||||
async def list_rooms_admin(db: AsyncSession) -> list[tuple[Room, int]]:
|
||||
# #52: DMs are fully private, not just unlisted -- excluded here rather
|
||||
# than merely omitted from the response, so there's no admin-portal
|
||||
# surface (this list, or the audit log via anything that touches this
|
||||
# query) that reveals a DM even exists between two users.
|
||||
result = await db.execute(
|
||||
select(Room, func.count(RoomMembership.user_id))
|
||||
.outerjoin(RoomMembership, RoomMembership.room_id == Room.id)
|
||||
.where(Room.is_dm.is_(False))
|
||||
.group_by(Room.id)
|
||||
.order_by(Room.created_at)
|
||||
)
|
||||
|
||||
@@ -60,6 +60,25 @@ class AlreadyMemberError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
class CannotDmSelfError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
class CannotModifyDmError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
def dm_room_name(user_a_id: uuid.UUID, user_b_id: uuid.UUID) -> str:
|
||||
"""Deterministic, internal-only name for the DM room between these two
|
||||
users -- same canonical string regardless of argument order, so
|
||||
find_or_create_dm can look up an existing DM with a single indexed
|
||||
query (Room.name is already unique+indexed) instead of a membership-set
|
||||
join. Never shown to a user -- the frontend renders a DM's dm_partner
|
||||
info instead of its `name` (see MyRoomItem)."""
|
||||
ids = sorted((str(user_a_id), str(user_b_id)))
|
||||
return f"dm:{ids[0]}:{ids[1]}"
|
||||
|
||||
|
||||
async def create_room(db: AsyncSession, owner_id: uuid.UUID, data: RoomCreate) -> Room:
|
||||
room = Room(
|
||||
name=data.name,
|
||||
@@ -80,10 +99,50 @@ async def create_room(db: AsyncSession, owner_id: uuid.UUID, data: RoomCreate) -
|
||||
return room
|
||||
|
||||
|
||||
async def find_or_create_dm(db: AsyncSession, user_id: uuid.UUID, other_user_id: uuid.UUID) -> Room:
|
||||
if user_id == other_user_id:
|
||||
raise CannotDmSelfError()
|
||||
other = await db.get(User, other_user_id)
|
||||
if other is None:
|
||||
raise TargetUserNotFoundError()
|
||||
|
||||
name = dm_room_name(user_id, other_user_id)
|
||||
result = await db.execute(select(Room).where(Room.name == name))
|
||||
room = result.scalar_one_or_none()
|
||||
if room is not None:
|
||||
return room
|
||||
|
||||
# is_private=True is belt-and-suspenders here -- list_open_rooms also
|
||||
# excludes is_dm directly -- but it's also just semantically correct: a
|
||||
# DM genuinely is a private room. Both participants get the plain
|
||||
# `member` role (there's no meaningful owner/admin distinction for a
|
||||
# 1:1 DM); `owner_id` still has to be someone to satisfy the column,
|
||||
# but nothing reads it as meaningful for a DM.
|
||||
room = Room(name=name, is_private=True, is_dm=True, owner_id=user_id)
|
||||
db.add(room)
|
||||
try:
|
||||
await db.flush()
|
||||
except IntegrityError:
|
||||
# Lost a race with a concurrent find_or_create_dm for the same pair
|
||||
# (e.g. both people click "message" on each other at once) -- the
|
||||
# unique constraint on `name` is exactly what caught it, same
|
||||
# pattern as create_room's DuplicateRoomError. The row that won the
|
||||
# race is the room we actually want.
|
||||
await db.rollback()
|
||||
result = await db.execute(select(Room).where(Room.name == name))
|
||||
return result.scalar_one()
|
||||
|
||||
db.add(RoomMembership(room_id=room.id, user_id=user_id, role=RoomRole.member))
|
||||
db.add(RoomMembership(room_id=room.id, user_id=other_user_id, role=RoomRole.member))
|
||||
await db.commit()
|
||||
await db.refresh(room)
|
||||
return room
|
||||
|
||||
|
||||
async def list_open_rooms(db: AsyncSession, user_id: uuid.UUID) -> list[tuple[Room, bool]]:
|
||||
result = await db.execute(
|
||||
select(Room)
|
||||
.where(Room.is_private.is_(False), Room.is_archived.is_(False))
|
||||
.where(Room.is_private.is_(False), Room.is_archived.is_(False), Room.is_dm.is_(False))
|
||||
.options(selectinload(Room.memberships))
|
||||
.order_by(Room.created_at, Room.id)
|
||||
)
|
||||
@@ -95,7 +154,7 @@ async def list_open_rooms(db: AsyncSession, user_id: uuid.UUID) -> list[tuple[Ro
|
||||
|
||||
async def list_member_rooms(
|
||||
db: AsyncSession, user_id: uuid.UUID
|
||||
) -> list[tuple[Room, RoomRole, bool, bool]]:
|
||||
) -> list[tuple[Room, RoomRole, bool, bool, User | None]]:
|
||||
last_message_at = (
|
||||
select(func.max(Message.created_at))
|
||||
.where(Message.room_id == Room.id)
|
||||
@@ -130,9 +189,30 @@ async def list_member_rooms(
|
||||
# device's fetch and another's.
|
||||
.order_by(Room.created_at, Room.id)
|
||||
)
|
||||
rows = result.all()
|
||||
|
||||
# #52: one batched follow-up query for every DM room's *other*
|
||||
# participant, rather than a fetch per row -- a DM only ever has
|
||||
# exactly two members, so "the other one" is unambiguous.
|
||||
dm_room_ids = [room.id for room, *_ in rows if room.is_dm]
|
||||
partners_by_room: dict[uuid.UUID, User] = {}
|
||||
if dm_room_ids:
|
||||
partner_result = await db.execute(
|
||||
select(RoomMembership.room_id, User)
|
||||
.join(User, User.id == RoomMembership.user_id)
|
||||
.where(RoomMembership.room_id.in_(dm_room_ids), RoomMembership.user_id != user_id)
|
||||
)
|
||||
partners_by_room = {room_id: user for room_id, user in partner_result.all()}
|
||||
|
||||
return [
|
||||
(room, role, last_message_at is not None and last_message_at > last_read_at, has_mention)
|
||||
for room, role, last_read_at, last_message_at, has_mention in result.all()
|
||||
(
|
||||
room,
|
||||
role,
|
||||
last_message_at is not None and last_message_at > last_read_at,
|
||||
has_mention,
|
||||
partners_by_room.get(room.id),
|
||||
)
|
||||
for room, role, last_read_at, last_message_at, has_mention in rows
|
||||
]
|
||||
|
||||
|
||||
@@ -200,6 +280,14 @@ async def add_member(
|
||||
|
||||
|
||||
async def update_room(db: AsyncSession, room: Room, data: RoomUpdate) -> Room:
|
||||
# A DM's `name` is an internal token find_or_create_dm's lookup depends
|
||||
# on being stable -- renaming it (even via the #48 site-admin bypass in
|
||||
# the router) would silently orphan that invariant, not just leak a
|
||||
# detail that's supposed to stay private. Blocked here, not just in the
|
||||
# UI, since it's a correctness issue for every caller, not a permission
|
||||
# one.
|
||||
if room.is_dm:
|
||||
raise CannotModifyDmError()
|
||||
if data.name is not None:
|
||||
room.name = data.name
|
||||
if data.description is not None:
|
||||
|
||||
@@ -0,0 +1,178 @@
|
||||
import uuid
|
||||
|
||||
from sqlalchemy import select
|
||||
|
||||
from app.models import Room, RoomMembership
|
||||
from app.services.room_service import dm_room_name
|
||||
from tests.conftest import login_as, register_and_login
|
||||
|
||||
|
||||
async def test_start_dm_creates_private_room_with_both_members(client, db_session):
|
||||
alice = await register_and_login(client, db_session, username="alice")
|
||||
await client.post("/api/auth/logout")
|
||||
bob = await register_and_login(client, db_session, username="bob")
|
||||
await client.post("/api/auth/logout")
|
||||
await login_as(client, "alice")
|
||||
|
||||
resp = await client.post("/api/rooms/dm", json={"other_user_id": bob["id"]})
|
||||
assert resp.status_code == 201, resp.text
|
||||
room = resp.json()
|
||||
assert room["is_dm"] is True
|
||||
assert room["is_private"] is True
|
||||
# The internal name is never meant to be shown, but its scheme is part
|
||||
# of the contract find_or_create_dm relies on -- pin it here so a
|
||||
# future refactor can't silently change it without this test noticing.
|
||||
assert room["name"] == dm_room_name(uuid.UUID(alice["id"]), uuid.UUID(bob["id"]))
|
||||
|
||||
result = await db_session.execute(
|
||||
select(RoomMembership.user_id).where(RoomMembership.room_id == uuid.UUID(room["id"]))
|
||||
)
|
||||
member_ids = {str(row[0]) for row in result.all()}
|
||||
assert member_ids == {alice["id"], bob["id"]}
|
||||
|
||||
|
||||
async def test_start_dm_is_idempotent_regardless_of_who_initiates(client, db_session):
|
||||
alice = await register_and_login(client, db_session, username="alice")
|
||||
await client.post("/api/auth/logout")
|
||||
bob = await register_and_login(client, db_session, username="bob")
|
||||
|
||||
resp1 = await client.post("/api/rooms/dm", json={"other_user_id": alice["id"]})
|
||||
assert resp1.status_code == 201
|
||||
room_id = resp1.json()["id"]
|
||||
|
||||
# bob -> alice again should return the same room, not create a second one.
|
||||
resp2 = await client.post("/api/rooms/dm", json={"other_user_id": alice["id"]})
|
||||
assert resp2.status_code == 201
|
||||
assert resp2.json()["id"] == room_id
|
||||
|
||||
await client.post("/api/auth/logout")
|
||||
await login_as(client, "alice")
|
||||
# alice -> bob (reversed direction) should also find the same room.
|
||||
resp3 = await client.post("/api/rooms/dm", json={"other_user_id": bob["id"]})
|
||||
assert resp3.status_code == 201
|
||||
assert resp3.json()["id"] == room_id
|
||||
|
||||
|
||||
async def test_start_dm_rejects_self(client, db_session):
|
||||
alice = await register_and_login(client, db_session, username="alice")
|
||||
resp = await client.post("/api/rooms/dm", json={"other_user_id": alice["id"]})
|
||||
assert resp.status_code == 400
|
||||
|
||||
|
||||
async def test_start_dm_404s_for_unknown_user(client, db_session):
|
||||
await register_and_login(client, db_session, username="alice")
|
||||
resp = await client.post("/api/rooms/dm", json={"other_user_id": str(uuid.uuid4())})
|
||||
assert resp.status_code == 404
|
||||
|
||||
|
||||
async def test_dm_excluded_from_browse_rooms(client, db_session):
|
||||
alice = await register_and_login(client, db_session, username="alice")
|
||||
await client.post("/api/auth/logout")
|
||||
bob = await register_and_login(client, db_session, username="bob")
|
||||
await client.post("/api/rooms/dm", json={"other_user_id": alice["id"]})
|
||||
|
||||
# A third user should never see the DM in the open-rooms listing, even
|
||||
# though find_or_create_dm sets is_private=True (which alone would
|
||||
# already exclude it) -- confirms the belt-and-suspenders is_dm filter
|
||||
# in list_open_rooms is doing something, not just is_private.
|
||||
await client.post("/api/auth/logout")
|
||||
await register_and_login(client, db_session, username="carol")
|
||||
resp = await client.get("/api/rooms")
|
||||
assert resp.status_code == 200
|
||||
assert all(not r["is_dm"] for r in resp.json())
|
||||
|
||||
|
||||
async def test_dm_excluded_from_admin_room_list(client, db_session):
|
||||
alice = await register_and_login(client, db_session, username="alice")
|
||||
await client.post("/api/auth/logout")
|
||||
bob = await register_and_login(client, db_session, username="bob")
|
||||
await client.post("/api/rooms/dm", json={"other_user_id": alice["id"]})
|
||||
|
||||
await client.post("/api/auth/logout")
|
||||
admin = await register_and_login(client, db_session, username="dave")
|
||||
from app.models import User
|
||||
|
||||
user = await db_session.get(User, uuid.UUID(admin["id"]))
|
||||
user.is_site_admin = True
|
||||
await db_session.commit()
|
||||
|
||||
resp = await client.get("/api/admin/rooms")
|
||||
assert resp.status_code == 200
|
||||
names = [r["name"] for r in resp.json()]
|
||||
assert dm_room_name(uuid.UUID(alice["id"]), uuid.UUID(bob["id"])) not in names
|
||||
|
||||
|
||||
async def test_dm_appears_in_mine_with_partner_info(client, db_session):
|
||||
alice = await register_and_login(client, db_session, username="alice")
|
||||
await client.post("/api/auth/logout")
|
||||
bob = await register_and_login(client, db_session, username="bob", password="password123")
|
||||
# Give bob a display name so the partner payload's precedence is checked
|
||||
# for something other than the fallback username.
|
||||
await client.patch("/api/auth/me", json={"display_name": "Bobby"})
|
||||
|
||||
dm = (await client.post("/api/rooms/dm", json={"other_user_id": alice["id"]})).json()
|
||||
|
||||
await client.post("/api/auth/logout")
|
||||
await login_as(client, "alice")
|
||||
mine = (await client.get("/api/rooms/mine")).json()
|
||||
dm_entry = next(r for r in mine if r["id"] == dm["id"])
|
||||
assert dm_entry["is_dm"] is True
|
||||
assert dm_entry["dm_partner"]["user_id"] == bob["id"]
|
||||
assert dm_entry["dm_partner"]["username"] == "bob"
|
||||
assert dm_entry["dm_partner"]["display_name"] == "Bobby"
|
||||
|
||||
# A regular room's dm_partner is always null.
|
||||
room = (await client.post("/api/rooms", json={"name": "general"})).json()
|
||||
mine = (await client.get("/api/rooms/mine")).json()
|
||||
room_entry = next(r for r in mine if r["id"] == room["id"])
|
||||
assert room_entry["is_dm"] is False
|
||||
assert room_entry["dm_partner"] is None
|
||||
|
||||
|
||||
async def test_dm_cannot_be_updated(client, db_session):
|
||||
alice = await register_and_login(client, db_session, username="alice")
|
||||
await client.post("/api/auth/logout")
|
||||
bob = await register_and_login(client, db_session, username="bob")
|
||||
dm = (await client.post("/api/rooms/dm", json={"other_user_id": alice["id"]})).json()
|
||||
|
||||
# bob is a plain 'member' of the DM (no admin/owner role exists for a
|
||||
# DM), so this 403s on the ordinary role gate before ever reaching
|
||||
# update_room's own is_dm guard.
|
||||
resp = await client.patch(f"/api/rooms/{dm['id']}", json={"name": "renamed"})
|
||||
assert resp.status_code == 403
|
||||
|
||||
# A site admin bypasses that role gate (see #48) -- confirms the
|
||||
# explicit is_dm guard inside update_room itself is what stops this,
|
||||
# not just incidental role-based protection.
|
||||
await client.post("/api/auth/logout")
|
||||
admin = await register_and_login(client, db_session, username="carol")
|
||||
from app.models import User
|
||||
|
||||
user = await db_session.get(User, uuid.UUID(admin["id"]))
|
||||
user.is_site_admin = True
|
||||
await db_session.commit()
|
||||
resp = await client.patch(f"/api/rooms/{dm['id']}", json={"name": "renamed"})
|
||||
assert resp.status_code == 400
|
||||
|
||||
|
||||
async def test_dm_rejects_add_member_and_join(client, db_session):
|
||||
alice = await register_and_login(client, db_session, username="alice")
|
||||
await client.post("/api/auth/logout")
|
||||
bob = await register_and_login(client, db_session, username="bob")
|
||||
dm = (await client.post("/api/rooms/dm", json={"other_user_id": alice["id"]})).json()
|
||||
|
||||
await client.post("/api/auth/logout")
|
||||
carol = await register_and_login(client, db_session, username="carol")
|
||||
|
||||
# Neither participant has admin/owner role in a DM, so adding a third
|
||||
# person 403s on the existing role gate.
|
||||
await client.post("/api/auth/logout")
|
||||
await login_as(client, "bob")
|
||||
resp = await client.post(f"/api/rooms/{dm['id']}/members", json={"user_id": carol["id"]})
|
||||
assert resp.status_code == 403
|
||||
|
||||
# is_private=True on the DM already blocks the plain join endpoint too.
|
||||
await client.post("/api/auth/logout")
|
||||
await login_as(client, "carol")
|
||||
resp = await client.post(f"/api/rooms/{dm['id']}/join")
|
||||
assert resp.status_code == 400
|
||||
@@ -29,6 +29,15 @@ export function createRoom(
|
||||
})
|
||||
}
|
||||
|
||||
// #52: find-or-create -- returns the existing DM with this person if one
|
||||
// already exists, rather than always creating a new room.
|
||||
export function startDm(otherUserId: string): Promise<Room> {
|
||||
return apiFetch<Room>('/api/rooms/dm', {
|
||||
method: 'POST',
|
||||
body: JSON.stringify({ other_user_id: otherUserId }),
|
||||
})
|
||||
}
|
||||
|
||||
export function updateRoom(
|
||||
roomId: string,
|
||||
data: { name?: string; description?: string; is_private?: boolean },
|
||||
|
||||
@@ -224,8 +224,16 @@ export function ChatPane({
|
||||
</button>
|
||||
)}
|
||||
<div className="chat-pane-title-block">
|
||||
<div className="chat-pane-title">#{room.name}</div>
|
||||
<div className="chat-pane-subtitle">{members.length} member{members.length === 1 ? '' : 's'}</div>
|
||||
<div className="chat-pane-title">
|
||||
{room.dm_partner ? room.dm_partner.display_name || room.dm_partner.username : `#${room.name}`}
|
||||
</div>
|
||||
<div className="chat-pane-subtitle">
|
||||
{room.dm_partner
|
||||
? room.dm_partner.status === 'online'
|
||||
? 'Online'
|
||||
: 'Offline'
|
||||
: `${members.length} member${members.length === 1 ? '' : 's'}`}
|
||||
</div>
|
||||
</div>
|
||||
<button
|
||||
type="button"
|
||||
@@ -259,7 +267,8 @@ export function ChatPane({
|
||||
/>
|
||||
<Composer
|
||||
roomId={room.id}
|
||||
roomName={room.name}
|
||||
roomName={room.dm_partner ? room.dm_partner.display_name || room.dm_partner.username : room.name}
|
||||
isDm={room.is_dm}
|
||||
members={members}
|
||||
rooms={rooms}
|
||||
disabled={!connected}
|
||||
|
||||
@@ -22,6 +22,7 @@ import './Composer.css'
|
||||
interface ComposerProps {
|
||||
roomId: string
|
||||
roomName: string
|
||||
isDm?: boolean
|
||||
members: RoomMember[]
|
||||
// #47: rooms this user belongs to, for the #roomname autocomplete --
|
||||
// deliberately the same list ChatPane already resolves message-display
|
||||
@@ -89,7 +90,7 @@ function AttachMenu({ onPickPhoto, onPickFile, onClose }: AttachMenuProps) {
|
||||
)
|
||||
}
|
||||
|
||||
export function Composer({ roomId, roomName, members, rooms, disabled, onSend }: ComposerProps) {
|
||||
export function Composer({ roomId, roomName, isDm, members, rooms, disabled, onSend }: ComposerProps) {
|
||||
const [value, setValue] = useState('')
|
||||
const [pendingImage, setPendingImage] = useState<{ id: string; previewUrl: string } | null>(null)
|
||||
const [pendingFile, setPendingFile] = useState<{ id: string; filename: string; size: number } | null>(
|
||||
@@ -488,7 +489,13 @@ export function Composer({ roomId, roomName, members, rooms, disabled, onSend }:
|
||||
}}
|
||||
onSelect={handleSelectionChange}
|
||||
onKeyDown={handleKeyDown}
|
||||
placeholder={disabled ? (online ? 'Connecting…' : "You're offline") : `Message #${roomName}`}
|
||||
placeholder={
|
||||
disabled
|
||||
? online
|
||||
? 'Connecting…'
|
||||
: "You're offline"
|
||||
: `Message ${isDm ? roomName : `#${roomName}`}`
|
||||
}
|
||||
spellCheck
|
||||
/>
|
||||
{mentionQuery && mentionMatches.length > 0 && (
|
||||
|
||||
@@ -389,6 +389,28 @@
|
||||
border-bottom: none;
|
||||
}
|
||||
|
||||
.modal-list-row-button {
|
||||
width: 100%;
|
||||
background: transparent;
|
||||
border-left: none;
|
||||
border-right: none;
|
||||
border-top: none;
|
||||
text-align: left;
|
||||
font: inherit;
|
||||
color: inherit;
|
||||
cursor: pointer;
|
||||
border-radius: var(--radius);
|
||||
}
|
||||
|
||||
.modal-list-row-button:hover:not(:disabled) {
|
||||
background: var(--ds-surface-2);
|
||||
}
|
||||
|
||||
.modal-list-row-button:disabled {
|
||||
cursor: not-allowed;
|
||||
opacity: 0.7;
|
||||
}
|
||||
|
||||
.modal-list-row-body {
|
||||
flex: 1;
|
||||
min-width: 0;
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import { useEffect, useMemo, useState } from 'react'
|
||||
import { ApiError } from '../api/client'
|
||||
import { startDm } from '../api/rooms'
|
||||
import { getUserAvatarUrl, listOnlineUserIds, listUserDirectory } from '../api/users'
|
||||
import { useAuth } from '../context/AuthContext'
|
||||
import { hashIndex } from '../lib/avatar'
|
||||
import type { UserDirectoryEntry } from '../types'
|
||||
import { UserAvatar } from './UserAvatar'
|
||||
@@ -8,17 +10,20 @@ import './Modal.css'
|
||||
|
||||
interface PeopleModalProps {
|
||||
onClose: () => void
|
||||
onOpenRoom: (roomId: string) => void
|
||||
}
|
||||
|
||||
// #25: a snapshot on open, not a live feed -- matches listOnlineUserIds'
|
||||
// own documented contract (also used as-is by the admin user list and the
|
||||
// room-invite search), rather than inventing a new live-updating design
|
||||
// for this first pass.
|
||||
export function PeopleModal({ onClose }: PeopleModalProps) {
|
||||
export function PeopleModal({ onClose, onOpenRoom }: PeopleModalProps) {
|
||||
const { user } = useAuth()
|
||||
const [users, setUsers] = useState<UserDirectoryEntry[]>([])
|
||||
const [onlineIds, setOnlineIds] = useState<Set<string>>(new Set())
|
||||
const [loading, setLoading] = useState(true)
|
||||
const [error, setError] = useState<string | null>(null)
|
||||
const [startingId, setStartingId] = useState<string | null>(null)
|
||||
|
||||
useEffect(() => {
|
||||
Promise.all([listUserDirectory(), listOnlineUserIds()])
|
||||
@@ -33,18 +38,34 @@ export function PeopleModal({ onClose }: PeopleModalProps) {
|
||||
// Online first (each group alphabetical, matching listUserDirectory's own
|
||||
// username ordering) -- who's actually around right now is the more
|
||||
// useful thing to see first in a list that can otherwise run to the
|
||||
// entire site's user base.
|
||||
// entire site's user base. Excludes the viewer themselves -- there's no
|
||||
// "DM yourself" affordance.
|
||||
const sorted = useMemo(
|
||||
() =>
|
||||
[...users].sort((a, b) => {
|
||||
[...users]
|
||||
.filter((u) => u.id !== user?.id)
|
||||
.sort((a, b) => {
|
||||
const aOnline = onlineIds.has(a.id)
|
||||
const bOnline = onlineIds.has(b.id)
|
||||
if (aOnline !== bOnline) return aOnline ? -1 : 1
|
||||
return a.username.localeCompare(b.username)
|
||||
}),
|
||||
[users, onlineIds],
|
||||
[users, onlineIds, user?.id],
|
||||
)
|
||||
|
||||
async function handleStartDm(otherUserId: string) {
|
||||
setStartingId(otherUserId)
|
||||
setError(null)
|
||||
try {
|
||||
const room = await startDm(otherUserId)
|
||||
onOpenRoom(room.id)
|
||||
onClose()
|
||||
} catch (err) {
|
||||
setError(err instanceof ApiError ? err.message : String(err))
|
||||
setStartingId(null)
|
||||
}
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="modal-scrim" onClick={onClose}>
|
||||
<div className="modal" onClick={(e) => e.stopPropagation()}>
|
||||
@@ -65,7 +86,13 @@ export function PeopleModal({ onClose }: PeopleModalProps) {
|
||||
sorted.map((u) => {
|
||||
const online = onlineIds.has(u.id)
|
||||
return (
|
||||
<div key={u.id} className="modal-list-row">
|
||||
<button
|
||||
key={u.id}
|
||||
type="button"
|
||||
className="modal-list-row modal-list-row-button"
|
||||
onClick={() => handleStartDm(u.id)}
|
||||
disabled={startingId !== null}
|
||||
>
|
||||
<UserAvatar
|
||||
username={u.username}
|
||||
colorIndex={hashIndex(u.username)}
|
||||
@@ -75,9 +102,11 @@ export function PeopleModal({ onClose }: PeopleModalProps) {
|
||||
/>
|
||||
<div className="modal-list-row-body">
|
||||
<div className="modal-list-row-title">{u.display_name || u.username}</div>
|
||||
<div className="modal-list-row-sub">{online ? 'Online' : 'Offline'}</div>
|
||||
<div className="modal-list-row-sub">
|
||||
{startingId === u.id ? 'Opening…' : online ? 'Online' : 'Offline'}
|
||||
</div>
|
||||
</div>
|
||||
</button>
|
||||
)
|
||||
})
|
||||
)}
|
||||
|
||||
@@ -23,6 +23,7 @@ import {
|
||||
} from '../api/webhooks'
|
||||
import { useAuth } from '../context/AuthContext'
|
||||
import { useResizableWidth } from '../hooks/useResizableWidth'
|
||||
import { hashIndex } from '../lib/avatar'
|
||||
import { MOBILE_BREAKPOINT, useWindowWidth } from '../hooks/useWindowWidth'
|
||||
import { formatFileSize } from '../lib/fileSize'
|
||||
import type {
|
||||
@@ -117,8 +118,11 @@ export function RoomInfoPanel({
|
||||
const canManage = myRole === 'admin' || myRole === 'owner'
|
||||
// #48: room owner, room admin, or site admin (regardless of their role in
|
||||
// *this* room) can edit room settings, including privacy -- matches the
|
||||
// backend PATCH /api/rooms/{id} gate exactly (see rooms.py).
|
||||
const canEditSettings = canManage || !!user?.is_site_admin
|
||||
// backend PATCH /api/rooms/{id} gate exactly (see rooms.py). #52: never
|
||||
// for a DM regardless of role -- mirrors update_room's own
|
||||
// CannotModifyDmError guard, since a DM's `name` is an internal token,
|
||||
// not something editable.
|
||||
const canEditSettings = !room.is_dm && (canManage || !!user?.is_site_admin)
|
||||
|
||||
useEffect(() => {
|
||||
setNameDraft(room.name)
|
||||
@@ -295,12 +299,32 @@ export function RoomInfoPanel({
|
||||
</div>
|
||||
|
||||
<div className="room-info-summary">
|
||||
{room.dm_partner ? (
|
||||
<>
|
||||
<UserAvatar
|
||||
username={room.dm_partner.username}
|
||||
colorIndex={hashIndex(room.dm_partner.username)}
|
||||
size={56}
|
||||
avatarUrl={
|
||||
room.dm_partner.avatar_filename
|
||||
? getUserAvatarUrl(room.dm_partner.user_id, room.dm_partner.avatar_filename)
|
||||
: null
|
||||
}
|
||||
status={room.dm_partner.status}
|
||||
/>
|
||||
<div className="room-info-name">{room.dm_partner.display_name || room.dm_partner.username}</div>
|
||||
<div className="room-info-sub">{room.dm_partner.status === 'online' ? 'Online' : 'Offline'}</div>
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<RoomAvatar colorIndex={0} size={56} />
|
||||
<div className="room-info-name">#{room.name}</div>
|
||||
<div className="room-info-sub">
|
||||
{members.length} member{members.length === 1 ? '' : 's'}
|
||||
{room.is_private && ' · Private'}
|
||||
</div>
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
|
||||
<div className="room-info-section">
|
||||
@@ -572,6 +596,7 @@ export function RoomInfoPanel({
|
||||
</div>
|
||||
)}
|
||||
|
||||
{!room.is_dm && (
|
||||
<button
|
||||
type="button"
|
||||
className="room-info-leave"
|
||||
@@ -581,6 +606,7 @@ export function RoomInfoPanel({
|
||||
>
|
||||
Leave room
|
||||
</button>
|
||||
)}
|
||||
|
||||
{lightboxSrc && <ImageLightbox src={lightboxSrc} onClose={() => setLightboxSrc(null)} />}
|
||||
{previewFile && (
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { Link } from 'react-router-dom'
|
||||
import { getUserAvatarUrl } from '../api/users'
|
||||
import { hashIndex } from '../lib/avatar'
|
||||
import type { MyRoomItem } from '../types'
|
||||
import { RoomAvatar } from './RoomAvatar'
|
||||
import { UserAvatar } from './UserAvatar'
|
||||
import './RoomRow.css'
|
||||
|
||||
interface RoomRowProps {
|
||||
@@ -10,13 +13,25 @@ interface RoomRowProps {
|
||||
}
|
||||
|
||||
export function RoomRow({ room, colorIndex, active }: RoomRowProps) {
|
||||
const partner = room.dm_partner
|
||||
|
||||
return (
|
||||
<Link to={`/rooms/${room.id}`} className={`room-row${active ? ' room-row-active' : ''}`}>
|
||||
{partner ? (
|
||||
<UserAvatar
|
||||
username={partner.username}
|
||||
colorIndex={hashIndex(partner.username)}
|
||||
size={34}
|
||||
avatarUrl={partner.avatar_filename ? getUserAvatarUrl(partner.user_id, partner.avatar_filename) : null}
|
||||
status={partner.status}
|
||||
/>
|
||||
) : (
|
||||
<RoomAvatar colorIndex={colorIndex} />
|
||||
)}
|
||||
<div className="room-row-body">
|
||||
<div className="room-row-name">
|
||||
{room.name}
|
||||
{room.is_private && (
|
||||
{partner ? partner.display_name || partner.username : room.name}
|
||||
{!partner && room.is_private && (
|
||||
<svg
|
||||
className="room-row-lock"
|
||||
width="12"
|
||||
@@ -30,7 +45,7 @@ export function RoomRow({ room, colorIndex, active }: RoomRowProps) {
|
||||
</svg>
|
||||
)}
|
||||
</div>
|
||||
{room.description && <div className="room-row-subtitle">{room.description}</div>}
|
||||
{!partner && room.description && <div className="room-row-subtitle">{room.description}</div>}
|
||||
</div>
|
||||
{!active && room.has_mention && (
|
||||
<span className="room-row-mention-dot" aria-label="You were mentioned" />
|
||||
|
||||
@@ -25,7 +25,21 @@ export function Sidebar({
|
||||
unavailableOffline,
|
||||
}: SidebarProps) {
|
||||
const query = searchQuery.trim().toLowerCase()
|
||||
const filtered = query ? rooms.filter((r) => r.name.toLowerCase().includes(query)) : rooms
|
||||
// A DM's `name` is an internal token, never what a user would search for
|
||||
// -- matched against the partner's display name/username instead.
|
||||
function matchesQuery(room: MyRoomItem): boolean {
|
||||
if (!query) return true
|
||||
if (room.is_dm && room.dm_partner) {
|
||||
return (
|
||||
(room.dm_partner.display_name ?? '').toLowerCase().includes(query) ||
|
||||
room.dm_partner.username.toLowerCase().includes(query)
|
||||
)
|
||||
}
|
||||
return room.name.toLowerCase().includes(query)
|
||||
}
|
||||
const filtered = rooms.filter(matchesQuery)
|
||||
const directMessages = filtered.filter((r) => r.is_dm)
|
||||
const regularRooms = filtered.filter((r) => !r.is_dm)
|
||||
|
||||
const { width, startResize } = useResizableWidth({
|
||||
storageKey: 'sidebar-width',
|
||||
@@ -83,9 +97,24 @@ export function Sidebar({
|
||||
</p>
|
||||
) : (
|
||||
<>
|
||||
{filtered.length > 0 && <div className="sidebar-section-label">Rooms</div>}
|
||||
{directMessages.length > 0 && (
|
||||
<>
|
||||
<div className="sidebar-section-label">Direct Messages</div>
|
||||
<nav>
|
||||
{filtered.map((room, i) => (
|
||||
{directMessages.map((room, i) => (
|
||||
<RoomRow
|
||||
key={room.id}
|
||||
room={room}
|
||||
colorIndex={i}
|
||||
active={room.id === activeRoomId}
|
||||
/>
|
||||
))}
|
||||
</nav>
|
||||
</>
|
||||
)}
|
||||
{regularRooms.length > 0 && <div className="sidebar-section-label">Rooms</div>}
|
||||
<nav>
|
||||
{regularRooms.map((room, i) => (
|
||||
<RoomRow
|
||||
key={room.id}
|
||||
room={room}
|
||||
|
||||
@@ -175,7 +175,15 @@ export function ChatShellPage() {
|
||||
}}
|
||||
/>
|
||||
)}
|
||||
{modal === 'people' && <PeopleModal onClose={() => setModal(null)} />}
|
||||
{modal === 'people' && (
|
||||
<PeopleModal
|
||||
onClose={() => setModal(null)}
|
||||
onOpenRoom={(id) => {
|
||||
setModal(null)
|
||||
refreshRooms().then(() => goToRoom(id))
|
||||
}}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -56,6 +56,7 @@ export interface Room {
|
||||
name: string
|
||||
description: string | null
|
||||
is_private: boolean
|
||||
is_dm: boolean
|
||||
owner_id: string
|
||||
created_at: string
|
||||
}
|
||||
@@ -64,12 +65,23 @@ export interface RoomListItem extends Room {
|
||||
is_member: boolean
|
||||
}
|
||||
|
||||
export interface DmPartnerInfo {
|
||||
user_id: string
|
||||
username: string
|
||||
display_name: string | null
|
||||
avatar_filename: string | null
|
||||
status: 'online' | 'offline'
|
||||
}
|
||||
|
||||
export interface MyRoomItem extends Room {
|
||||
role: RoomRole
|
||||
has_unread: boolean
|
||||
// Unread and mentions the current user -- takes visual priority over
|
||||
// has_unread in the sidebar (see RoomRow.tsx), not shown alongside it.
|
||||
has_mention: boolean
|
||||
// #52: the other participant, only for is_dm rooms -- see backend
|
||||
// schemas/room.py's MyRoomItem for why this is precomputed server-side.
|
||||
dm_partner: DmPartnerInfo | null
|
||||
}
|
||||
|
||||
export interface RoomMember {
|
||||
|
||||
Reference in New Issue
Block a user