Private
Public Access
Add the ability to delete a message (#53)
Message.deleted_at has existed since the initial schema but was never wired up -- no WS envelope, no permission check, no frontend concept of it at all. Soft delete, author-only (mirrors the existing edit permission exactly): content and any attached image/file are cleared and the underlying MessageImage/MessageFile row and stored file are actually removed, not just detached, so the message becomes a "This message was deleted" tombstone with nothing left to recover through a stale attachment URL. A deleted message can no longer be edited or reacted to. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,39 @@
|
|||||||
|
"""allow deleted messages to clear content and attachments
|
||||||
|
|
||||||
|
Revision ID: e2fc4d65f93e
|
||||||
|
Revises: f3f255da9c96
|
||||||
|
Create Date: 2026-08-28 16:20:02.114630
|
||||||
|
|
||||||
|
"""
|
||||||
|
from typing import Sequence, Union
|
||||||
|
|
||||||
|
from alembic import op
|
||||||
|
import sqlalchemy as sa
|
||||||
|
|
||||||
|
|
||||||
|
# revision identifiers, used by Alembic.
|
||||||
|
revision: str = 'e2fc4d65f93e'
|
||||||
|
down_revision: Union[str, Sequence[str], None] = 'f3f255da9c96'
|
||||||
|
branch_labels: Union[str, Sequence[str], None] = None
|
||||||
|
depends_on: Union[str, Sequence[str], None] = None
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
"""Upgrade schema."""
|
||||||
|
op.drop_constraint('messages_content_or_attachment_required', 'messages', type_='check')
|
||||||
|
op.create_check_constraint(
|
||||||
|
'messages_content_or_attachment_required',
|
||||||
|
'messages',
|
||||||
|
'content IS NOT NULL OR image_id IS NOT NULL OR file_id IS NOT NULL '
|
||||||
|
'OR deleted_at IS NOT NULL',
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
"""Downgrade schema."""
|
||||||
|
op.drop_constraint('messages_content_or_attachment_required', 'messages', type_='check')
|
||||||
|
op.create_check_constraint(
|
||||||
|
'messages_content_or_attachment_required',
|
||||||
|
'messages',
|
||||||
|
'content IS NOT NULL OR image_id IS NOT NULL OR file_id IS NOT NULL',
|
||||||
|
)
|
||||||
@@ -10,8 +10,12 @@ from app.models.base import Base
|
|||||||
class Message(Base):
|
class Message(Base):
|
||||||
__tablename__ = "messages"
|
__tablename__ = "messages"
|
||||||
__table_args__ = (
|
__table_args__ = (
|
||||||
|
# #53: a deleted message clears content/image_id/file_id entirely
|
||||||
|
# (see message_service.delete_message) -- the "must have something"
|
||||||
|
# rule only applies while the message is actually live.
|
||||||
CheckConstraint(
|
CheckConstraint(
|
||||||
"content IS NOT NULL OR image_id IS NOT NULL OR file_id IS NOT NULL",
|
"content IS NOT NULL OR image_id IS NOT NULL OR file_id IS NOT NULL "
|
||||||
|
"OR deleted_at IS NOT NULL",
|
||||||
name="messages_content_or_attachment_required",
|
name="messages_content_or_attachment_required",
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -433,6 +433,7 @@ async def get_room_messages_endpoint(
|
|||||||
reactions=reactions_by_message.get(m.id, []),
|
reactions=reactions_by_message.get(m.id, []),
|
||||||
created_at=m.created_at,
|
created_at=m.created_at,
|
||||||
edited_at=m.edited_at,
|
edited_at=m.edited_at,
|
||||||
|
deleted_at=m.deleted_at,
|
||||||
)
|
)
|
||||||
for m in messages
|
for m in messages
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -44,3 +44,9 @@ class MessageRead(BaseModel):
|
|||||||
reactions: list[ReactionSummary]
|
reactions: list[ReactionSummary]
|
||||||
created_at: datetime
|
created_at: datetime
|
||||||
edited_at: datetime | None
|
edited_at: datetime | None
|
||||||
|
# #53: null for a live message; set once deleted, at which point
|
||||||
|
# content/image_id/file/link_preview are all already cleared
|
||||||
|
# server-side (see message_service.delete_message). `reactions` isn't
|
||||||
|
# cleared server-side -- the frontend just doesn't render them once
|
||||||
|
# deleted_at is set, same as it doesn't render the rest of a tombstone.
|
||||||
|
deleted_at: datetime | None
|
||||||
|
|||||||
@@ -134,6 +134,10 @@ async def _message_payload(db: AsyncSession, message: Message, username: str) ->
|
|||||||
"reactions": [],
|
"reactions": [],
|
||||||
"created_at": message.created_at.isoformat(),
|
"created_at": message.created_at.isoformat(),
|
||||||
"edited_at": message.edited_at.isoformat() if message.edited_at else None,
|
"edited_at": message.edited_at.isoformat() if message.edited_at else None,
|
||||||
|
# Always null here -- a message just being created can't already be
|
||||||
|
# deleted -- but included for wire-format parity with MessageRead
|
||||||
|
# and message_deleted (#53).
|
||||||
|
"deleted_at": None,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@@ -204,6 +208,16 @@ async def broadcast_message_update(
|
|||||||
_maybe_fetch_link_preview(broadcaster, room_id, message)
|
_maybe_fetch_link_preview(broadcaster, room_id, message)
|
||||||
|
|
||||||
|
|
||||||
|
async def broadcast_message_delete(broadcaster: Broadcaster, room_id: uuid.UUID, message_id: uuid.UUID) -> None:
|
||||||
|
# #53: no dispatch_event() call, deliberately -- same scope cut as
|
||||||
|
# broadcast_reaction_update's, and for the same reason (see
|
||||||
|
# backend/README.md): message.deleted isn't an outgoing-webhook event
|
||||||
|
# type here.
|
||||||
|
await broadcaster.publish(
|
||||||
|
room_id, {"type": "message_deleted", "id": str(message_id), "room_id": str(room_id)}
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
async def broadcast_reaction_update(
|
async def broadcast_reaction_update(
|
||||||
broadcaster: Broadcaster,
|
broadcaster: Broadcaster,
|
||||||
room_id: uuid.UUID,
|
room_id: uuid.UUID,
|
||||||
|
|||||||
@@ -6,11 +6,19 @@ from sqlalchemy import or_, select
|
|||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
from sqlalchemy.orm import selectinload
|
from sqlalchemy.orm import selectinload
|
||||||
|
|
||||||
from app.models import Message, MessageMention, MessageReaction, MessageRoomReference
|
from app.models import (
|
||||||
|
Message,
|
||||||
|
MessageFile,
|
||||||
|
MessageImage,
|
||||||
|
MessageMention,
|
||||||
|
MessageReaction,
|
||||||
|
MessageRoomReference,
|
||||||
|
)
|
||||||
from app.schemas.message import ReactionSummary
|
from app.schemas.message import ReactionSummary
|
||||||
from app.services.link_preview_service import extract_first_url
|
from app.services.link_preview_service import extract_first_url
|
||||||
from app.services.mention_service import extract_mentioned_user_ids
|
from app.services.mention_service import extract_mentioned_user_ids
|
||||||
from app.services.room_reference_service import extract_referenced_room_ids
|
from app.services.room_reference_service import extract_referenced_room_ids
|
||||||
|
from app.storage import delete_file
|
||||||
|
|
||||||
|
|
||||||
class MessageNotFoundError(Exception):
|
class MessageNotFoundError(Exception):
|
||||||
@@ -56,7 +64,9 @@ async def edit_message(
|
|||||||
db: AsyncSession, message_id: uuid.UUID, editor_id: uuid.UUID, content: str
|
db: AsyncSession, message_id: uuid.UUID, editor_id: uuid.UUID, content: str
|
||||||
) -> Message:
|
) -> Message:
|
||||||
message = await db.get(Message, message_id)
|
message = await db.get(Message, message_id)
|
||||||
if message is None:
|
# A deleted message might as well not exist for editing purposes --
|
||||||
|
# same MessageNotFoundError a genuinely missing id would raise.
|
||||||
|
if message is None or message.deleted_at is not None:
|
||||||
raise MessageNotFoundError()
|
raise MessageNotFoundError()
|
||||||
if message.user_id != editor_id:
|
if message.user_id != editor_id:
|
||||||
raise NotMessageAuthorError()
|
raise NotMessageAuthorError()
|
||||||
@@ -69,6 +79,50 @@ async def edit_message(
|
|||||||
return message
|
return message
|
||||||
|
|
||||||
|
|
||||||
|
async def delete_message(db: AsyncSession, message_id: uuid.UUID, deleter_id: uuid.UUID) -> Message:
|
||||||
|
message = await db.get(Message, message_id)
|
||||||
|
if message is None or message.deleted_at is not None:
|
||||||
|
raise MessageNotFoundError()
|
||||||
|
if message.user_id != deleter_id:
|
||||||
|
raise NotMessageAuthorError()
|
||||||
|
|
||||||
|
# Fetch the attachment's storage filename (if any) before clearing the
|
||||||
|
# message's own FK to it -- the file is only unlinked from disk after a
|
||||||
|
# successful commit below, mirroring delete_room's identical ordering:
|
||||||
|
# a rolled-back transaction should never leave us having destroyed
|
||||||
|
# something we couldn't get back.
|
||||||
|
image_filename: str | None = None
|
||||||
|
file_filename: str | None = None
|
||||||
|
if message.image_id is not None:
|
||||||
|
image = await db.get(MessageImage, message.image_id)
|
||||||
|
if image is not None:
|
||||||
|
image_filename = image.storage_filename
|
||||||
|
await db.delete(image)
|
||||||
|
if message.file_id is not None:
|
||||||
|
message_file = await db.get(MessageFile, message.file_id)
|
||||||
|
if message_file is not None:
|
||||||
|
file_filename = message_file.storage_filename
|
||||||
|
await db.delete(message_file)
|
||||||
|
|
||||||
|
# #53: a real delete, not just a UI hide -- content and any attachment
|
||||||
|
# are actually gone, not merely unlinked-but-still-fetchable. Only
|
||||||
|
# deleted_at (plus id/room_id/user_id/created_at, kept so the tombstone
|
||||||
|
# still occupies its place in history) survives.
|
||||||
|
message.content = None
|
||||||
|
message.image_id = None
|
||||||
|
message.file_id = None
|
||||||
|
message.preview_url = None
|
||||||
|
message.deleted_at = datetime.now(timezone.utc)
|
||||||
|
await db.commit()
|
||||||
|
await db.refresh(message)
|
||||||
|
|
||||||
|
for filename in (image_filename, file_filename):
|
||||||
|
if filename is not None:
|
||||||
|
delete_file(filename)
|
||||||
|
|
||||||
|
return message
|
||||||
|
|
||||||
|
|
||||||
async def list_recent_messages(
|
async def list_recent_messages(
|
||||||
db: AsyncSession, room_id: uuid.UUID, limit: int = 50
|
db: AsyncSession, room_id: uuid.UUID, limit: int = 50
|
||||||
) -> list[Message]:
|
) -> list[Message]:
|
||||||
|
|||||||
+37
-1
@@ -11,6 +11,7 @@ from app.services.bot_service import resolve_token
|
|||||||
from app.services.message_events import (
|
from app.services.message_events import (
|
||||||
broadcast_dm_presence_update,
|
broadcast_dm_presence_update,
|
||||||
broadcast_member_updated,
|
broadcast_member_updated,
|
||||||
|
broadcast_message_delete,
|
||||||
broadcast_message_update,
|
broadcast_message_update,
|
||||||
broadcast_new_message,
|
broadcast_new_message,
|
||||||
broadcast_reaction_update,
|
broadcast_reaction_update,
|
||||||
@@ -19,6 +20,7 @@ from app.services.message_service import (
|
|||||||
MessageNotFoundError,
|
MessageNotFoundError,
|
||||||
NotMessageAuthorError,
|
NotMessageAuthorError,
|
||||||
create_message,
|
create_message,
|
||||||
|
delete_message,
|
||||||
edit_message,
|
edit_message,
|
||||||
toggle_reaction,
|
toggle_reaction,
|
||||||
)
|
)
|
||||||
@@ -261,6 +263,36 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db)
|
|||||||
continue
|
continue
|
||||||
await broadcast_message_update(db, broadcaster, envelope.room_id, message)
|
await broadcast_message_update(db, broadcaster, envelope.room_id, message)
|
||||||
|
|
||||||
|
elif envelope.type == "delete":
|
||||||
|
if envelope.room_id is None or envelope.message_id is None:
|
||||||
|
await websocket.send_json(
|
||||||
|
{"type": "error", "detail": "room_id and message_id required"}
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
if _missing_scope(api_token, "write:messages"):
|
||||||
|
await websocket.send_json(
|
||||||
|
{"type": "error", "detail": "Token missing required scope: write:messages"}
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
if envelope.room_id not in joined_rooms or not await _is_room_member(
|
||||||
|
db, envelope.room_id, user.id
|
||||||
|
):
|
||||||
|
await websocket.send_json(
|
||||||
|
{"type": "error", "detail": "Not a member of this room"}
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
try:
|
||||||
|
await delete_message(db, envelope.message_id, user.id)
|
||||||
|
except MessageNotFoundError:
|
||||||
|
await websocket.send_json({"type": "error", "detail": "Message not found"})
|
||||||
|
continue
|
||||||
|
except NotMessageAuthorError:
|
||||||
|
await websocket.send_json(
|
||||||
|
{"type": "error", "detail": "You can only delete your own messages"}
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
await broadcast_message_delete(broadcaster, envelope.room_id, envelope.message_id)
|
||||||
|
|
||||||
elif envelope.type == "reaction":
|
elif envelope.type == "reaction":
|
||||||
if (
|
if (
|
||||||
envelope.room_id is None
|
envelope.room_id is None
|
||||||
@@ -285,7 +317,11 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db)
|
|||||||
)
|
)
|
||||||
continue
|
continue
|
||||||
target_message = await db.get(Message, envelope.message_id)
|
target_message = await db.get(Message, envelope.message_id)
|
||||||
if target_message is None or target_message.room_id != envelope.room_id:
|
if (
|
||||||
|
target_message is None
|
||||||
|
or target_message.room_id != envelope.room_id
|
||||||
|
or target_message.deleted_at is not None
|
||||||
|
):
|
||||||
await websocket.send_json({"type": "error", "detail": "Message not found"})
|
await websocket.send_json({"type": "error", "detail": "Message not found"})
|
||||||
continue
|
continue
|
||||||
reactions = await toggle_reaction(db, envelope.message_id, user.id, envelope.emoji)
|
reactions = await toggle_reaction(db, envelope.message_id, user.id, envelope.emoji)
|
||||||
|
|||||||
@@ -0,0 +1,222 @@
|
|||||||
|
import io
|
||||||
|
import uuid
|
||||||
|
|
||||||
|
from PIL import Image
|
||||||
|
|
||||||
|
from app.schemas.user import UserCreate
|
||||||
|
from app.services.auth_service import register_user
|
||||||
|
|
||||||
|
|
||||||
|
def _unique(prefix: str) -> str:
|
||||||
|
return f"{prefix}-{uuid.uuid4().hex[:8]}"
|
||||||
|
|
||||||
|
|
||||||
|
_NOISE_TYPES = {"member_updated", "desktop_notification", "unread_update"}
|
||||||
|
|
||||||
|
|
||||||
|
def _recv(ws) -> dict:
|
||||||
|
"""Reads the next frame, transparently discarding presence/offline-
|
||||||
|
notify noise -- see test_message_edit.py's identical helper."""
|
||||||
|
while True:
|
||||||
|
msg = ws.receive_json()
|
||||||
|
if msg.get("type") not in _NOISE_TYPES:
|
||||||
|
return msg
|
||||||
|
|
||||||
|
|
||||||
|
def _register_ws(ws_client, username: str) -> dict:
|
||||||
|
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 _png_bytes() -> bytes:
|
||||||
|
buf = io.BytesIO()
|
||||||
|
Image.new("RGB", (10, 10), color=(255, 0, 0)).save(buf, format="PNG")
|
||||||
|
return buf.getvalue()
|
||||||
|
|
||||||
|
|
||||||
|
def test_ws_delete_clears_content_and_broadcasts(ws_client):
|
||||||
|
username = _unique("alice")
|
||||||
|
_register_ws(ws_client, username=username)
|
||||||
|
room = ws_client.post("/api/rooms", json={"name": _unique("general")}).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"], "content": "hello"})
|
||||||
|
message = ws.receive_json()
|
||||||
|
|
||||||
|
ws.send_json({"type": "delete", "room_id": room["id"], "message_id": message["id"]})
|
||||||
|
deleted = ws.receive_json()
|
||||||
|
assert deleted == {"type": "message_deleted", "id": message["id"], "room_id": room["id"]}
|
||||||
|
|
||||||
|
resp = ws_client.get(f"/api/rooms/{room['id']}/messages")
|
||||||
|
history = resp.json()
|
||||||
|
tombstone = next(m for m in history if m["id"] == message["id"])
|
||||||
|
assert tombstone["content"] is None
|
||||||
|
assert tombstone["deleted_at"] is not None
|
||||||
|
assert tombstone["image_id"] is None
|
||||||
|
assert tombstone["file"] is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_ws_delete_rejects_non_author(ws_client):
|
||||||
|
alice = _register_ws(ws_client, username=_unique("alice"))
|
||||||
|
room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json()
|
||||||
|
|
||||||
|
bob = _register_ws(ws_client, username=_unique("bob"))
|
||||||
|
ws_client.post(f"/api/rooms/{room['id']}/join")
|
||||||
|
|
||||||
|
ws_client.post(
|
||||||
|
"/api/auth/login", json={"username_or_email": alice["username"], "password": "password123"}
|
||||||
|
)
|
||||||
|
with ws_client.websocket_connect("/ws/chat") as alice_ws:
|
||||||
|
alice_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||||
|
assert alice_ws.receive_json()["type"] == "joined"
|
||||||
|
alice_ws.send_json({"type": "message", "room_id": room["id"], "content": "hello"})
|
||||||
|
message = alice_ws.receive_json()
|
||||||
|
|
||||||
|
ws_client.post(
|
||||||
|
"/api/auth/login",
|
||||||
|
json={"username_or_email": bob["username"], "password": "password123"},
|
||||||
|
)
|
||||||
|
with ws_client.websocket_connect("/ws/chat") as bob_ws:
|
||||||
|
bob_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||||
|
assert _recv(bob_ws)["type"] == "joined"
|
||||||
|
bob_ws.send_json(
|
||||||
|
{"type": "delete", "room_id": room["id"], "message_id": message["id"]}
|
||||||
|
)
|
||||||
|
resp = bob_ws.receive_json()
|
||||||
|
assert resp["type"] == "error"
|
||||||
|
assert "own messages" in resp["detail"]
|
||||||
|
|
||||||
|
resp = ws_client.get(f"/api/rooms/{room['id']}/messages")
|
||||||
|
history = resp.json()
|
||||||
|
still_there = next(m for m in history if m["id"] == message["id"])
|
||||||
|
assert still_there["deleted_at"] is None
|
||||||
|
assert still_there["content"] == "hello"
|
||||||
|
|
||||||
|
|
||||||
|
def test_ws_delete_of_unknown_message_errors(ws_client):
|
||||||
|
_register_ws(ws_client, username=_unique("alice"))
|
||||||
|
room = ws_client.post("/api/rooms", json={"name": _unique("general")}).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": "delete", "room_id": room["id"], "message_id": str(uuid.uuid4())}
|
||||||
|
)
|
||||||
|
resp = ws.receive_json()
|
||||||
|
assert resp == {"type": "error", "detail": "Message not found"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_deleted_message_cannot_be_edited_or_reacted_to(ws_client):
|
||||||
|
_register_ws(ws_client, username=_unique("alice"))
|
||||||
|
room = ws_client.post("/api/rooms", json={"name": _unique("general")}).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"], "content": "hello"})
|
||||||
|
message = ws.receive_json()
|
||||||
|
|
||||||
|
ws.send_json({"type": "delete", "room_id": room["id"], "message_id": message["id"]})
|
||||||
|
assert ws.receive_json()["type"] == "message_deleted"
|
||||||
|
|
||||||
|
ws.send_json(
|
||||||
|
{
|
||||||
|
"type": "edit",
|
||||||
|
"room_id": room["id"],
|
||||||
|
"message_id": message["id"],
|
||||||
|
"content": "resurrected",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
assert ws.receive_json() == {"type": "error", "detail": "Message not found"}
|
||||||
|
|
||||||
|
ws.send_json(
|
||||||
|
{
|
||||||
|
"type": "reaction",
|
||||||
|
"room_id": room["id"],
|
||||||
|
"message_id": message["id"],
|
||||||
|
"emoji": "👍",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
assert ws.receive_json() == {"type": "error", "detail": "Message not found"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_fans_out_across_instances(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:
|
||||||
|
bob_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||||
|
assert _recv(bob_ws)["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"
|
||||||
|
alice_ws.send_json({"type": "message", "room_id": room["id"], "content": "hi"})
|
||||||
|
message = alice_ws.receive_json()
|
||||||
|
assert _recv(bob_ws)["type"] == "message"
|
||||||
|
|
||||||
|
alice_ws.send_json(
|
||||||
|
{"type": "delete", "room_id": room["id"], "message_id": message["id"]}
|
||||||
|
)
|
||||||
|
assert alice_ws.receive_json()["type"] == "message_deleted"
|
||||||
|
|
||||||
|
deleted = _recv(bob_ws)
|
||||||
|
assert deleted == {"type": "message_deleted", "id": message["id"], "room_id": room["id"]}
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_removes_underlying_image_from_disk(ws_client):
|
||||||
|
username = _unique("alice")
|
||||||
|
_register_ws(ws_client, username=username)
|
||||||
|
room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json()
|
||||||
|
|
||||||
|
upload = ws_client.post(
|
||||||
|
f"/api/rooms/{room['id']}/images",
|
||||||
|
files={"file": ("test.png", _png_bytes(), "image/png")},
|
||||||
|
).json()
|
||||||
|
image_id = upload["id"]
|
||||||
|
|
||||||
|
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"], "image_id": image_id})
|
||||||
|
message = ws.receive_json()
|
||||||
|
assert message["image_id"] == image_id
|
||||||
|
|
||||||
|
# Confirm the image actually serves before deleting, so a false
|
||||||
|
# pass (it was never reachable to begin with) can't hide as a true
|
||||||
|
# one below.
|
||||||
|
get_resp = ws_client.get(f"/api/rooms/{room['id']}/images/{image_id}")
|
||||||
|
assert get_resp.status_code == 200
|
||||||
|
|
||||||
|
ws.send_json({"type": "delete", "room_id": room["id"], "message_id": message["id"]})
|
||||||
|
assert ws.receive_json()["type"] == "message_deleted"
|
||||||
|
|
||||||
|
# The image is gone -- both the DB row (via the now-404ing serve
|
||||||
|
# endpoint) and, per #53's "delete the file too" choice, the file
|
||||||
|
# actually unlinked from disk (not just detached and orphaned).
|
||||||
|
get_resp = ws_client.get(f"/api/rooms/{room['id']}/images/{image_id}")
|
||||||
|
assert get_resp.status_code == 404
|
||||||
@@ -171,6 +171,21 @@ export function ChatPane({
|
|||||||
setLive((prev) =>
|
setLive((prev) =>
|
||||||
prev.map((m) => (m.id === envelope.id ? { ...m, reactions: envelope.reactions } : m)),
|
prev.map((m) => (m.id === envelope.id ? { ...m, reactions: envelope.reactions } : m)),
|
||||||
)
|
)
|
||||||
|
} else if (envelope.type === 'message_deleted' && envelope.room_id === room.id) {
|
||||||
|
// Mirrors what the server already did to the row (see
|
||||||
|
// message_service.delete_message) -- content/image/file/preview
|
||||||
|
// cleared, deleted_at set. `reactions` is left alone; MessageList
|
||||||
|
// just doesn't render it once deleted_at is set, same as it
|
||||||
|
// doesn't render anything else here.
|
||||||
|
const tombstone = {
|
||||||
|
content: null,
|
||||||
|
image_id: null,
|
||||||
|
file: null,
|
||||||
|
link_preview: null,
|
||||||
|
deleted_at: new Date().toISOString(),
|
||||||
|
}
|
||||||
|
setHistory((prev) => prev.map((m) => (m.id === envelope.id ? { ...m, ...tombstone } : m)))
|
||||||
|
setLive((prev) => prev.map((m) => (m.id === envelope.id ? { ...m, ...tombstone } : m)))
|
||||||
} else if (envelope.type === 'error') {
|
} else if (envelope.type === 'error') {
|
||||||
setWsError(envelope.detail)
|
setWsError(envelope.detail)
|
||||||
}
|
}
|
||||||
@@ -212,6 +227,10 @@ export function ChatPane({
|
|||||||
(messageId: string, emoji: string) => socket.sendReaction(room.id, messageId, emoji),
|
(messageId: string, emoji: string) => socket.sendReaction(room.id, messageId, emoji),
|
||||||
[socket, room.id],
|
[socket, room.id],
|
||||||
)
|
)
|
||||||
|
const sendDelete = useCallback(
|
||||||
|
(messageId: string) => socket.sendDelete(room.id, messageId),
|
||||||
|
[socket, room.id],
|
||||||
|
)
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<section className="chat-pane">
|
<section className="chat-pane">
|
||||||
@@ -264,6 +283,7 @@ export function ChatPane({
|
|||||||
myRooms={myRooms}
|
myRooms={myRooms}
|
||||||
onEdit={sendEdit}
|
onEdit={sendEdit}
|
||||||
onReact={sendReaction}
|
onReact={sendReaction}
|
||||||
|
onDelete={sendDelete}
|
||||||
/>
|
/>
|
||||||
<Composer
|
<Composer
|
||||||
roomId={room.id}
|
roomId={room.id}
|
||||||
|
|||||||
@@ -304,6 +304,25 @@
|
|||||||
border-color: var(--ds-accent);
|
border-color: var(--ds-accent);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.message-delete-link {
|
||||||
|
background: var(--ds-surface-2);
|
||||||
|
border: 1px solid var(--ds-border);
|
||||||
|
color: var(--ds-muted);
|
||||||
|
font-size: 0.68rem;
|
||||||
|
cursor: pointer;
|
||||||
|
padding: 2px 8px;
|
||||||
|
border-radius: 6px;
|
||||||
|
}
|
||||||
|
|
||||||
|
.message-delete-link:hover {
|
||||||
|
color: var(--ds-danger);
|
||||||
|
border-color: var(--ds-danger);
|
||||||
|
}
|
||||||
|
|
||||||
|
.message-deleted-text {
|
||||||
|
color: var(--ds-muted);
|
||||||
|
}
|
||||||
|
|
||||||
.message-reaction-wrap {
|
.message-reaction-wrap {
|
||||||
position: relative;
|
position: relative;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -67,9 +67,18 @@ interface MessageListProps {
|
|||||||
myRooms: Map<string, string>
|
myRooms: Map<string, string>
|
||||||
onEdit: (messageId: string, content: string) => void
|
onEdit: (messageId: string, content: string) => void
|
||||||
onReact: (messageId: string, emoji: string) => void
|
onReact: (messageId: string, emoji: string) => void
|
||||||
|
onDelete: (messageId: string) => void
|
||||||
}
|
}
|
||||||
|
|
||||||
export function MessageList({ roomId, messages, members, myRooms, onEdit, onReact }: MessageListProps) {
|
export function MessageList({
|
||||||
|
roomId,
|
||||||
|
messages,
|
||||||
|
members,
|
||||||
|
myRooms,
|
||||||
|
onEdit,
|
||||||
|
onReact,
|
||||||
|
onDelete,
|
||||||
|
}: MessageListProps) {
|
||||||
const { user } = useAuth()
|
const { user } = useAuth()
|
||||||
const containerRef = useRef<HTMLDivElement>(null)
|
const containerRef = useRef<HTMLDivElement>(null)
|
||||||
const bottomRef = useRef<HTMLDivElement>(null)
|
const bottomRef = useRef<HTMLDivElement>(null)
|
||||||
@@ -133,6 +142,14 @@ export function MessageList({ roomId, messages, members, myRooms, onEdit, onReac
|
|||||||
setEditingId(null)
|
setEditingId(null)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function handleDelete(messageId: string) {
|
||||||
|
// Matches the confirm() pattern already used for other destructive
|
||||||
|
// actions in this app (RoomInfoPanel's leave/delete-room,
|
||||||
|
// ProfileModal's delete-theme) rather than a custom dialog.
|
||||||
|
if (!confirm("Delete this message? This can't be undone.")) return
|
||||||
|
onDelete(messageId)
|
||||||
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="message-list" ref={containerRef}>
|
<div className="message-list" ref={containerRef}>
|
||||||
{messages.map((msg, i) => {
|
{messages.map((msg, i) => {
|
||||||
@@ -144,6 +161,7 @@ export function MessageList({ roomId, messages, members, myRooms, onEdit, onReac
|
|||||||
// applies uniformly, including to your own messages.
|
// applies uniformly, including to your own messages.
|
||||||
const isGroupStart = !prev || prev.user_id !== msg.user_id
|
const isGroupStart = !prev || prev.user_id !== msg.user_id
|
||||||
const editing = editingId === msg.id
|
const editing = editingId === msg.id
|
||||||
|
const deleted = !!msg.deleted_at
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div key={msg.id} className={`message-row${isGroupStart ? ' message-row-start' : ''}`}>
|
<div key={msg.id} className={`message-row${isGroupStart ? ' message-row-start' : ''}`}>
|
||||||
@@ -166,7 +184,11 @@ export function MessageList({ roomId, messages, members, myRooms, onEdit, onReac
|
|||||||
</span>
|
</span>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
{editing ? (
|
{deleted ? (
|
||||||
|
<div className="message-text message-deleted-text">
|
||||||
|
<em>This message was deleted</em>
|
||||||
|
</div>
|
||||||
|
) : editing ? (
|
||||||
<textarea
|
<textarea
|
||||||
autoFocus
|
autoFocus
|
||||||
rows={Math.min(10, draft.split('\n').length)}
|
rows={Math.min(10, draft.split('\n').length)}
|
||||||
@@ -231,7 +253,7 @@ export function MessageList({ roomId, messages, members, myRooms, onEdit, onReac
|
|||||||
</>
|
</>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
{!editing && (
|
{!editing && !deleted && (
|
||||||
<div className="message-row-actions">
|
<div className="message-row-actions">
|
||||||
<div className="message-reaction-wrap">
|
<div className="message-reaction-wrap">
|
||||||
<button
|
<button
|
||||||
@@ -277,6 +299,16 @@ export function MessageList({ roomId, messages, members, myRooms, onEdit, onReac
|
|||||||
Edit
|
Edit
|
||||||
</button>
|
</button>
|
||||||
)}
|
)}
|
||||||
|
{mine && (
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
className="message-delete-link"
|
||||||
|
onClick={() => handleDelete(msg.id)}
|
||||||
|
aria-label="Delete message"
|
||||||
|
>
|
||||||
|
Delete
|
||||||
|
</button>
|
||||||
|
)}
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -148,6 +148,11 @@ export interface Message {
|
|||||||
reactions: ReactionSummary[]
|
reactions: ReactionSummary[]
|
||||||
created_at: string
|
created_at: string
|
||||||
edited_at: string | null
|
edited_at: string | null
|
||||||
|
// #53: null for a live message. content/image_id/file/link_preview are
|
||||||
|
// already cleared server-side once this is set -- MessageList renders a
|
||||||
|
// tombstone off this alone rather than inferring deletion from the rest
|
||||||
|
// being empty.
|
||||||
|
deleted_at: string | null
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface ChatMessageEnvelope {
|
export interface ChatMessageEnvelope {
|
||||||
@@ -163,6 +168,10 @@ export interface ChatMessageEnvelope {
|
|||||||
reactions: ReactionSummary[]
|
reactions: ReactionSummary[]
|
||||||
created_at: string
|
created_at: string
|
||||||
edited_at: string | null
|
edited_at: string | null
|
||||||
|
// Always null here -- a just-sent message can't already be deleted --
|
||||||
|
// but declared so MessageList can read msg.deleted_at uniformly across
|
||||||
|
// the Message | ChatMessageEnvelope union, same as edited_at above.
|
||||||
|
deleted_at: string | null
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface ChatMessageUpdateEnvelope {
|
export interface ChatMessageUpdateEnvelope {
|
||||||
@@ -196,6 +205,12 @@ export interface ChatReactionUpdateEnvelope {
|
|||||||
reactions: ReactionSummary[]
|
reactions: ReactionSummary[]
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export interface ChatMessageDeletedEnvelope {
|
||||||
|
type: 'message_deleted'
|
||||||
|
id: string
|
||||||
|
room_id: string
|
||||||
|
}
|
||||||
|
|
||||||
export interface ChatJoinedEnvelope {
|
export interface ChatJoinedEnvelope {
|
||||||
type: 'joined'
|
type: 'joined'
|
||||||
room_id: string
|
room_id: string
|
||||||
@@ -252,6 +267,7 @@ export type ServerEnvelope =
|
|||||||
| ChatMessageEnvelope
|
| ChatMessageEnvelope
|
||||||
| ChatMessageUpdateEnvelope
|
| ChatMessageUpdateEnvelope
|
||||||
| ChatReactionUpdateEnvelope
|
| ChatReactionUpdateEnvelope
|
||||||
|
| ChatMessageDeletedEnvelope
|
||||||
| ChatLinkPreviewEnvelope
|
| ChatLinkPreviewEnvelope
|
||||||
| ChatJoinedEnvelope
|
| ChatJoinedEnvelope
|
||||||
| ChatErrorEnvelope
|
| ChatErrorEnvelope
|
||||||
|
|||||||
@@ -258,7 +258,13 @@ export function useChatSocket({ onUnauthenticated }: UseChatSocketOptions) {
|
|||||||
ws.send(JSON.stringify({ type: 'reaction', room_id: roomId, message_id: messageId, emoji }))
|
ws.send(JSON.stringify({ type: 'reaction', room_id: roomId, message_id: messageId, emoji }))
|
||||||
}, [])
|
}, [])
|
||||||
|
|
||||||
return { connected, subscribe, joinRoom, leaveRoom, send, sendEdit, sendReaction }
|
const sendDelete = useCallback((roomId: string, messageId: string) => {
|
||||||
|
const ws = socketRef.current
|
||||||
|
if (!ws || ws.readyState !== WebSocket.OPEN) return
|
||||||
|
ws.send(JSON.stringify({ type: 'delete', room_id: roomId, message_id: messageId }))
|
||||||
|
}, [])
|
||||||
|
|
||||||
|
return { connected, subscribe, joinRoom, leaveRoom, send, sendEdit, sendReaction, sendDelete }
|
||||||
}
|
}
|
||||||
|
|
||||||
export type ChatSocketHandle = ReturnType<typeof useChatSocket>
|
export type ChatSocketHandle = ReturnType<typeof useChatSocket>
|
||||||
|
|||||||
Reference in New Issue
Block a user