From 84dc99d1a1ad2ae86abac5a78eed38cd54790b13 Mon Sep 17 00:00:00 2001 From: Keith Smith Date: Mon, 17 Aug 2026 18:25:33 -0600 Subject: [PATCH] Allow toggling room privacy after creation (#48) is_private was previously only settable at room creation. RoomUpdate now accepts it, update_room() applies it, and PATCH /api/rooms/{id} allows a site admin to make the change even for a room they haven't joined (in addition to the existing room owner/admin gate) -- require_room_role normally 403s a non-member before the role check ever runs, so this is a deliberate bypass for site admins specifically. Flipping the flag has no effect on existing members either direction (confirmed is_private is only ever checked at self-serve join time) -- it purely controls Browse Rooms visibility and future self-joins. Frontend: RoomInfoPanel's "Room settings" section is now visible to room owner, room admin, or site admin (was owner-only), with a privacy toggle reusing NewRoomModal's existing toggle-switch UI. "Delete room" stays owner-only, now nested inside that wider section rather than gating the whole thing. Co-Authored-By: Claude Sonnet 5 --- backend/app/routers/rooms.py | 7 ++- backend/app/schemas/room.py | 1 + backend/app/services/room_service.py | 2 + backend/tests/test_rooms.py | 58 ++++++++++++++++++++++- frontend/src/api/rooms.ts | 2 +- frontend/src/components/RoomInfoPanel.tsx | 39 ++++++++++++--- 6 files changed, 100 insertions(+), 9 deletions(-) diff --git a/backend/app/routers/rooms.py b/backend/app/routers/rooms.py index d32b3ec..3468898 100644 --- a/backend/app/routers/rooms.py +++ b/backend/app/routers/rooms.py @@ -156,7 +156,12 @@ async def update_room_endpoint( ): try: room = await get_room(db, room_id) - await require_room_role(room_id, current_user, db, RoomRole.admin) + # Room owner/admin (the pre-existing gate for name/description) or a + # site admin regardless of membership -- #48 explicitly wants site + # admins able to toggle is_private even for rooms they haven't + # joined, unlike require_room_role's normal membership requirement. + if not current_user.is_site_admin: + await require_room_role(room_id, current_user, db, RoomRole.admin) return await update_room(db, room, data) except RoomNotFoundError: raise HTTPException(status_code=404, detail="Room not found") diff --git a/backend/app/schemas/room.py b/backend/app/schemas/room.py index 16e504a..33e6335 100644 --- a/backend/app/schemas/room.py +++ b/backend/app/schemas/room.py @@ -16,6 +16,7 @@ class RoomCreate(BaseModel): class RoomUpdate(BaseModel): name: str | None = Field(default=None, min_length=1, max_length=100) description: str | None = Field(default=None, max_length=2000) + is_private: bool | None = None class RoomRead(BaseModel): diff --git a/backend/app/services/room_service.py b/backend/app/services/room_service.py index 84bc778..f70f2fb 100644 --- a/backend/app/services/room_service.py +++ b/backend/app/services/room_service.py @@ -190,6 +190,8 @@ async def update_room(db: AsyncSession, room: Room, data: RoomUpdate) -> Room: room.name = data.name if data.description is not None: room.description = data.description + if data.is_private is not None: + room.is_private = data.is_private try: await db.commit() except IntegrityError as exc: diff --git a/backend/tests/test_rooms.py b/backend/tests/test_rooms.py index 76117c6..b586484 100644 --- a/backend/tests/test_rooms.py +++ b/backend/tests/test_rooms.py @@ -2,7 +2,7 @@ import uuid from sqlalchemy import select -from app.models import Room, RoomMembership, RoomRole +from app.models import Room, RoomMembership, RoomRole, User from tests.conftest import login_as, register_and_login @@ -132,6 +132,62 @@ async def test_update_room_requires_admin(client, db_session): assert resp.json()["description"] == "updated" +async def test_update_room_is_private_toggles_open_room_visibility(client, db_session): + # #48: owner can flip an already-created room's privacy after the fact. + await register_and_login(client, db_session, username="alice") + room_id = (await client.post("/api/rooms", json={"name": "general"})).json()["id"] + assert "general" in {r["name"] for r in (await client.get("/api/rooms")).json()} + + resp = await client.patch(f"/api/rooms/{room_id}", json={"is_private": True}) + assert resp.status_code == 200 + assert resp.json()["is_private"] is True + assert "general" not in {r["name"] for r in (await client.get("/api/rooms")).json()} + + resp = await client.patch(f"/api/rooms/{room_id}", json={"is_private": False}) + assert resp.status_code == 200 + assert resp.json()["is_private"] is False + assert "general" in {r["name"] for r in (await client.get("/api/rooms")).json()} + + +async def test_update_room_is_private_allowed_for_room_admin_not_just_owner(client, db_session): + alice = await register_and_login(client, db_session, username="alice") + room_id = (await client.post("/api/rooms", json={"name": "general"})).json()["id"] + + await client.post("/api/auth/logout") + bob = await register_and_login(client, db_session, username="bob") + await client.post(f"/api/rooms/{room_id}/join") + + await client.post("/api/auth/logout") + await login_as(client, "alice") + resp = await client.patch( + f"/api/rooms/{room_id}/members/{bob['id']}", json={"role": "admin"} + ) + assert resp.status_code == 200 + + await client.post("/api/auth/logout") + await login_as(client, "bob") + resp = await client.patch(f"/api/rooms/{room_id}", json={"is_private": True}) + assert resp.status_code == 200 + assert resp.json()["is_private"] is True + + +async def test_update_room_is_private_allowed_for_site_admin_non_member(client, db_session): + await register_and_login(client, db_session, username="alice") + room_id = (await client.post("/api/rooms", json={"name": "general"})).json()["id"] + + await client.post("/api/auth/logout") + admin = await register_and_login(client, db_session, username="carol") + user = await db_session.get(User, uuid.UUID(admin["id"])) + user.is_site_admin = True + await db_session.commit() + + # Never joined "general" -- ordinarily require_room_role would 403 this + # as "Not a member of this room" before even checking role. + resp = await client.patch(f"/api/rooms/{room_id}", json={"is_private": True}) + assert resp.status_code == 200 + assert resp.json()["is_private"] is True + + async def test_delete_room_owner_only(client, db_session): await register_and_login(client, db_session, username="alice") room_id = (await client.post("/api/rooms", json={"name": "general"})).json()["id"] diff --git a/frontend/src/api/rooms.ts b/frontend/src/api/rooms.ts index deca755..52fa6d3 100644 --- a/frontend/src/api/rooms.ts +++ b/frontend/src/api/rooms.ts @@ -31,7 +31,7 @@ export function createRoom( export function updateRoom( roomId: string, - data: { name?: string; description?: string }, + data: { name?: string; description?: string; is_private?: boolean }, ): Promise { return apiFetch(`/api/rooms/${roomId}`, { method: 'PATCH', diff --git a/frontend/src/components/RoomInfoPanel.tsx b/frontend/src/components/RoomInfoPanel.tsx index 7c0bf99..e43bb6a 100644 --- a/frontend/src/components/RoomInfoPanel.tsx +++ b/frontend/src/components/RoomInfoPanel.tsx @@ -42,6 +42,7 @@ import { FileAttachmentIcon } from './MessageList' import { RoomAvatar } from './RoomAvatar' import { UserAvatar } from './UserAvatar' import { UserPicker } from './UserPicker' +import './Modal.css' import './RoomInfoPanel.css' const EVENT_TYPES: EventType[] = ['message.created', 'message.updated'] @@ -87,6 +88,7 @@ export function RoomInfoPanel({ const [settingsOpen, setSettingsOpen] = useState(false) const [nameDraft, setNameDraft] = useState(room.name) const [descDraft, setDescDraft] = useState(room.description ?? '') + const [isPrivateDraft, setIsPrivateDraft] = useState(room.is_private) const [roomError, setRoomError] = useState(null) const [integrationsOpen, setIntegrationsOpen] = useState(false) @@ -99,10 +101,15 @@ export function RoomInfoPanel({ const [integrationsError, setIntegrationsError] = useState(null) 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 useEffect(() => { setNameDraft(room.name) setDescDraft(room.description ?? '') + setIsPrivateDraft(room.is_private) if (canManage) { listIncomingWebhooks(room.id).then(setIncomingWebhooks).catch(() => setIncomingWebhooks([])) listEventSubscriptions(room.id).then(setEventSubscriptions).catch(() => setEventSubscriptions([])) @@ -112,7 +119,7 @@ export function RoomInfoPanel({ setEventSubscriptions([]) setDirectoryUsers([]) } - }, [room.id, room.name, room.description, canManage]) + }, [room.id, room.name, room.description, room.is_private, canManage]) useEffect(() => { // Fetched lazily (only once expanded), not alongside the section above @@ -246,7 +253,11 @@ export function RoomInfoPanel({ e.preventDefault() setRoomError(null) try { - await updateRoom(room.id, { name: nameDraft.trim(), description: descDraft.trim() }) + await updateRoom(room.id, { + name: nameDraft.trim(), + description: descDraft.trim(), + is_private: isPrivateDraft, + }) onRoomUpdated() } catch (err) { setRoomError(err instanceof ApiError ? err.message : String(err)) @@ -500,7 +511,7 @@ export function RoomInfoPanel({ )} - {myRole === 'owner' && ( + {canEditSettings && (
- + {myRole === 'owner' && ( + + )} )}