diff --git a/apps/rooms/migrations/0017_drop_friendly_name.sql b/apps/rooms/migrations/0017_drop_friendly_name.sql new file mode 100644 index 0000000..67bf47a --- /dev/null +++ b/apps/rooms/migrations/0017_drop_friendly_name.sql @@ -0,0 +1,12 @@ +-- Retire `FriendlyName` from the room blob. +-- +-- The rename route (PUT /rooms/:id/name) briefly wrote `FriendlyName` alongside `Name`, +-- and every read defaulted it to `Name`. Neither exists any more: this server serves no +-- display name apart from the unique `Name`, and a stored value would otherwise survive in +-- the blob forever (`json_set` on rename is gone, so nothing would ever update it again). +-- +-- Strip the key from every room that carries one. `json_remove` on a blob without the key +-- is a no-op, so the WHERE only spares the rows that need no rewrite. +UPDATE room +SET data = json_remove(data, '$.FriendlyName') +WHERE json_type(data, '$.FriendlyName') IS NOT NULL; diff --git a/apps/rooms/src/openapi.ts b/apps/rooms/src/openapi.ts index 8e60e0f..10a8487 100644 --- a/apps/rooms/src/openapi.ts +++ b/apps/rooms/src/openapi.ts @@ -317,9 +317,6 @@ export const RoomDto = z.object({ .int() .nullable() .describe('The room’s published snapshot. Nothing takes snapshots here, so always null'), - FriendlyName: z - .string() - .describe('Display name. Nothing sets one apart from `Name` here, so it mirrors `Name`'), CCU: z .int() .nullable() diff --git a/apps/rooms/src/rooms.app.ts b/apps/rooms/src/rooms.app.ts index 77f5a00..8922568 100644 --- a/apps/rooms/src/rooms.app.ts +++ b/apps/rooms/src/rooms.app.ts @@ -1690,13 +1690,11 @@ const app = new Hono() }) } - // Writes `FriendlyName` too — see `setRoomName`. The two are the same string here, - // and the client labels the room from the display one. await setRoomName(c.env.DB, roomId, name) // The rename answers a bare `{ Success }` with no room in it, so the client has // nothing to re-render from and kept showing the old name until the push arrived. // Built from the room already in hand rather than re-read, like the image route's. - await pushRoomUpdate(c, accountId, { ...room, Name: name, FriendlyName: name }) + await pushRoomUpdate(c, accountId, { ...room, Name: name }) return roomResult(c, { Success: true }) } ) diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index b49ffdc..bea98a4 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -156,16 +156,16 @@ describe('rooms endpoints', () => { // None of these are stored — the seed blobs predate the keys — so they are defaulted on // read. The client's room DTO always carries them, and an ABSENT key is not the same as a - // zero/null one to its parser. `FriendlyName` is the one that can't be null: the client - // labels the room from it. - it('GET /rooms/:id carries BoostCount, CurrentSnapshotId, FriendlyName and CCU', async () => { + // zero/null one to its parser. `FriendlyName` is deliberately NOT among them: this server + // does not serve a display name apart from `Name`, and migration 0017 strips any stored one. + it('GET /rooms/:id carries BoostCount, CurrentSnapshotId and CCU, and no FriendlyName', async () => { const res = await SELF.fetch(`${ORIGIN}/rooms/1`) expect(res.status).toBe(200) const body = (await res.json()) as Record expect(body).toHaveProperty('BoostCount', 0) expect(body).toHaveProperty('CurrentSnapshotId', null) - expect(body).toHaveProperty('FriendlyName', body.Name) expect(body).toHaveProperty('CCU', null) + expect(body).not.toHaveProperty('FriendlyName') }) // Pinned whole: these are the numbers the client's publish UI counts against, and @@ -2999,13 +2999,12 @@ describe('rooms endpoints', () => { expect(await bodyOf(ok)).toMatchObject({ Success: true }) const room = (await (await SELF.fetch(`${ORIGIN}/rooms?name=RenamedCenter`)).json()) as { RoomId: number - FriendlyName: string + Name: string } expect(room.RoomId).toBe(2) - // The DISPLAY name follows the rename. It is otherwise only defaulted to `Name` on - // read, so a room that had ever stored one would keep labelling itself with the old - // name while every name-keyed lookup used the new one. - expect(room.FriendlyName).toBe('RenamedCenter') + expect(room.Name).toBe('RenamedCenter') + // A rename must not resurrect the retired display name. + expect(room).not.toHaveProperty('FriendlyName') }) /** The hub stub records every notifyPlayer call — see vitest.config.ts. */ @@ -3025,14 +3024,11 @@ describe('rooms endpoints', () => { const sent = await sentNotifications() expect(sent).toHaveLength(1) - // RoomUpdate, to the OWNER, carrying the room as it now stands — both names. + // RoomUpdate, to the OWNER, carrying the room as it now stands. expect(sent[0].playerId).toBe(1) expect(sent[0].notificationType).toBe(NotificationType.SubscriptionUpdateRoom) - expect(sent[0].data).toMatchObject({ - RoomId: 2, - Name: 'PushedRename', - FriendlyName: 'PushedRename', - }) + expect(sent[0].data).toMatchObject({ RoomId: 2, Name: 'PushedRename' }) + expect(sent[0].data).not.toHaveProperty('FriendlyName') // Put it back for the tests that read room 2 by name. await putForm('/rooms/2/name', { name: 'RenamedCenter' }, '1') diff --git a/packages/domain/src/rooms-db.ts b/packages/domain/src/rooms-db.ts index edf40b9..10cd012 100644 --- a/packages/domain/src/rooms-db.ts +++ b/packages/domain/src/rooms-db.ts @@ -467,23 +467,10 @@ export async function setRoomDescription( .run() } -/** - * Set a room's Name in place (the caller checks ownership + name uniqueness first). - * - * Writes `FriendlyName` to the same string. That is the DISPLAY name — what the client - * labels the room with — and it is only defaulted to `Name` on read - * ({@link attachRoomDtoDefaults}), with `??=`, so a room whose blob has ever carried one - * keeps it. Renaming without this leaves that room displaying its old name forever while - * every name-keyed lookup uses the new one. - * - * The reference lets a creator set a display name apart from the unique `Name`; nothing - * here exposes that, so the two are kept in step rather than allowed to diverge silently. - */ +/** Set a room's Name in place (the caller checks ownership + name uniqueness first). */ export async function setRoomName(db: D1Database, roomId: number, name: string): Promise { await db - .prepare( - "UPDATE room SET data = json_set(data, '$.Name', ?2, '$.FriendlyName', ?2) WHERE room_id = ?1" - ) + .prepare("UPDATE room SET data = json_set(data, '$.Name', ?2) WHERE room_id = ?1") .bind(roomId, name) .run() } @@ -1162,10 +1149,6 @@ const PUBLIC_WHERE = 'is_dorm IS NOT 1 AND accessibility = 1' * it is 0 for every room. * - `CurrentSnapshotId` — the room's published snapshot. Nothing takes snapshots, so it is * null, which is also what the reference serves for a room that has none. - * - `FriendlyName` — the display name, which the reference lets a creator set apart from - * the unique `Name`. Nothing sets one here, so it falls back to `Name`; it must never be - * null, because the client labels a room from it and renders nothing for a room without - * one. * - `CCU` — concurrent users. No live-population counter exists here, so it is null, which * is what the reference serves when it has no number rather than 0 (a 0 reads as "nobody * is in here" in the browse feeds). @@ -1176,7 +1159,6 @@ const PUBLIC_WHERE = 'is_dorm IS NOT 1 AND accessibility = 1' function attachRoomDtoDefaults(room: Room): void { room.BoostCount ??= 0 room.CurrentSnapshotId ??= null - room.FriendlyName ??= room.Name room.CCU ??= null }