Private
Public Access
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 <noreply@anthropic.com>
This commit is contained in:
@@ -156,6 +156,11 @@ async def update_room_endpoint(
|
|||||||
):
|
):
|
||||||
try:
|
try:
|
||||||
room = await get_room(db, room_id)
|
room = await get_room(db, room_id)
|
||||||
|
# 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)
|
await require_room_role(room_id, current_user, db, RoomRole.admin)
|
||||||
return await update_room(db, room, data)
|
return await update_room(db, room, data)
|
||||||
except RoomNotFoundError:
|
except RoomNotFoundError:
|
||||||
|
|||||||
@@ -16,6 +16,7 @@ class RoomCreate(BaseModel):
|
|||||||
class RoomUpdate(BaseModel):
|
class RoomUpdate(BaseModel):
|
||||||
name: str | None = Field(default=None, min_length=1, max_length=100)
|
name: str | None = Field(default=None, min_length=1, max_length=100)
|
||||||
description: str | None = Field(default=None, max_length=2000)
|
description: str | None = Field(default=None, max_length=2000)
|
||||||
|
is_private: bool | None = None
|
||||||
|
|
||||||
|
|
||||||
class RoomRead(BaseModel):
|
class RoomRead(BaseModel):
|
||||||
|
|||||||
@@ -190,6 +190,8 @@ async def update_room(db: AsyncSession, room: Room, data: RoomUpdate) -> Room:
|
|||||||
room.name = data.name
|
room.name = data.name
|
||||||
if data.description is not None:
|
if data.description is not None:
|
||||||
room.description = data.description
|
room.description = data.description
|
||||||
|
if data.is_private is not None:
|
||||||
|
room.is_private = data.is_private
|
||||||
try:
|
try:
|
||||||
await db.commit()
|
await db.commit()
|
||||||
except IntegrityError as exc:
|
except IntegrityError as exc:
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ import uuid
|
|||||||
|
|
||||||
from sqlalchemy import select
|
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
|
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"
|
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):
|
async def test_delete_room_owner_only(client, db_session):
|
||||||
await register_and_login(client, db_session, username="alice")
|
await register_and_login(client, db_session, username="alice")
|
||||||
room_id = (await client.post("/api/rooms", json={"name": "general"})).json()["id"]
|
room_id = (await client.post("/api/rooms", json={"name": "general"})).json()["id"]
|
||||||
|
|||||||
@@ -31,7 +31,7 @@ export function createRoom(
|
|||||||
|
|
||||||
export function updateRoom(
|
export function updateRoom(
|
||||||
roomId: string,
|
roomId: string,
|
||||||
data: { name?: string; description?: string },
|
data: { name?: string; description?: string; is_private?: boolean },
|
||||||
): Promise<Room> {
|
): Promise<Room> {
|
||||||
return apiFetch<Room>(`/api/rooms/${roomId}`, {
|
return apiFetch<Room>(`/api/rooms/${roomId}`, {
|
||||||
method: 'PATCH',
|
method: 'PATCH',
|
||||||
|
|||||||
@@ -42,6 +42,7 @@ import { FileAttachmentIcon } from './MessageList'
|
|||||||
import { RoomAvatar } from './RoomAvatar'
|
import { RoomAvatar } from './RoomAvatar'
|
||||||
import { UserAvatar } from './UserAvatar'
|
import { UserAvatar } from './UserAvatar'
|
||||||
import { UserPicker } from './UserPicker'
|
import { UserPicker } from './UserPicker'
|
||||||
|
import './Modal.css'
|
||||||
import './RoomInfoPanel.css'
|
import './RoomInfoPanel.css'
|
||||||
|
|
||||||
const EVENT_TYPES: EventType[] = ['message.created', 'message.updated']
|
const EVENT_TYPES: EventType[] = ['message.created', 'message.updated']
|
||||||
@@ -87,6 +88,7 @@ export function RoomInfoPanel({
|
|||||||
const [settingsOpen, setSettingsOpen] = useState(false)
|
const [settingsOpen, setSettingsOpen] = useState(false)
|
||||||
const [nameDraft, setNameDraft] = useState(room.name)
|
const [nameDraft, setNameDraft] = useState(room.name)
|
||||||
const [descDraft, setDescDraft] = useState(room.description ?? '')
|
const [descDraft, setDescDraft] = useState(room.description ?? '')
|
||||||
|
const [isPrivateDraft, setIsPrivateDraft] = useState(room.is_private)
|
||||||
const [roomError, setRoomError] = useState<string | null>(null)
|
const [roomError, setRoomError] = useState<string | null>(null)
|
||||||
|
|
||||||
const [integrationsOpen, setIntegrationsOpen] = useState(false)
|
const [integrationsOpen, setIntegrationsOpen] = useState(false)
|
||||||
@@ -99,10 +101,15 @@ export function RoomInfoPanel({
|
|||||||
const [integrationsError, setIntegrationsError] = useState<string | null>(null)
|
const [integrationsError, setIntegrationsError] = useState<string | null>(null)
|
||||||
|
|
||||||
const canManage = myRole === 'admin' || myRole === 'owner'
|
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(() => {
|
useEffect(() => {
|
||||||
setNameDraft(room.name)
|
setNameDraft(room.name)
|
||||||
setDescDraft(room.description ?? '')
|
setDescDraft(room.description ?? '')
|
||||||
|
setIsPrivateDraft(room.is_private)
|
||||||
if (canManage) {
|
if (canManage) {
|
||||||
listIncomingWebhooks(room.id).then(setIncomingWebhooks).catch(() => setIncomingWebhooks([]))
|
listIncomingWebhooks(room.id).then(setIncomingWebhooks).catch(() => setIncomingWebhooks([]))
|
||||||
listEventSubscriptions(room.id).then(setEventSubscriptions).catch(() => setEventSubscriptions([]))
|
listEventSubscriptions(room.id).then(setEventSubscriptions).catch(() => setEventSubscriptions([]))
|
||||||
@@ -112,7 +119,7 @@ export function RoomInfoPanel({
|
|||||||
setEventSubscriptions([])
|
setEventSubscriptions([])
|
||||||
setDirectoryUsers([])
|
setDirectoryUsers([])
|
||||||
}
|
}
|
||||||
}, [room.id, room.name, room.description, canManage])
|
}, [room.id, room.name, room.description, room.is_private, canManage])
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
// Fetched lazily (only once expanded), not alongside the section above
|
// Fetched lazily (only once expanded), not alongside the section above
|
||||||
@@ -246,7 +253,11 @@ export function RoomInfoPanel({
|
|||||||
e.preventDefault()
|
e.preventDefault()
|
||||||
setRoomError(null)
|
setRoomError(null)
|
||||||
try {
|
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()
|
onRoomUpdated()
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
setRoomError(err instanceof ApiError ? err.message : String(err))
|
setRoomError(err instanceof ApiError ? err.message : String(err))
|
||||||
@@ -500,7 +511,7 @@ export function RoomInfoPanel({
|
|||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
{myRole === 'owner' && (
|
{canEditSettings && (
|
||||||
<div className="room-info-section">
|
<div className="room-info-section">
|
||||||
<button
|
<button
|
||||||
type="button"
|
type="button"
|
||||||
@@ -519,13 +530,29 @@ export function RoomInfoPanel({
|
|||||||
Description
|
Description
|
||||||
<textarea value={descDraft} onChange={(e) => setDescDraft(e.target.value)} rows={2} />
|
<textarea value={descDraft} onChange={(e) => setDescDraft(e.target.value)} rows={2} />
|
||||||
</label>
|
</label>
|
||||||
|
<div className="toggle-row">
|
||||||
|
<div className="toggle-label">
|
||||||
|
<span className="t">Private room</span>
|
||||||
|
<span className="d">Joinable by invite only</span>
|
||||||
|
</div>
|
||||||
|
<label className="switch">
|
||||||
|
<input
|
||||||
|
type="checkbox"
|
||||||
|
checked={isPrivateDraft}
|
||||||
|
onChange={(e) => setIsPrivateDraft(e.target.checked)}
|
||||||
|
/>
|
||||||
|
<span className="track" />
|
||||||
|
</label>
|
||||||
|
</div>
|
||||||
{roomError && <p className="room-info-error">{roomError}</p>}
|
{roomError && <p className="room-info-error">{roomError}</p>}
|
||||||
<button type="submit" className="btn-secondary">
|
<button type="submit" className="btn-secondary">
|
||||||
Save
|
Save
|
||||||
</button>
|
</button>
|
||||||
|
{myRole === 'owner' && (
|
||||||
<button type="button" className="room-info-danger-link" onClick={handleDelete}>
|
<button type="button" className="room-info-danger-link" onClick={handleDelete}>
|
||||||
Delete room
|
Delete room
|
||||||
</button>
|
</button>
|
||||||
|
)}
|
||||||
</form>
|
</form>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
Reference in New Issue
Block a user