diff --git a/backend/app/routers/rooms.py b/backend/app/routers/rooms.py index a318f34..700d1f8 100644 --- a/backend/app/routers/rooms.py +++ b/backend/app/routers/rooms.py @@ -18,6 +18,7 @@ from app.schemas.message_file import MessageFileCreated from app.schemas.message_image import MessageImageCreated from app.schemas.room import ( MyRoomItem, + RoomAttachmentRead, RoomCreate, RoomListItem, RoomMemberAdd, @@ -35,7 +36,11 @@ from app.schemas.webhook import ( WebhookIncomingRead, ) from app.services.message_events import broadcast_room_added -from app.services.message_service import get_reactions_for_messages, list_recent_messages +from app.services.message_service import ( + get_reactions_for_messages, + list_recent_messages, + list_room_attachments, +) from app.services.upload_settings_service import format_mb, get_upload_settings from app.services.room_service import ( AlreadyMemberError, @@ -463,6 +468,50 @@ async def get_room_file_endpoint( ) +@router.get("/{room_id}/attachments", response_model=list[RoomAttachmentRead]) +async def list_room_attachments_endpoint( + room_id: uuid.UUID, + limit: int = Query(default=100, ge=1, le=200), + current_user: User = Depends(get_current_user), + db: AsyncSession = Depends(get_db), +): + await require_room_member(room_id, current_user, db) + messages = await list_room_attachments(db, room_id, limit) + attachments: list[RoomAttachmentRead] = [] + for m in messages: + # A message could in principle carry both an image and a file (the + # DB doesn't forbid it, even though the composer's UI only ever + # attaches one) -- emit an entry per attachment actually present + # rather than assuming exactly one. + if m.file: + attachments.append( + RoomAttachmentRead( + id=m.file.id, + kind="file", + filename=m.file.original_filename, + content_type=m.file.content_type, + size_bytes=m.file.size_bytes, + uploaded_by=m.user.username, + message_id=m.id, + created_at=m.created_at, + ) + ) + if m.image: + attachments.append( + RoomAttachmentRead( + id=m.image.id, + kind="image", + filename=None, + content_type=m.image.content_type, + size_bytes=m.image.size_bytes, + uploaded_by=m.user.username, + message_id=m.id, + created_at=m.created_at, + ) + ) + return attachments + + @router.post("/{room_id}/members", response_model=RoomMemberRead, status_code=201) async def add_member_endpoint( room_id: uuid.UUID, diff --git a/backend/app/schemas/room.py b/backend/app/schemas/room.py index 98a26d5..18b2837 100644 --- a/backend/app/schemas/room.py +++ b/backend/app/schemas/room.py @@ -60,3 +60,16 @@ class RoomMemberRoleUpdate(BaseModel): class TransferOwnershipRequest(BaseModel): new_owner_user_id: uuid.UUID + + +class RoomAttachmentRead(BaseModel): + id: uuid.UUID + kind: Literal["file", "image"] + # None for images -- MessageImage has no stored original filename, + # unlike MessageFile (see backend/app/models/message_image.py). + filename: str | None + content_type: str + size_bytes: int + uploaded_by: str + message_id: uuid.UUID + created_at: datetime diff --git a/backend/app/services/message_service.py b/backend/app/services/message_service.py index 13548e8..6f900e0 100644 --- a/backend/app/services/message_service.py +++ b/backend/app/services/message_service.py @@ -2,7 +2,7 @@ import uuid from collections import defaultdict from datetime import datetime, timezone -from sqlalchemy import select +from sqlalchemy import or_, select from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload @@ -66,6 +66,27 @@ async def list_recent_messages( return messages +async def list_room_attachments( + db: AsyncSession, room_id: uuid.UUID, limit: int = 100 +) -> list[Message]: + # Joins through messages.image_id/file_id rather than querying + # message_files/message_images directly -- a file/image is uploaded (and + # gets a row) *before* the message referencing it is ever sent, so an + # upload the user abandoned without sending would otherwise show up as + # a phantom attachment the room never actually saw. + result = await db.execute( + select(Message) + .where( + Message.room_id == room_id, + or_(Message.image_id.isnot(None), Message.file_id.isnot(None)), + ) + .options(selectinload(Message.user), selectinload(Message.file), selectinload(Message.image)) + .order_by(Message.created_at.desc()) + .limit(limit) + ) + return list(result.scalars().all()) + + async def get_reactions_for_messages( db: AsyncSession, message_ids: list[uuid.UUID] ) -> dict[uuid.UUID, list[ReactionSummary]]: diff --git a/backend/tests/test_attachments.py b/backend/tests/test_attachments.py new file mode 100644 index 0000000..01ff080 --- /dev/null +++ b/backend/tests/test_attachments.py @@ -0,0 +1,127 @@ +import io +import uuid + +from PIL import Image + +from tests.conftest import register_and_login + + +def _unique(prefix: str) -> str: + return f"{prefix}-{uuid.uuid4().hex[:8]}" + + +def _png_bytes(size: tuple[int, int] = (10, 10)) -> bytes: + buf = io.BytesIO() + Image.new("RGB", size, color=(255, 0, 0)).save(buf, format="PNG") + return buf.getvalue() + + +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 test_lists_files_and_images_actually_sent(ws_client): + alice = _register_ws(ws_client, _unique("alice")) + room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json() + + file_upload = ws_client.post( + f"/api/rooms/{room['id']}/files", + files={"file": ("notes.txt", b"hello", "text/plain")}, + ).json() + image_upload = ws_client.post( + f"/api/rooms/{room['id']}/images", + files={"file": ("pic.png", _png_bytes(), "image/png")}, + ).json() + + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + + ws.send_json({"type": "message", "room_id": room["id"], "file_id": file_upload["id"]}) + file_message = ws.receive_json() + + ws.send_json({"type": "message", "room_id": room["id"], "image_id": image_upload["id"]}) + image_message = ws.receive_json() + + resp = ws_client.get(f"/api/rooms/{room['id']}/attachments") + assert resp.status_code == 200, resp.text + attachments = resp.json() + + # Newest first. + assert [a["kind"] for a in attachments] == ["image", "file"] + + image_entry, file_entry = attachments + assert image_entry["id"] == image_upload["id"] + assert image_entry["filename"] is None + assert image_entry["content_type"] == "image/png" + assert image_entry["uploaded_by"] == alice["username"] + assert image_entry["message_id"] == image_message["id"] + + assert file_entry["id"] == file_upload["id"] + assert file_entry["filename"] == "notes.txt" + assert file_entry["content_type"] == "text/plain" + assert file_entry["size_bytes"] == len(b"hello") + assert file_entry["uploaded_by"] == alice["username"] + assert file_entry["message_id"] == file_message["id"] + + +async def test_abandoned_upload_never_sent_is_excluded(client, db_session): + # The scoping gotcha this feature exists to avoid: a file/image is + # uploaded (and gets a DB row) before the message referencing it is + # ever sent -- an upload nobody actually sent as a message must not + # show up as a phantom attachment. + await register_and_login(client, db_session, username=_unique("alice")) + room = (await client.post("/api/rooms", json={"name": _unique("general")})).json() + + await client.post( + f"/api/rooms/{room['id']}/files", + files={"file": ("never-sent.txt", b"abandoned", "text/plain")}, + ) + + resp = await client.get(f"/api/rooms/{room['id']}/attachments") + assert resp.status_code == 200 + assert resp.json() == [] + + +async def test_attachments_require_room_membership(client, db_session): + await register_and_login(client, db_session, username=_unique("alice")) + room = (await client.post("/api/rooms", json={"name": _unique("general")})).json() + + await register_and_login(client, db_session, username=_unique("bob")) + resp = await client.get(f"/api/rooms/{room['id']}/attachments") + assert resp.status_code == 403 + + +def test_attachments_scoped_to_room(ws_client): + _register_ws(ws_client, _unique("alice")) + room_a = ws_client.post("/api/rooms", json={"name": _unique("room-a")}).json() + room_b = ws_client.post("/api/rooms", json={"name": _unique("room-b")}).json() + + upload = ws_client.post( + f"/api/rooms/{room_a['id']}/files", + files={"file": ("notes.txt", b"hello", "text/plain")}, + ).json() + + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room_a["id"]}) + assert ws.receive_json()["type"] == "joined" + ws.send_json({"type": "message", "room_id": room_a["id"], "file_id": upload["id"]}) + assert ws.receive_json()["type"] == "message" + + assert len(ws_client.get(f"/api/rooms/{room_a['id']}/attachments").json()) == 1 + assert ws_client.get(f"/api/rooms/{room_b['id']}/attachments").json() == [] diff --git a/frontend/src/api/rooms.ts b/frontend/src/api/rooms.ts index 264c46f..cb0d2a5 100644 --- a/frontend/src/api/rooms.ts +++ b/frontend/src/api/rooms.ts @@ -4,6 +4,7 @@ import type { MessageFileInfo, MyRoomItem, Room, + RoomAttachment, RoomListItem, RoomMember, RoomRole, @@ -54,6 +55,10 @@ export function listRoomMembers(roomId: string): Promise { return apiFetch(`/api/rooms/${roomId}/members`) } +export function listRoomAttachments(roomId: string): Promise { + return apiFetch(`/api/rooms/${roomId}/attachments`) +} + export function addRoomMember(roomId: string, userId: string): Promise { return apiFetch(`/api/rooms/${roomId}/members`, { method: 'POST', diff --git a/frontend/src/components/Composer.tsx b/frontend/src/components/Composer.tsx index 0a1dd04..16ce668 100644 --- a/frontend/src/components/Composer.tsx +++ b/frontend/src/components/Composer.tsx @@ -2,6 +2,7 @@ import { useEffect, useRef, useState, type ChangeEvent, type KeyboardEvent } fro import { useOnlineStatus } from '../hooks/useOnlineStatus' import { uploadRoomFile, uploadRoomImage } from '../api/rooms' import { getUploadLimit } from '../api/uploads' +import { formatFileSize } from '../lib/fileSize' import { EmojiPicker } from './EmojiPicker' import './Composer.css' @@ -12,11 +13,6 @@ interface ComposerProps { onSend: (content: string, imageId?: string, fileId?: string) => void } -function formatFileSize(bytes: number): string { - if (bytes < 1024) return `${bytes} B` - if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB` - return `${(bytes / (1024 * 1024)).toFixed(1)} MB` -} export function Composer({ roomId, roomName, disabled, onSend }: ComposerProps) { const [value, setValue] = useState('') diff --git a/frontend/src/components/MessageList.tsx b/frontend/src/components/MessageList.tsx index 050b09a..d864726 100644 --- a/frontend/src/components/MessageList.tsx +++ b/frontend/src/components/MessageList.tsx @@ -1,6 +1,7 @@ import { useEffect, useRef, useState } from 'react' import { getRoomFileUrl, getRoomImageUrl } from '../api/rooms' import { useAuth } from '../context/AuthContext' +import { formatFileSize } from '../lib/fileSize' import { avatarUrlFor, displayNameFor, senderColorIndex, statusFor } from '../lib/messageGrouping' import type { ChatMessageEnvelope, Message, MessageFileInfo, RoomMember } from '../types' import { EMOJI_PICKER_MAX_HEIGHT, EmojiPicker } from './EmojiPicker' @@ -10,13 +11,7 @@ import { MessageContent } from './MessageContent' import { UserAvatar } from './UserAvatar' import './MessageList.css' -function formatFileSize(bytes: number): string { - if (bytes < 1024) return `${bytes} B` - if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB` - return `${(bytes / (1024 * 1024)).toFixed(1)} MB` -} - -function FileAttachmentIcon() { +export function FileAttachmentIcon() { return (
+ + {filesOpen && ( +
+ {attachmentsError &&

{attachmentsError}

} + {!attachmentsError && attachments.length === 0 && ( +

No files or images yet.

+ )} + {attachments.map((a) => { + const label = a.filename ?? 'Image' + const meta = `${formatFileSize(a.size_bytes)} · ${a.uploaded_by}` + const inner = ( + <> + + + {label} + {meta} + + + ) + const key = `${a.kind}-${a.id}` + + if (a.kind === 'image') { + return ( + + ) + } + if (getPreviewKind(a.filename ?? '')) { + return ( + + ) + } + return ( + + {inner} + + ) + })} +
+ )} +
+ {canManage && (
Add someone
@@ -444,6 +540,16 @@ export function RoomInfoPanel({ > Leave room + + {lightboxSrc && setLightboxSrc(null)} />} + {previewFile && ( + setPreviewFile(null)} + /> + )} ) } diff --git a/frontend/src/lib/fileSize.ts b/frontend/src/lib/fileSize.ts new file mode 100644 index 0000000..e55b7cc --- /dev/null +++ b/frontend/src/lib/fileSize.ts @@ -0,0 +1,5 @@ +export function formatFileSize(bytes: number): string { + if (bytes < 1024) return `${bytes} B` + if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB` + return `${(bytes / (1024 * 1024)).toFixed(1)} MB` +} diff --git a/frontend/src/types.ts b/frontend/src/types.ts index dd9e543..a639328 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -49,6 +49,17 @@ export interface RoomMember { status: 'online' | 'offline' } +export interface RoomAttachment { + id: string + kind: 'file' | 'image' + filename: string | null + content_type: string + size_bytes: number + uploaded_by: string + message_id: string + created_at: string +} + export type InviteStatus = 'pending' | 'accepted' | 'revoked' export interface ReactionSummary {