diff --git a/CLAUDE.md b/CLAUDE.md index cc2faa0..67321bd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -73,6 +73,16 @@ inconsistency here without checking the client first. - Endpoints the client re-renders from must return the updated entity, not `{ error, success, value: null }` — e.g. `clubs` `PUT /club/:id/clubhouse` left the old clubhouse on screen until it answered the full details envelope. +- Every subroom mutation (`rooms`: create, delete, `/subrooms/:sid/clone`, + `/subrooms/:sid/accessibility`) answers `{ success, error, value }` with the whole + updated ROOM — the client re-renders the room's subroom list from `value`. Notably + `value` is the room even for `clone`, whose product is a new SUBROOM; only the + room-level `POST /rooms/:id/clone` returns the thing it created. +- Accessibility is sent as the `RoomAccessibility` enum NAME on + `rooms` `PUT /rooms/:id/subrooms/:sid/accessibility` (`accessibility=Private`), not the + ordinal the room-level `/rooms/:id/accessibility` takes. The enum has five members + (Private, Public, Unlisted, Dev_only, Dev_Unlisted); parse via `parseAccessibility`, + which accepts either form. diff --git a/apps/rooms/src/openapi.ts b/apps/rooms/src/openapi.ts index b9c0ca2..1c3fbdc 100644 --- a/apps/rooms/src/openapi.ts +++ b/apps/rooms/src/openapi.ts @@ -156,7 +156,9 @@ export const SubRoomDto = z.object({ LastModeratedSaveModerationState: z.int(), IsSandbox: z.boolean(), MaxPlayers: z.int(), - Accessibility: z.int().describe('0 = Private, 1 = Public, 2 = Unlisted'), + Accessibility: z + .int() + .describe('0 Private, 1 Public, 2 Unlisted, 3 Dev_only, 4 Dev_Unlisted — set independently'), ShouldAutoStageSaves: z.boolean(), StagedSubRoomDataSaveId: z.int().nullable(), DataBlob: z.string().optional().describe('Uploaded scene-data key; absent until first save'), @@ -324,13 +326,6 @@ export const RoomEnvelope = z.object({ */ export const SubRoomSaveResult = z.union([SubRoomDto, RoomResultEnvelope]) -/** The same envelope carrying a subroom (`POST …/subrooms/{subRoomId}/clone`). */ -export const SubRoomEnvelope = z.object({ - success: z.boolean(), - error: z.string().describe('Empty on success'), - value: SubRoomDto.nullable(), -}) - /** The 401 the envelope-returning routes answer with — the only one that isn’t HTTP 200. */ export const UNAUTHORIZED_ENVELOPE = json( z.object({ success: z.literal(false), error: z.literal('Unauthorized'), value: z.null() }), @@ -406,6 +401,20 @@ export const AccessibilityRequest = z.object({ accessibility: z.string().describe('0 = Private, 1 = Public, 2 = Unlisted'), }) +/** + * `PUT /rooms/{roomId}/subrooms/{subRoomId}/accessibility`. Unlike the room-level route + * above, the client sends the enum NAME here (`accessibility=Private`), so both the name + * and the number are accepted. + */ +export const SubRoomAccessibilityRequest = z.object({ + accessibility: z + .string() + .describe( + 'A `RoomAccessibility` name — `Private`, `Public`, `Unlisted`, `Dev_only`, ' + + '`Dev_Unlisted` (case-insensitive) — or its ordinal 0–4' + ), +}) + /** `POST /rooms/{roomId}/subrooms`. */ export const CreateSubRoomRequest = z.object({ name: z.string().describe('The new subroom’s name'), @@ -414,7 +423,10 @@ export const CreateSubRoomRequest = z.object({ /** `PUT /rooms/{roomId}/subrooms/{subRoomId}/modify`. */ export const ModifySubRoomRequest = z.object({ name: z.string().describe('Required — an empty name is rejected'), - accessibility: z.string().optional().describe('0 = Private, 1 = Public, 2 = Unlisted'), + accessibility: z + .string() + .optional() + .describe('A `RoomAccessibility` name (case-insensitive) or its ordinal 0–4'), maxPlayers: z.string().optional().describe('Ignored when not a positive integer'), }) diff --git a/apps/rooms/src/rooms.app.ts b/apps/rooms/src/rooms.app.ts index c880bbe..8cee976 100644 --- a/apps/rooms/src/rooms.app.ts +++ b/apps/rooms/src/rooms.app.ts @@ -3,6 +3,7 @@ import { describeRoute, openAPIRouteHandler } from 'hono-openapi' import { useWorkersLogger } from 'workers-tagged-logger' import { + Accessibility, canManageRoom, cloneRoom, cloneSubRoom, @@ -74,8 +75,8 @@ import { SaveSubRoomDataRequest, ServiceStatus, stringQuery, + SubRoomAccessibilityRequest, SubRoomDto, - SubRoomEnvelope, subRoomIdParam, SubRoomSaveResult, SubRoomSavesPage, @@ -195,6 +196,22 @@ function unauthorized(c: Context) { return c.json({ error: 'Unauthorized' }, 401) } +/** + * Parse an `accessibility` form field into a `RoomAccessibility` value. The client + * sends the enum NAME on the subroom route (`accessibility=Private`), not the number + * the room-level route takes, so both forms are accepted. Returns undefined when the + * field is missing or names nothing in the enum. + */ +function parseAccessibility(value: unknown): number | undefined { + if (typeof value !== 'string') return undefined + const raw = value.trim() + if (/^-?\d+$/.test(raw)) return Number.parseInt(raw, 10) + const named = Object.entries(Accessibility).find( + ([name, ordinal]) => typeof ordinal === 'number' && name.toLowerCase() === raw.toLowerCase() + ) + return named ? (named[1] as number) : undefined +} + /** The notifications hub is a single global DO instance (see the `notify` worker). */ const HUB_INSTANCE = 'global' @@ -1607,16 +1624,14 @@ const app = new Hono() Error: 'You must enter a name for your room!', }) } - const accessibility = - typeof body.accessibility === 'string' - ? Number.parseInt(body.accessibility, 10) - : Number.NaN const maxPlayers = typeof body.maxPlayers === 'string' ? Number.parseInt(body.maxPlayers, 10) : Number.NaN const updated = await modifySubRoom(c.env.DB, roomId, subRoomId, { name, - accessibility: Number.isNaN(accessibility) ? undefined : accessibility, + // Accepts the enum name as well as the ordinal — the dedicated + // `/accessibility` route below is sent names, so this may be too. + accessibility: parseAccessibility(body.accessibility), maxPlayers: Number.isNaN(maxPlayers) || maxPlayers <= 0 ? undefined : maxPlayers, }) if (!updated) { @@ -1632,11 +1647,74 @@ const app = new Hono() } ) + // Set a single subroom's `Accessibility`. Same effect as the `accessibility` field of + // the subroom `modify` call, but this is what the client actually calls when the + // player flips one subroom's visibility, and the body carries the enum NAME + // (`accessibility=Private`), not the number the room-level `/accessibility` takes. + // Auth-gated (401) and owner-only, like the other subroom mutations. Answers the + // updated ROOM in the `{ success, error, value }` envelope — the client re-renders + // the room's subroom list from `value`, the same as subroom create/delete. + .put( + '/rooms/:roomId{[0-9]+}/subrooms/:subRoomId{[0-9]+}/accessibility', + describeRoute({ + tags: ['Subrooms'], + summary: 'Set a subroom’s accessibility', + description: [ + 'A subroom’s own visibility, independent of the room’s top-level `Accessibility`.', + 'The client sends the `RoomAccessibility` NAME here (`accessibility=Private`) rather', + 'than the ordinal the room-level route takes, so both forms are accepted; an', + 'unrecognised value is rejected. Owner-only — only the room’s creator may change', + 'its subrooms, not co-owners.', + '', + 'Answers the updated ROOM, not the bare subroom, so the client can re-render the', + 'room’s subroom list from `value`.', + ].join(' '), + security: AUTHED, + parameters: [roomIdParam, subRoomIdParam], + requestBody: form(SubRoomAccessibilityRequest, 'The new accessibility'), + responses: { + 200: json(RoomEnvelope, 'The updated room, or a rejection with `success: false`'), + 401: UNAUTHORIZED_ENVELOPE, + }, + }), + async (c) => { + const accountId = await authedAccountId(c) + if (accountId === null) { + return c.json({ success: false, error: 'Unauthorized', value: null }, 401) + } + + const roomId = Number.parseInt(c.req.param('roomId'), 10) + const subRoomId = Number.parseInt(c.req.param('subRoomId'), 10) + + const room = await getRoomById(c.env.DB, roomId) + if (!room) return roomEnvelope(c, null, 'This room does not exist!') + if (room.CreatorAccountId !== accountId) { + return roomEnvelope(c, null, 'You are not the owner of this room!') + } + if (!findSubRoom(room, subRoomId)) { + return roomEnvelope(c, null, 'This subroom does not exist!') + } + + const body = (await c.req.parseBody().catch(() => ({}))) as Record + const accessibility = parseAccessibility(body.accessibility) + if (accessibility === undefined) { + return roomEnvelope(c, null, 'You must provide a valid accessibility!') + } + + const updated = await modifySubRoom(c.env.DB, roomId, subRoomId, { accessibility }) + if (!updated) return roomEnvelope(c, null, 'This subroom does not exist!') + + await pushRoomUpdate(c, accountId, updated) + return roomEnvelope(c, updated) + } + ) + // Clone a subroom into a new subroom of the same room (fresh SubRoomId, same // scene/settings/data). Auth-gated (401) and owner-only. Notifies the owner and - // returns the `{ success, error, value }` envelope with the new subroom as `value`, - // mirroring the room-level `/clone`. Response shape is a best guess (the real - // client's expected body is unknown). + // returns the updated ROOM in the `{ success, error, value }` envelope — NOT the new + // subroom, even though the new subroom is what the call produces. The client + // re-renders the room's subroom list from `value`, the same as subroom + // create/delete/accessibility. .post( '/rooms/:roomId{[0-9]+}/subrooms/:subRoomId{[0-9]+}/clone', describeRoute({ @@ -1647,13 +1725,14 @@ const app = new Hono() 'data blobs, so it loads identical content — with a fresh globally-unique `SubRoomId`.', 'Owner-only.', '', - 'The response shape is a best guess: it mirrors the room-level `/clone` envelope, but', - 'the real client’s expected body for this call is unknown.', + 'Answers the updated ROOM, not the new subroom — the client re-renders the room’s', + 'subroom list from `value`. Unlike the room-level `/clone`, whose `value` IS the new', + 'room, the thing this call creates is not what comes back.', ].join('\n'), security: AUTHED, parameters: [roomIdParam, subRoomIdParam], responses: { - 200: json(SubRoomEnvelope, 'The new subroom, or a rejection with `success: false`'), + 200: json(RoomEnvelope, 'The updated room, or a rejection with `success: false`'), 401: UNAUTHORIZED_ENVELOPE, }, }), @@ -1676,7 +1755,7 @@ const app = new Hono() if (!result) return roomEnvelope(c, null, 'This subroom does not exist!') await pushRoomUpdate(c, accountId, result.room) - return roomEnvelope(c, result.subRoom) + return roomEnvelope(c, result.room) } ) diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index ba33b2d..25c3918 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -1334,17 +1334,69 @@ describe('rooms endpoints', () => { expect(sub).toMatchObject({ Name: 'My Cool Subroom', Accessibility: 1, MaxPlayers: 20 }) }) + it('PUT /rooms/:id/subrooms/:sid/accessibility takes the enum name the client sends', async () => { + const path = '/rooms/2/subrooms/2/accessibility' + const accessibilityOf = async () => + ( + (await (await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/2/data`)).json()) as { + Accessibility: number + } + ).Accessibility + + // No token → 401. + expect((await putForm(path, { accessibility: 'Private' })).status).toBe(401) + // Not the owner (room 2 is owned by account 1) → failure envelope. + expect(await envOf(await putForm(path, { accessibility: 'Private' }, '999'))).toMatchObject({ + success: false, + error: 'You are not the owner of this room!', + }) + // Unknown room / unknown subroom → failure envelope. + expect( + await envOf( + await putForm('/rooms/99999/subrooms/2/accessibility', { accessibility: '0' }, '1') + ) + ).toMatchObject({ success: false }) + expect( + await envOf( + await putForm('/rooms/2/subrooms/9999/accessibility', { accessibility: '0' }, '1') + ) + ).toMatchObject({ success: false }) + // A value that names nothing in the enum → rejected, not silently stored. + expect(await envOf(await putForm(path, { accessibility: 'Nonsense' }, '1'))).toMatchObject({ + success: false, + error: 'You must provide a valid accessibility!', + }) + + // The name form is what the live client sends. + const priv = await envOf(await putForm(path, { accessibility: 'Private' }, '1')) + expect(priv.success).toBe(true) + // The envelope carries the updated ROOM, so the client can re-render the subroom list. + expect(priv.value).toMatchObject({ RoomId: 2 }) + expect(await accessibilityOf()).toBe(0) + + // Case-insensitive, and the later enum members resolve too. + expect((await envOf(await putForm(path, { accessibility: 'dev_unlisted' }, '1'))).success).toBe( + true + ) + expect(await accessibilityOf()).toBe(4) + + // The ordinal still works. + expect((await envOf(await putForm(path, { accessibility: '1' }, '1'))).success).toBe(true) + expect(await accessibilityOf()).toBe(1) + }) + it('POST /rooms/:id/subrooms/:sid/clone is auth-gated, owner-only, and copies the subroom', async () => { const clone = async (roomId: number, subRoomId: number, sub?: string) => SELF.fetch(`${ORIGIN}/rooms/${roomId}/subrooms/${subRoomId}/clone`, { method: 'POST', headers: sub ? await bearer(sub) : {}, }) + type SubRoom = { SubRoomId: number; CreatorAccountId: number } const envelope = async (res: Response) => (await res.json()) as { success: boolean error: string - value: { SubRoomId: number; CreatorAccountId: number } | null + value: { RoomId: number; SubRooms: SubRoom[] } | null } // No token → 401. @@ -1354,17 +1406,28 @@ describe('rooms endpoints', () => { // Unknown subroom → success:false envelope. expect((await envelope(await clone(2, 9999, '1'))).success).toBe(false) - // Owner clones → success, a fresh SubRoomId owned by the caller, fetchable on the room. + const before = new Set( + ( + (await (await SELF.fetch(`${ORIGIN}/rooms/2`)).json()) as { SubRooms: SubRoom[] } + ).SubRooms.map((s) => s.SubRoomId) + ) + + // Owner clones → success. `value` is the updated ROOM, not the new subroom, so the + // clone shows up as one extra entry in its re-attached SubRooms list. const res = await clone(2, 2, '1') expect(res.status).toBe(200) const body = await envelope(res) expect(body.success).toBe(true) - expect(body.value?.SubRoomId).not.toBe(2) - expect(body.value?.CreatorAccountId).toBe(1) + expect(body.value?.RoomId).toBe(2) + const added = body.value!.SubRooms.filter((s) => !before.has(s.SubRoomId)) + expect(added).toHaveLength(1) + expect(added[0]!.CreatorAccountId).toBe(1) + // A fresh id, and fetchable as a subroom of the room. + expect(added[0]!.SubRoomId).not.toBe(2) const fetched = (await ( - await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/${body.value?.SubRoomId}/data`) + await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/${added[0]!.SubRoomId}/data`) ).json()) as { SubRoomId: number } - expect(fetched.SubRoomId).toBe(body.value?.SubRoomId) + expect(fetched.SubRoomId).toBe(added[0]!.SubRoomId) }) it('subroom clone mints a globally-unique SubRoomId (no cross-room clash)', async () => { @@ -1379,14 +1442,18 @@ describe('rooms endpoints', () => { method: 'POST', headers: await bearer('1'), }) - const body = (await res.json()) as { value: { SubRoomId: number; RoomId: number } } + // `value` is the updated room; the clone is its highest-numbered subroom. + const body = (await res.json()) as { + value: { RoomId: number; SubRooms: Array<{ SubRoomId: number }> } + } + const cloned = Math.max(...body.value.SubRooms.map((s) => s.SubRoomId)) // Above every prior subroom id — a fresh global id, not a per-room collision. - expect(body.value.SubRoomId).toBeGreaterThan(maxBefore) + expect(cloned).toBeGreaterThan(maxBefore) expect(body.value.RoomId).toBe(2) // The id is unique across the whole table (exactly one row owns it). const dupes = (await env.DB.prepare('SELECT COUNT(*) AS n FROM subroom WHERE sub_room_id = ?1') - .bind(body.value.SubRoomId) + .bind(cloned) .first<{ n: number }>())!.n expect(dupes).toBe(1) }) @@ -1560,6 +1627,7 @@ describe('rooms endpoints', () => { 'PUT /rooms/{roomId}/name', 'PUT /rooms/{roomId}/restrictions', 'PUT /rooms/{roomId}/roles/{accountId}', + 'PUT /rooms/{roomId}/subrooms/{subRoomId}/accessibility', 'PUT /rooms/{roomId}/subrooms/{subRoomId}/modify', 'PUT /rooms/{roomId}/tags', 'PUT /rooms/{roomId}/warning', diff --git a/packages/domain/src/enums.ts b/packages/domain/src/enums.ts index 6744ce6..b309d5d 100644 --- a/packages/domain/src/enums.ts +++ b/packages/domain/src/enums.ts @@ -64,11 +64,17 @@ export enum MessageType { VirtualRoomNotification = 100008, } -/** A room's (or image's) visibility, matching the client's `RoomAccessibility`. */ +/** + * A room's (or image's) visibility, matching the client's `RoomAccessibility`. The + * client declares the enum without explicit values, so these are its ordinals — and + * it sends the NAME, not the number, on the subroom accessibility route. + */ export enum Accessibility { Private = 0, Public = 1, Unlisted = 2, + Dev_only = 3, + Dev_Unlisted = 4, } /**