Private
Public Access
Make archiving a room actually affect existing members (#57)
is_archived was previously only exposed on the admin-only AdminRoom schema and checked in one place (excluding a room from Browse rooms) -- for anyone already a member it was a complete no-op: still in their sidebar, still fully postable, no indication anywhere it was archived. Expose is_archived on the regular RoomRead/MyRoomItem schemas, drop archived rooms from the sidebar list (while keeping them directly reachable via URL so history stays readable), and reject new messages in one -- both the WS "message" handler and incoming webhooks -- with a clear "archived and read-only" response instead of silently no-op'ing or a confusing membership error. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -147,6 +147,7 @@ async def list_rooms_endpoint(
|
||||
description=room.description,
|
||||
is_private=room.is_private,
|
||||
is_dm=room.is_dm,
|
||||
is_archived=room.is_archived,
|
||||
owner_id=room.owner_id,
|
||||
created_at=room.created_at,
|
||||
is_member=is_member,
|
||||
@@ -171,6 +172,7 @@ async def list_my_rooms_endpoint(
|
||||
description=room.description,
|
||||
is_private=room.is_private,
|
||||
is_dm=room.is_dm,
|
||||
is_archived=room.is_archived,
|
||||
owner_id=room.owner_id,
|
||||
created_at=room.created_at,
|
||||
role=role,
|
||||
|
||||
@@ -4,7 +4,7 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from app.database import get_db
|
||||
from app.schemas.webhook import IncomingWebhookPost
|
||||
from app.services.message_events import broadcast_new_message
|
||||
from app.services.webhook_service import WebhookNotFoundError, post_via_webhook
|
||||
from app.services.webhook_service import RoomArchivedError, WebhookNotFoundError, post_via_webhook
|
||||
|
||||
router = APIRouter(prefix="/api/webhooks", tags=["webhooks"])
|
||||
|
||||
@@ -22,6 +22,8 @@ async def incoming_webhook_endpoint(
|
||||
message, room, sender = await post_via_webhook(db, token, data.content)
|
||||
except WebhookNotFoundError:
|
||||
raise HTTPException(status_code=404, detail="Unknown webhook")
|
||||
except RoomArchivedError:
|
||||
raise HTTPException(status_code=403, detail="This room has been archived and is read-only")
|
||||
|
||||
broadcaster = request.app.state.broadcaster
|
||||
presence = request.app.state.presence
|
||||
|
||||
@@ -27,6 +27,11 @@ class RoomRead(BaseModel):
|
||||
description: str | None
|
||||
is_private: bool
|
||||
is_dm: bool
|
||||
# #57: previously only exposed on the admin-only AdminRoom schema, so a
|
||||
# member of an archived room had no way to even know it was archived --
|
||||
# the flag was set server-side but had no effect on their own view of
|
||||
# the room at all.
|
||||
is_archived: bool
|
||||
owner_id: uuid.UUID
|
||||
created_at: datetime
|
||||
|
||||
|
||||
@@ -27,6 +27,10 @@ class SubscriptionNotFoundError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
class RoomArchivedError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
async def create_incoming_webhook(
|
||||
db: AsyncSession, actor: User, room_id: uuid.UUID, description: str | None
|
||||
) -> WebhookIncoming:
|
||||
@@ -82,6 +86,11 @@ async def post_via_webhook(db: AsyncSession, token: str, content: str) -> tuple[
|
||||
webhook = result.scalar_one_or_none()
|
||||
if webhook is None:
|
||||
raise WebhookNotFoundError()
|
||||
# #57: same read-only rule as a human posting from the composer -- an
|
||||
# archived room shouldn't gain new messages through a bot integration
|
||||
# either.
|
||||
if webhook.room.is_archived:
|
||||
raise RoomArchivedError()
|
||||
|
||||
message = await create_message(db, webhook.room_id, webhook.created_by, content)
|
||||
return message, webhook.room, webhook.creator
|
||||
|
||||
+15
-1
@@ -6,7 +6,7 @@ from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.database import get_db
|
||||
from app.models import ApiToken, Message, MessageFile, MessageImage, RoomMembership, User
|
||||
from app.models import ApiToken, Message, MessageFile, MessageImage, Room, RoomMembership, User
|
||||
from app.services.bot_service import resolve_token
|
||||
from app.services.message_events import (
|
||||
broadcast_dm_presence_update,
|
||||
@@ -49,6 +49,11 @@ async def _is_room_member(db: AsyncSession, room_id: uuid.UUID, user_id: uuid.UU
|
||||
return result.scalar_one_or_none() is not None
|
||||
|
||||
|
||||
async def _is_room_archived(db: AsyncSession, room_id: uuid.UUID) -> bool:
|
||||
room = await db.get(Room, room_id)
|
||||
return room is not None and room.is_archived
|
||||
|
||||
|
||||
def _missing_scope(api_token: ApiToken | None, scope: str) -> bool:
|
||||
return api_token is not None and scope not in api_token.scopes
|
||||
|
||||
@@ -186,6 +191,15 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db)
|
||||
{"type": "error", "detail": "Not a member of this room"}
|
||||
)
|
||||
continue
|
||||
if await _is_room_archived(db, envelope.room_id):
|
||||
# #57: history stays fully readable (joining/reading
|
||||
# an archived room's channel is untouched above),
|
||||
# this is the one gate that actually makes archiving
|
||||
# do something for people who were already members.
|
||||
await websocket.send_json(
|
||||
{"type": "error", "detail": "This room has been archived and is read-only"}
|
||||
)
|
||||
continue
|
||||
image_id = None
|
||||
if envelope.image_id is not None:
|
||||
image = await db.get(MessageImage, envelope.image_id)
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
import uuid
|
||||
|
||||
from app.models import User
|
||||
from app.schemas.user import UserCreate
|
||||
from app.services.auth_service import register_user
|
||||
from tests.conftest import register_and_login
|
||||
|
||||
|
||||
def _unique(prefix: str) -> str:
|
||||
return f"{prefix}-{uuid.uuid4().hex[:8]}"
|
||||
|
||||
|
||||
async def _make_admin(db_session, user_id: str) -> None:
|
||||
user = await db_session.get(User, uuid.UUID(user_id))
|
||||
user.is_site_admin = True
|
||||
await db_session.commit()
|
||||
|
||||
|
||||
def _make_admin_ws(ws_client, user_id: str) -> None:
|
||||
async def _promote():
|
||||
async with ws_client.session_factory() as session:
|
||||
user = await session.get(User, uuid.UUID(user_id))
|
||||
user.is_site_admin = True
|
||||
await session.commit()
|
||||
|
||||
ws_client.portal.call(_promote)
|
||||
|
||||
|
||||
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()
|
||||
|
||||
|
||||
async def test_archived_room_flag_reaches_existing_members(client, db_session):
|
||||
# #57: is_archived used to only ever reach the admin portal's own
|
||||
# AdminRoom schema -- a member's own view of the room (GET /rooms/mine)
|
||||
# had no way to know it was archived at all.
|
||||
admin = await register_and_login(client, db_session, username=_unique("admin"))
|
||||
await _make_admin(db_session, admin["id"])
|
||||
room = (await client.post("/api/rooms", json={"name": _unique("general")})).json()
|
||||
|
||||
resp = await client.post(f"/api/admin/rooms/{room['id']}/archive")
|
||||
assert resp.status_code == 200
|
||||
|
||||
mine = (await client.get("/api/rooms/mine")).json()
|
||||
entry = next(r for r in mine if r["id"] == room["id"])
|
||||
assert entry["is_archived"] is True
|
||||
|
||||
# History stays fully readable for an existing member.
|
||||
messages_resp = await client.get(f"/api/rooms/{room['id']}/messages")
|
||||
assert messages_resp.status_code == 200
|
||||
|
||||
resp = await client.post(f"/api/admin/rooms/{room['id']}/unarchive")
|
||||
assert resp.status_code == 200
|
||||
mine = (await client.get("/api/rooms/mine")).json()
|
||||
entry = next(r for r in mine if r["id"] == room["id"])
|
||||
assert entry["is_archived"] is False
|
||||
|
||||
|
||||
def test_ws_message_rejected_in_archived_room(ws_client_factory, db_session):
|
||||
admin_ws = ws_client_factory()
|
||||
admin = _register_ws(admin_ws, _unique("admin"))
|
||||
_make_admin_ws(admin_ws, admin["id"])
|
||||
room = admin_ws.post("/api/rooms", json={"name": _unique("general")}).json()
|
||||
|
||||
archive_resp = admin_ws.post(f"/api/admin/rooms/{room['id']}/archive")
|
||||
assert archive_resp.status_code == 200
|
||||
|
||||
with admin_ws.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": "should not send"})
|
||||
received = ws.receive_json()
|
||||
assert received["type"] == "error"
|
||||
assert "archived" in received["detail"].lower()
|
||||
|
||||
|
||||
def test_ws_message_allowed_again_after_unarchive(ws_client_factory):
|
||||
admin_ws = ws_client_factory()
|
||||
admin = _register_ws(admin_ws, _unique("admin"))
|
||||
_make_admin_ws(admin_ws, admin["id"])
|
||||
room = admin_ws.post("/api/rooms", json={"name": _unique("general")}).json()
|
||||
|
||||
admin_ws.post(f"/api/admin/rooms/{room['id']}/archive")
|
||||
admin_ws.post(f"/api/admin/rooms/{room['id']}/unarchive")
|
||||
|
||||
with admin_ws.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": "back online"})
|
||||
received = ws.receive_json()
|
||||
assert received["type"] == "message"
|
||||
assert received["content"] == "back online"
|
||||
|
||||
|
||||
async def test_incoming_webhook_rejected_in_archived_room(client, db_session):
|
||||
admin = await register_and_login(client, db_session, username=_unique("admin"))
|
||||
await _make_admin(db_session, admin["id"])
|
||||
room = (await client.post("/api/rooms", json={"name": _unique("general")})).json()
|
||||
webhook = (await client.post(f"/api/rooms/{room['id']}/webhooks/incoming", json={})).json()
|
||||
|
||||
await client.post(f"/api/admin/rooms/{room['id']}/archive")
|
||||
|
||||
resp = await client.post(
|
||||
f"/api/webhooks/incoming/{webhook['token']}", json={"content": "should not post"}
|
||||
)
|
||||
assert resp.status_code == 403
|
||||
assert "archived" in resp.json()["detail"].lower()
|
||||
@@ -272,6 +272,7 @@ export function ChatPane({
|
||||
members={members}
|
||||
rooms={rooms}
|
||||
disabled={!connected}
|
||||
archived={room.is_archived}
|
||||
onSend={send}
|
||||
/>
|
||||
</section>
|
||||
|
||||
@@ -33,6 +33,12 @@ interface ComposerProps {
|
||||
// while typing and what actually renders as a link later agree.
|
||||
rooms: MyRoomItem[]
|
||||
disabled?: boolean
|
||||
// #57: an archived room is permanently read-only, not just transiently
|
||||
// disconnected -- kept as its own prop rather than folded into `disabled`
|
||||
// so the placeholder/status text can say why, instead of the connecting/
|
||||
// offline copy below (which would be actively misleading here: waiting
|
||||
// won't ever re-enable this).
|
||||
archived?: boolean
|
||||
onSend: (content: string, imageId?: string, fileId?: string) => void
|
||||
}
|
||||
|
||||
@@ -106,7 +112,12 @@ function AttachMenu({ onPickPhoto, onPickFile, onClose }: AttachMenuProps) {
|
||||
)
|
||||
}
|
||||
|
||||
export function Composer({ roomId, roomName, isDm, members, rooms, disabled, onSend }: ComposerProps) {
|
||||
export function Composer({ roomId, roomName, isDm, members, rooms, disabled, archived, onSend }: ComposerProps) {
|
||||
// Every gate below (attach/emoji buttons, textarea, send button) reads
|
||||
// this instead of the raw `disabled` prop -- an archived room must be
|
||||
// just as unwritable as a disconnected one, it just says why differently
|
||||
// (see the placeholder/status text further down).
|
||||
const isDisabled = disabled || archived
|
||||
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>(
|
||||
@@ -379,7 +390,7 @@ export function Composer({ roomId, roomName, isDm, members, rooms, disabled, onS
|
||||
|
||||
function handleDragEnter(e: DragEvent<HTMLDivElement>) {
|
||||
e.preventDefault()
|
||||
if (disabled) return
|
||||
if (isDisabled) return
|
||||
dragCounterRef.current++
|
||||
setDragActive(true)
|
||||
}
|
||||
@@ -401,7 +412,7 @@ export function Composer({ roomId, roomName, isDm, members, rooms, disabled, onS
|
||||
e.preventDefault()
|
||||
dragCounterRef.current = 0
|
||||
setDragActive(false)
|
||||
if (disabled) return
|
||||
if (isDisabled) return
|
||||
// Only the first dropped file, matching the existing single-attachment-
|
||||
// per-message limit (the button-triggered file input isn't `multiple`
|
||||
// either).
|
||||
@@ -503,7 +514,7 @@ export function Composer({ roomId, roomName, isDm, members, rooms, disabled, onS
|
||||
type="button"
|
||||
className="composer-attach"
|
||||
onClick={() => setAttachMenuOpen((v) => !v)}
|
||||
disabled={disabled || uploading}
|
||||
disabled={isDisabled || uploading}
|
||||
aria-label="Attach a photo or file"
|
||||
aria-expanded={attachMenuOpen}
|
||||
>
|
||||
@@ -542,7 +553,7 @@ export function Composer({ roomId, roomName, isDm, members, rooms, disabled, onS
|
||||
type="button"
|
||||
className="composer-emoji-trigger"
|
||||
onClick={() => setEmojiPickerOpen((v) => !v)}
|
||||
disabled={disabled}
|
||||
disabled={isDisabled}
|
||||
aria-label="Insert an emoji"
|
||||
>
|
||||
🙂
|
||||
@@ -561,7 +572,7 @@ export function Composer({ roomId, roomName, isDm, members, rooms, disabled, onS
|
||||
ref={textareaRef}
|
||||
rows={1}
|
||||
value={value}
|
||||
disabled={disabled}
|
||||
disabled={isDisabled}
|
||||
onChange={(e) => {
|
||||
setValue(e.target.value)
|
||||
autoGrow()
|
||||
@@ -576,11 +587,13 @@ export function Composer({ roomId, roomName, isDm, members, rooms, disabled, onS
|
||||
onSelect={handleSelectionChange}
|
||||
onKeyDown={handleKeyDown}
|
||||
placeholder={
|
||||
disabled
|
||||
? online
|
||||
? 'Connecting…'
|
||||
: "You're offline"
|
||||
: `Message ${isDm ? roomName : `#${roomName}`}`
|
||||
archived
|
||||
? 'This room has been archived'
|
||||
: disabled
|
||||
? online
|
||||
? 'Connecting…'
|
||||
: "You're offline"
|
||||
: `Message ${isDm ? roomName : `#${roomName}`}`
|
||||
}
|
||||
spellCheck
|
||||
/>
|
||||
@@ -613,7 +626,7 @@ export function Composer({ roomId, roomName, isDm, members, rooms, disabled, onS
|
||||
type="button"
|
||||
className="composer-send"
|
||||
onClick={handleSend}
|
||||
disabled={disabled || (!value.trim() && !pendingImage && !pendingFile)}
|
||||
disabled={isDisabled || (!value.trim() && !pendingImage && !pendingFile)}
|
||||
aria-label="Send message"
|
||||
>
|
||||
<svg width="15" height="15" viewBox="0 0 20 20" aria-hidden="true">
|
||||
@@ -621,8 +634,12 @@ export function Composer({ roomId, roomName, isDm, members, rooms, disabled, onS
|
||||
</svg>
|
||||
</button>
|
||||
</div>
|
||||
{disabled && (
|
||||
<div className="composer-status">{online ? 'Connecting…' : "You're offline — messages can't be sent right now"}</div>
|
||||
{archived ? (
|
||||
<div className="composer-status">This room has been archived and is read-only</div>
|
||||
) : (
|
||||
disabled && (
|
||||
<div className="composer-status">{online ? 'Connecting…' : "You're offline — messages can't be sent right now"}</div>
|
||||
)
|
||||
)}
|
||||
</div>
|
||||
)
|
||||
|
||||
@@ -37,7 +37,11 @@ export function Sidebar({
|
||||
}
|
||||
return room.name.toLowerCase().includes(query)
|
||||
}
|
||||
const filtered = rooms.filter(matchesQuery)
|
||||
// #57: archived rooms keep flowing through in `rooms` (so a member who
|
||||
// still has one open via a direct link resolves fine -- see ChatPane),
|
||||
// but they're a dead end going forward, so they don't belong in the list
|
||||
// you'd browse/search from.
|
||||
const filtered = rooms.filter((r) => !r.is_archived).filter(matchesQuery)
|
||||
const directMessages = filtered.filter((r) => r.is_dm)
|
||||
const regularRooms = filtered.filter((r) => !r.is_dm)
|
||||
|
||||
|
||||
@@ -57,6 +57,11 @@ export interface Room {
|
||||
description: string | null
|
||||
is_private: boolean
|
||||
is_dm: boolean
|
||||
// #57: exposed here (not just the admin-only AdminRoom) so a member who
|
||||
// still has this room -- direct link, or before their sidebar list next
|
||||
// refreshes -- can be shown it's read-only instead of just silently
|
||||
// failing to send.
|
||||
is_archived: boolean
|
||||
owner_id: string
|
||||
created_at: string
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user