From cdb17f5284db7e0adf857abd3638f3ce7e192939 Mon Sep 17 00:00:00 2001 From: Devin Zuczek Date: Wed, 9 Sep 2026 14:12:07 -0400 Subject: [PATCH] [api] fix missing playerevents endpoint --- apps/api/src/events-db.ts | 26 ++++--- apps/api/src/http.ts | 26 +++++-- apps/api/src/openapi.ts | 32 ++++++--- apps/api/src/routes/events.ts | 88 ++++++++++++++++++----- apps/api/src/test/integration/api.test.ts | 83 +++++++++++++++++++-- 5 files changed, 210 insertions(+), 45 deletions(-) diff --git a/apps/api/src/events-db.ts b/apps/api/src/events-db.ts index a09887d..05381c2 100644 --- a/apps/api/src/events-db.ts +++ b/apps/api/src/events-db.ts @@ -290,9 +290,13 @@ export interface PlayerEventNotification { } /** - * The client's BASE event — the 17-key shape the browse feed (`GET /api/playerevents/v1`) - * serves, and the same thing the v2 envelope carries once `Tags` is added. PascalCase like - * the stored record, but not identical to it — don't unify them: + * The client's BASE event — the 17-key shape the browse feed (`GET /api/playerevents/v1`), + * the room shelf (`.../room/{roomId}`) and the bulk read (`POST|GET .../bulk`) all serve, + * and the same thing the v2 envelope carries once `Tags` is added. Those three are one + * generic helper over one element type on the client side, so they are shape-identical by + * construction there; `toEventBase` is what holds that here. + * + * PascalCase like the stored record, but not identical to it — don't unify them: * * - it drops `State`, which neither the feed nor the envelope carries; * - it carries `BroadcastingRoomInstanceId`, which the record has no field for (nothing @@ -300,8 +304,8 @@ export interface PlayerEventNotification { * - its `ImageName` is a string: an event with no image reads `""`, where the record holds * null. * - * The by-id / bulk / search reads serve the stored RECORD verbatim instead, `State` and - * nullable `ImageName` included. Two shapes; keep them apart. + * The by-id, search, searchlive and club reads serve the stored RECORD verbatim instead, + * `State` and nullable `ImageName` included. Two shapes; keep them apart. */ export interface PlayerEventBase extends Omit { ImageName: string @@ -856,9 +860,12 @@ export async function getEventById(db: D1Database, eventId: number): Promise { if (ids.length === 0) return [] @@ -907,7 +914,8 @@ export async function getEventsByClubs(db: D1Database, clubIds: number[]): Promi /** * A room's events — what is happening in this room and what is coming up, soonest first. - * Backs the room's event shelf (`GET /api/playerevents/v1/room/{roomId}`). + * Backs the room's event shelf (`GET /api/playerevents/v1/room/{roomId}`), which serves + * them through `toEventBase` like the browse feed and the bulk read. * * FINISHED events are left out, like the browse feed's: this answers "what can I still turn * up to in this room", and an event that ended last month is not that. Running events count diff --git a/apps/api/src/http.ts b/apps/api/src/http.ts index 8c9ac24..cee663a 100644 --- a/apps/api/src/http.ts +++ b/apps/api/src/http.ts @@ -27,13 +27,27 @@ export function unauthorized(c: Context) { return c.body(null, 401) } -/** Reads the `Ids` form field into a list of integer ids. */ +/** + * Reads the `Ids` form field of a bulk POST into a list of integer ids. + * + * BOTH spellings, because the client uses both: `Ids` REPEATED once per id + * (`Ids=101&Ids=102&Ids=103`, what the player-events bulk sends) and a single + * comma-separated `Ids=1,2,3`. `parseBody({ all: true })` is what keeps the repeated form + * from collapsing to its last value — plain `parseBody()` would answer one id out of + * three, which reads as a short result rather than as an error. + * + * `ids` is accepted alongside `Ids` so a hand-written request doesn't silently come back + * empty. Values that aren't integers are dropped; duplicates and order are left alone, + * since the caller renders them in request order. + */ export async function parseFormIds(c: Context): Promise { - const body = await c.req.parseBody().catch(() => ({}) as Record) - const ids = body.Ids - if (typeof ids !== 'string') return [] - return ids - .split(',') + const body = await c.req + .parseBody({ all: true }) + .catch(() => ({}) as Record) + const raw = [body.Ids, body.ids].flat() + return raw + .filter((v): v is string => typeof v === 'string') + .flatMap((v) => v.split(',')) .map((s) => Number.parseInt(s.trim(), 10)) .filter((n) => !Number.isNaN(n)) } diff --git a/apps/api/src/openapi.ts b/apps/api/src/openapi.ts index 500cf61..2d7eca6 100644 --- a/apps/api/src/openapi.ts +++ b/apps/api/src/openapi.ts @@ -400,9 +400,15 @@ export const CustomAvatarItemResponse = z.object({ error_id: z.string().nullable(), }) -/** The `Ids` form body the bulk POST endpoints take. */ +/** + * The `Ids` form body the bulk POST endpoints take, in either of the two spellings the + * client sends: `Ids` REPEATED once per id (`Ids=101&Ids=102&Ids=103`) or a single + * comma-separated `Ids=1,2,3`. Both are read by `parseFormIds`. + */ export const BulkIdsRequest = z.object({ - Ids: z.string().describe('Comma-separated account ids, e.g. `1,2,3`'), + Ids: z + .union([z.string(), z.array(z.string())]) + .describe('Repeated (`Ids=101&Ids=102`) or comma-separated (`Ids=1,2,3`)'), }) // ---- Inventions ------------------------------------------------------------ @@ -1013,13 +1019,23 @@ export const PlayerEventDetailsDto = PlayerEventDto.extend({ }) /** - * The client's BASE event, 17 keys — what `GET /api/playerevents/v1` serves, and what the - * v2 envelope carries once `Tags` is added. The stored record minus `State`, with - * `ImageName` as a string (`""`, not null) and a `BroadcastingRoomInstanceId` (always null — - * nothing broadcasts an event yet). + * The client's BASE event, 17 keys — the stored record minus `State`, with `ImageName` as a + * string (`""`, not null) and a `BroadcastingRoomInstanceId` (always null — nothing + * broadcasts an event yet). It is also what the v2 envelope carries once `Tags` is added. * - * The by-id, bulk and search reads serve the stored RECORD verbatim instead, so don't unify - * the two. + * THREE reads serve exactly this, through one generic helper on the client and one element + * type: the browse feed (`GET /api/playerevents/v1`), the room shelf + * (`GET /api/playerevents/v1/room/{roomId}`) and the bulk read + * (`POST|GET /api/playerevents/v1/bulk`). They are shape-identical by construction on the + * client side; keep them that way here. + * + * The remaining reads — by id, search, searchlive and the club feeds — serve the stored + * RECORD verbatim, `State` and nullable `ImageName` included. Two projections; don't unify + * them. + * + * `DefaultBroadcastPermissions` and `CanRequestBroadcastPermissions` are the client's + * broadcast-permission enum, whose members are NOT 0/1/2: None 0, RoomOwners 256, All + * 2147483647. Reading them as an ordinal is the classic way to break broadcast. */ export const PlayerEventBaseDto = PlayerEventDto.omit({ State: true, ImageName: true }).extend({ ImageName: z.string().describe('Empty string when the event has no image, never null'), diff --git a/apps/api/src/routes/events.ts b/apps/api/src/routes/events.ts index 914ad2c..fb4225a 100644 --- a/apps/api/src/routes/events.ts +++ b/apps/api/src/routes/events.ts @@ -35,9 +35,10 @@ import { toEventResult, updateEvent, } from '../events-db' -import { authedId, queryIds, unauthorized } from '../http' +import { authedId, parseFormIds, queryIds, unauthorized } from '../http' import { AUTHED, + BulkIdsRequest, form, idParam, intQuery, @@ -383,9 +384,12 @@ export const eventRoutes = new Hono({ strict: false }) ) // A room's event shelf (`/room/12`) — what is on in this room, current and upcoming. - // A bare array of the stored record, like the multi-club shelf and `/searchlive`: the - // single-club form's `{ ContinuationToken, Events }` envelope is the odd one out, and a - // room's shelf is small enough that there is nothing to page. + // A bare array, no envelope: the single-club form's `{ ContinuationToken, Events }` is + // the odd one out, and a room's shelf is small enough that there is nothing to page. + // + // The BASE event, not the stored record. The client reads this through the same generic + // helper and the same element type as the browse feed and the bulk read, so those three + // are shape-identical on its side — `toEventBase` is what keeps them identical here. .get( '/api/playerevents/v1/room/:roomId{[0-9]+}', describeRoute({ @@ -393,17 +397,25 @@ export const eventRoutes = new Hono({ strict: false }) summary: 'Player events in one room', description: 'The events scheduled in a room — the shelf on the room’s page — soonest first. A ' + - 'bare array of the stored record, the same projection `/searchlive` and the ' + - 'multi-club shelf serve.\n\n' + + 'bare array of the client’s BASE event (17 keys — no `State`, `ImageName` as `""` ' + + 'rather than null, plus `BroadcastingRoomInstanceId`), the same projection the ' + + 'browse feed and the bulk read serve: the client decodes all three through one ' + + 'generic helper and one element type. `/searchlive` and the club shelves serve the ' + + 'stored record instead.\n\n' + 'CURRENT and UPCOMING only: the filter is on the END time, so a running event stays ' + 'listed until it is over rather than vanishing the moment it starts, and an event ' + 'that has finished is dropped — this answers what someone can still turn up to. A ' + 'room with nothing scheduled, and a room id that does not exist, both answer an ' + 'empty array; the shelf is about events, not about whether the room is real.', parameters: [idParam('roomId', 'Room id')], - responses: { 200: json(PlayerEventDto.array(), 'The room’s current and upcoming events') }, + responses: { + 200: json(PlayerEventBaseDto.array(), 'The room’s current and upcoming events'), + }, }), - async (c) => c.json(await getEventsByRoom(c.env.DB, Number.parseInt(c.req.param('roomId'), 10))) + async (c) => { + const events = await getEventsByRoom(c.env.DB, Number.parseInt(c.req.param('roomId'), 10)) + return c.json(events.map(toEventBase)) + } ) // Live player-event search (the "happening now" browse query) — events that have @@ -457,22 +469,62 @@ export const eventRoutes = new Hono({ strict: false }) } ) - // Bulk fetch (`?id=1&id=2`) — the events behind a list of ids the client already - // holds. Answers in the order asked for; ids with no event are skipped. - .get( + // Bulk fetch — the events behind a list of ids the client already holds. What the + // client actually calls is the POST, with the ids in a form body + // (`Ids=101&Ids=102&Ids=103`, or `Ids=13` for one); the GET below is the same read with + // the ids in the query, kept for hand-written calls. + // + // The BASE event, like the browse feed and the room shelf: one generic helper and one + // element type decode all three on the client, so this is "the feed, filtered to these + // ids" and must not drift into the stored-record shape the by-id read serves. + // + // Answers in the order asked for — the client renders them in request order — and skips + // ids with no event rather than leaving a hole, so the result may be shorter than the + // request. A bare array either way: no envelope, no `{ ContinuationToken, Events }`. + .post( '/api/playerevents/v1/bulk', describeRoute({ tags: ['Events'], summary: 'Several player events by id', description: - 'The events behind a list of ids the client already holds (`?id=1&id=2`). Answers ' + - 'in the order the ids were asked for — the client renders them in request order — ' + - 'and skips ids with no event rather than leaving a hole, so the result may be ' + - 'shorter than the request. A bare array.', - parameters: [intQuery('id', 'Repeatable event id')], - responses: { 200: json(PlayerEventDto.array(), 'The events that exist, in request order') }, + 'The events behind a list of ids the client already holds, as a form body: `Ids` ' + + 'repeated once per id (`Ids=101&Ids=102&Ids=103`), or one comma-separated `Ids=1,2,3`. ' + + 'A bare array of the client’s BASE event — the same projection the browse feed and ' + + 'the room shelf serve, this one filtered to the requested ids.\n\n' + + 'Answers in the order the ids were asked for and skips ids with no event rather ' + + 'than leaving a hole, so the result may be shorter than the request. No ids at all ' + + 'is an empty array, not a 400.', + requestBody: form(BulkIdsRequest, 'The event ids to look up'), + responses: { + 200: json(PlayerEventBaseDto.array(), 'The events that exist, in request order'), + }, }), - async (c) => c.json(await getEventsByIds(c.env.DB, queryIds(c))) + async (c) => { + const events = await getEventsByIds(c.env.DB, await parseFormIds(c)) + return c.json(events.map(toEventBase)) + } + ) + + // The same read with the ids in the query (`?id=1&id=2`) — not what the client sends, + // but the shape stays identical to the POST's so the path can't answer two things. + .get( + '/api/playerevents/v1/bulk', + describeRoute({ + tags: ['Events'], + summary: 'Several player events by id (query form)', + description: + 'The same read as the POST on this path, with the ids in the query (`?id=1&id=2`) ' + + 'rather than a form body — the client sends the POST. Identical response: a bare ' + + 'array of the BASE event, in request order, skipping ids with no event.', + parameters: [intQuery('id', 'Repeatable event id')], + responses: { + 200: json(PlayerEventBaseDto.array(), 'The events that exist, in request order'), + }, + }), + async (c) => { + const events = await getEventsByIds(c.env.DB, queryIds(c)) + return c.json(events.map(toEventBase)) + } ) // RSVP. One row per player per event, so responding again replaces the previous diff --git a/apps/api/src/test/integration/api.test.ts b/apps/api/src/test/integration/api.test.ts index 22ca5a1..040bce9 100644 --- a/apps/api/src/test/integration/api.test.ts +++ b/apps/api/src/test/integration/api.test.ts @@ -6210,6 +6210,16 @@ describe('player events', () => { return { ...rest, ImageName: imageName, State: 0 } } + /** + * The client's BASE event behind an envelope's event — what the browse feed, the room + * shelf and the bulk read all serve. The envelope minus `Tags`, plus a null + * `BroadcastingRoomInstanceId`; `ImageName` is already `""` on the envelope. + */ + const asBase = (event: PlayerEventEnvelope): Record => { + const { Tags: _tags, ...rest } = event + return { ...rest, BroadcastingRoomInstanceId: null } + } + // The fixture set every test below reads. Times are relative to the run so the // upcoming/live/finished distinction the browse queries make is real. let upcoming: PlayerEventEnvelope @@ -6610,7 +6620,61 @@ describe('player events', () => { expect(await (await get(path)).json()).toEqual(asRecord(upcoming)) }) - test('GET /api/playerevents/v1/bulk answers in request order, skipping unknown ids', async () => { + test('POST /api/playerevents/v1/bulk answers the requested ids as base events', async () => { + // What the client sends: `Ids` repeated once per id, form-urlencoded. + const body = new URLSearchParams() + for (const id of [clubEvent.PlayerEventId, 999999, upcoming.PlayerEventId]) { + body.append('Ids', String(id)) + } + const res = await exports.default.fetch(`${ORIGIN}/api/playerevents/v1/bulk`, { + method: 'POST', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body, + }) + expect(res.status).toBe(200) + const events = (await res.json()) as PlayerEvent[] + + // Request order, not id order — and the missing id leaves no hole. + expect(events.map((e) => e.PlayerEventId)).toEqual([ + clubEvent.PlayerEventId, + upcoming.PlayerEventId, + ]) + + // A bare array — no envelope — of the BASE event, the same projection the browse feed + // and the room shelf serve. Not the stored record: no `State`. + const entry = events.find((e) => e.PlayerEventId === upcoming.PlayerEventId)! + expect(entry).toEqual(asBase(upcoming)) + expect(Object.keys(entry)).toHaveLength(17) + expect(Object.hasOwn(entry, 'State')).toBe(false) + }) + + test('POST /api/playerevents/v1/bulk reads a single id and the comma-separated form', async () => { + const bulk = async (raw: string): Promise => { + const res = await exports.default.fetch(`${ORIGIN}/api/playerevents/v1/bulk`, { + method: 'POST', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body: raw, + }) + expect(res.status).toBe(200) + return (await res.json()) as PlayerEvent[] + } + + // The raw one-id body the client sends for a single event. + expect((await bulk(`Ids=${upcoming.PlayerEventId}`)).map((e) => e.PlayerEventId)).toEqual([ + upcoming.PlayerEventId, + ]) + // …and the comma-separated spelling the other bulk POSTs take. + expect( + (await bulk(`Ids=${clubEvent.PlayerEventId},${upcoming.PlayerEventId}`)).map( + (e) => e.PlayerEventId + ) + ).toEqual([clubEvent.PlayerEventId, upcoming.PlayerEventId]) + // Nothing to look up is an empty array, not every event and not a 400. + expect(await bulk('')).toEqual([]) + expect(await bulk('Ids=')).toEqual([]) + }) + + test('GET /api/playerevents/v1/bulk answers the same shape as the POST', async () => { const res = await get( `/api/playerevents/v1/bulk?id=${clubEvent.PlayerEventId}&id=999999&id=${upcoming.PlayerEventId}` ) @@ -6621,6 +6685,10 @@ describe('player events', () => { clubEvent.PlayerEventId, upcoming.PlayerEventId, ]) + // The same base projection the POST serves: one path, one shape. + expect(events.find((e) => e.PlayerEventId === upcoming.PlayerEventId)).toEqual( + asBase(upcoming) + ) // No ids is an empty list, not every event. expect(await (await get('/api/playerevents/v1/bulk')).json()).toEqual([]) @@ -6812,9 +6880,15 @@ describe('player events', () => { expect(events.map((e) => e.PlayerEventId)).not.toContain(finished.PlayerEventId) expect(events.map((e) => e.PlayerEventId)).not.toContain(elsewhere.PlayerEventId) - // A bare array of the STORED record, like `/searchlive` and the multi-club shelf — - // not the base projection the browse feed serves, and not the single-club envelope. - expect(events[0]).toEqual(asRecord(running, null)) + // The BASE event, 17 keys — the same projection the browse feed and the bulk read + // serve, since the client decodes all three through one helper and one element type. + // Not the stored record (`/searchlive` and the club shelves keep that), and not the + // single-club envelope. + expect(events[0]).toEqual(asBase(running)) + expect(Object.keys(events[0]!)).toHaveLength(17) + expect(Object.hasOwn(events[0]!, 'State')).toBe(false) + // An event created with no banner reads `""` here, never the record's null. + expect(events[0]!.ImageName).toBe('') // A room with nothing scheduled, and a room id nothing knows about, are both empty. expect(await (await get('/api/playerevents/v1/room/999999')).json()).toEqual([]) @@ -7605,6 +7679,7 @@ describe('openapi', () => { 'POST /api/messages/v3/delete', 'POST /api/playerReputation/v1/bulk', 'POST /api/playerReputation/v2/bulk', + 'POST /api/playerevents/v1/bulk', 'POST /api/playerevents/v1/bulkInvite', 'POST /api/playerevents/v1/report', 'POST /api/playerevents/v1/respond',