diff --git a/apps/lists/src/lists.app.ts b/apps/lists/src/lists.app.ts index f07cb6e..9ec3750 100644 --- a/apps/lists/src/lists.app.ts +++ b/apps/lists/src/lists.app.ts @@ -10,6 +10,7 @@ import { getPlayerList, getRecentlyUpdatedRooms, getVisitedRooms, + removePlayerListItem, } from '@repo/domain' import { withCleanSpec, withNotFound, withOnError } from '@repo/hono-helpers' import { validateAndGetAccountId } from '@repo/jwt' @@ -24,15 +25,17 @@ import { AUTHED, ContextualFeaturesAck, CREATOR_ACCOUNT_ID_PARAM, + CuratedListEnvelope, CuratedListRead, - CuratedListSaved, CuratedListsBulk, form, ITEM_ID_PARAM, json, + LIST_ID_PARAM, LIST_IDS_PARAM, LIST_NAME_PARAM, LIST_TYPE_PARAM, + REMOVE_ITEM_ID_PARAM, SAVE_LIST_NAME_PARAM, SaveItemBody, UNAUTHORIZED_RESPONSE, @@ -54,6 +57,24 @@ async function authedId(c: Context): Promise { return validateAndGetAccountId(c.req.raw, await c.env.JWT_SECRET.get()) } +/** + * What a list WRITE answers (the save and the remove): the list as it now stands, in the + * save's projection — every key the read serves minus `Accessibility`, a real difference + * from the read, not an oversight — wrapped in `{ Value, Success, Error, error_id }`. The + * reads serve the list bare; don't unify them. + * + * Spliced as a string because `serializeCuratedList` is what puts the 64-bit `ListId` on the + * wire unquoted with every digit intact, and `c.json` would undo that. + */ +function listWritten(c: Context, list: CuratedList) { + const { Accessibility: _accessibility, ...saved } = list + return c.body( + `{"Value":${serializeCuratedList(saved)},"Success":true,"Error":null,"error_id":null}`, + 200, + { 'content-type': 'application/json' } + ) +} + /** Results.Unauthorized() equivalent — 401 with empty body. */ function unauthorized(c: Context) { return c.body(null, 401) @@ -224,6 +245,18 @@ const PERSONAL_ROW_FEEDS: Record recentlyvisited: (db, accountId) => getVisitedRooms(db, accountId, 0, LIST_SIZE), } +/** + * Rows that are a HAND-CURATED list in the `list` table, keyed by row slug and naming the + * list behind it. The list is the Coach's (account 1), of the row's requested `type`, so + * staff edit a row by editing that list's `list_item`s; its insertion order is the row's. + * + * The owner is pinned rather than matched on name alone: any player can create a list of any + * name through `createlistifneeded`, and a name-only lookup would let one fill the row. + */ +const LIST_ROWS: Record = { + staffpicks: 'staffpicks', +} + /** * What a GENERIC row's ids look like. A `Generic` row is the one kind that can name things * of MORE THAN ONE sort in a single list, so the sort travels in the id itself: the client @@ -731,7 +764,7 @@ const app = new Hono() // that took an owner from the client would let anyone write into anyone's list. // // Answers the list as it now stands rather than an acknowledgement, so the row the client - // re-renders is the one this call just changed. + // re-renders is the one this call just changed — wrapped, see `listWritten`. .put( '/curatedlists/:name/items/:itemId/createlistifneeded', describeRoute({ @@ -751,15 +784,16 @@ const app = new Hono() 're-renders is the one this call just changed. Saving the same item twice leaves it in', 'the list once.', '', - 'The response drops `Accessibility`, which the read keeps. That is a real difference in', - 'what the client is sent, not an oversight — every other key, and their order, is the', - 'read’s.', + 'The list comes wrapped in a `{ Value, Success, Error, error_id }` envelope, which the', + 'read does not use, and drops `Accessibility`, which the read keeps. Both are real', + 'differences in what the client is sent, not oversights — every other key, and their', + 'order, is the read’s.', ].join('\n'), security: AUTHED, parameters: [SAVE_LIST_NAME_PARAM, ITEM_ID_PARAM], requestBody: form(SaveItemBody, 'Applied only when the list is created'), responses: { - 200: json(CuratedListSaved, 'The list as it now stands, without `Accessibility`'), + 200: json(CuratedListEnvelope, 'The list as it now stands, wrapped'), 401: UNAUTHORIZED_RESPONSE, }, }), @@ -785,14 +819,50 @@ const app = new Hono() c.req.param('itemId') ) - // The SAVE's projection of a list drops `Accessibility`; the read's keeps it. That is a - // real difference in what the client is sent, not an oversight — don't unify them. - // Every other key, and their order, is the read's. - const { Accessibility: _accessibility, ...saved } = list + return listWritten(c, list) + } + ) - // Serialized by hand for the same reason the read is: the 64-bit `ListId` has to reach - // the client unquoted with every digit intact. - return c.body(serializeCuratedList(saved), 200, { 'content-type': 'application/json' }) + // Remove an item from one of the caller's own lists (`DELETE /curatedlists/:listId/items/:itemId`) + // — what the client calls to un-save a room. The path names the list by ID rather than by + // name, so unlike the save the owner is not implied by the token: the list must BE the + // caller's, or the call is refused. Answers the list as it now stands, exactly as the save + // does — see `listWritten`. + .delete( + '/curatedlists/:listId/items/:itemId', + describeRoute({ + tags: ['Lists', '2025'], + summary: 'Remove an item from the caller’s list', + description: [ + 'Removes an item from one of the caller’s own lists — what the client calls to un-save', + 'a room. The path names the list by ID, so the list must belong to the token’s account:', + 'another player’s list is refused with 403, and an unknown id is 404.', + '', + 'Answers the list as it now stands exactly as the save does: wrapped in a', + '`{ Value, Success, Error, error_id }` envelope, without `Accessibility`.', + 'Removing an item the list doesn’t hold answers the list unchanged, and a list that', + 'empties is kept rather than deleted.', + ].join('\n'), + security: AUTHED, + parameters: [LIST_ID_PARAM, REMOVE_ITEM_ID_PARAM], + responses: { + 200: json(CuratedListEnvelope, 'The list as it now stands, wrapped'), + 401: UNAUTHORIZED_RESPONSE, + 403: { description: 'The list is another player’s (empty body)' }, + 404: { description: 'No list has that id (empty body)' }, + }, + }), + async (c) => { + const accountId = await authedId(c) + if (accountId === null) return unauthorized(c) + + const listId = Number(c.req.param('listId')) + if (!Number.isSafeInteger(listId)) return c.body(null, 404) + + const result = await removePlayerListItem(c.env.DB, accountId, listId, c.req.param('itemId')) + if (!result.ok) return c.body(null, result.reason === 'not_owner' ? 403 : 404) + + return listWritten(c, result.list) } ) @@ -828,7 +898,9 @@ const app = new Hono() '`recentlyvisited` is per-caller and is the one row that reads the token; without one it', 'answers EMPTY rather than 401ing, since canned rooms would claim the caller visited', 'rooms they never did — and an empty carousel is what a brand-new account legitimately', - 'has. A couple of store rows are hand-picked id lists.', + 'has. A couple of store rows are hand-picked id lists. `staffpicks` serves the Coach’s', + '(account 1) list named `staffpicks` of the requested type from the `list` table, in', + 'insertion order — empty until that list exists.', '', 'Every other row — an unknown key included — answers an EMPTY 200 rather than a 404,', 'which the client renders as a row that failed to load instead of one it hides.', @@ -905,6 +977,13 @@ const app = new Hono() return c.json({ Type: echoed, Entities: toEntities(rooms) }) } + // A hand-curated row serves its list's ids as stored. No list yet answers an empty row. + const listName = LIST_ROWS[key] + if (listName !== undefined) { + const list = await getPlayerList(c.env.DB, COACH_ACCOUNT_ID, echoed, listName) + return c.json({ Type: echoed, Entities: entities(list?.ItemIds ?? []) }) + } + // A row with a live feed behind it serves that; everything else gets the canned // entities. Only the ids travel — the client resolves each room itself — so the // ranking is read for its order and the room blobs are thrown away. diff --git a/apps/lists/src/openapi.ts b/apps/lists/src/openapi.ts index ed551ac..acdefe2 100644 --- a/apps/lists/src/openapi.ts +++ b/apps/lists/src/openapi.ts @@ -135,6 +135,27 @@ export const SAVE_LIST_NAME_PARAM: OpenAPIV3_1.ParameterObject = { schema: { type: 'string', example: '__SavedForLater_Rooms' }, } +/** `{listId}` on the remove — one of the caller's own lists, by id. */ +export const LIST_ID_PARAM: OpenAPIV3_1.ParameterObject = { + name: 'listId', + in: 'path', + required: true, + description: 'The list to remove from. It must be the caller’s own.', + schema: { type: 'integer', example: 3437 }, +} + +/** `{itemId}` on the remove — what comes out of the list, as a string. */ +export const REMOVE_ITEM_ID_PARAM: OpenAPIV3_1.ParameterObject = { + name: 'itemId', + in: 'path', + required: true, + description: [ + 'The item to remove, as a string. Removing one the list doesn’t hold answers the list', + 'unchanged.', + ].join(' '), + schema: { type: 'string', example: '2576' }, +} + /** `{itemId}` on the save — what goes into the list, as a string. */ export const ITEM_ID_PARAM: OpenAPIV3_1.ParameterObject = { name: 'itemId', @@ -232,6 +253,18 @@ export const CuratedListRead = z.object({ /** The SAVE's projection: every key of the read, in the read's order, minus `Accessibility`. */ export const CuratedListSaved = z.object(CuratedListFields) +/** + * What the WRITES answer (`PUT …/createlistifneeded`, `DELETE …/items/{itemId}`): the save's + * projection of the list, wrapped in a `{ Value, Success, Error, error_id }` envelope. The + * reads serve the list bare. + */ +export const CuratedListEnvelope = z.object({ + Value: CuratedListSaved, + Success: z.literal(true), + Error: z.null(), + error_id: z.null(), +}) + /** `GET /curatedlists/bulk` — a list per id asked for; today one canned list, always. */ export const CuratedListsBulk = CuratedListRead.array() diff --git a/apps/lists/src/test/integration/api.test.ts b/apps/lists/src/test/integration/api.test.ts index 8b490fc..3fe28d4 100644 --- a/apps/lists/src/test/integration/api.test.ts +++ b/apps/lists/src/test/integration/api.test.ts @@ -797,9 +797,12 @@ it('creates the list and adds the item on PUT …/createlistifneeded', async () // changed — with the id unquoted, as every read of a list serves it. const body = await res.text() expect(body).toMatch(/"ListId":\d+,/) - // The SAVE's projection: every key the read serves EXCEPT `Accessibility`, in the read's - // order. Compared exactly, not loosely, because the missing key is the point. - const { ListId: _id, ...rest } = JSON.parse(body) as Record + // Wrapped in the write envelope, around the SAVE's projection: every key the read serves + // EXCEPT `Accessibility`, in the read's order. Compared exactly, not loosely, because the + // missing key is the point. + const { Value, ...envelope } = JSON.parse(body) as { Value: Record } + expect(envelope).toEqual({ Success: true, Error: null, error_id: null }) + const { ListId: _id, ...rest } = Value expect(rest).toEqual({ CreatorAccountId: 300, Name: '__SavedForLater_Rooms', @@ -1510,6 +1513,7 @@ it('generates a spec with no dangling $refs', async () => { '/curatedlists', '/curatedlists/bulk', '/curatedlists/{name}/items/{itemId}/createlistifneeded', + '/curatedlists/{listId}/items/{itemId}', '/algorithmiclists/{list}', '/contextualfeatures', ]) @@ -1518,3 +1522,88 @@ it('generates a spec with no dangling $refs', async () => { expect(Object.keys(spec.paths)).not.toContain('/openapi.json') expect(JSON.stringify(spec).match(/\$ref/g)).toBeNull() }) + +it('serves the Coach’s `staffpicks` list for /algorithmiclists/staffpicks', async () => { + // No list yet: an empty row, not a 404. + const empty = await SELF.fetch(`${ORIGIN}/algorithmiclists/staffpicks?type=1`) + expect(await empty.json()).toEqual({ Type: 1, Entities: [] }) + + // A player's list of the same name must not fill the row — only the Coach's does. + await SELF.fetch(`${ORIGIN}/curatedlists/staffpicks/items/9/createlistifneeded`, { + method: 'PUT', + headers: await bearer('42'), + body: new URLSearchParams({ type: '1', accessibility: '1' }), + }) + const now = new Date().toISOString() + const { list_id } = (await env.DB.prepare( + `INSERT INTO list (creator_account_id, list_type, list_name, accessibility, created_at) + VALUES (1, 1, 'StaffPicks', 1, ?1) RETURNING list_id` + ) + .bind(now) + .first<{ list_id: number }>())! + for (const id of ['7', '3', '5']) { + await env.DB.prepare('INSERT INTO list_item (list_id, item_id) VALUES (?1, ?2)') + .bind(list_id, id) + .run() + } + + const res = await SELF.fetch(`${ORIGIN}/algorithmiclists/StaffPicks?type=1`) + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ + Type: 1, + Entities: ['7', '3', '5'].map((Id) => ({ Id, Context: null })), + }) +}) + +/** The client's un-save call: DELETE the item from the list by id. */ +async function removeFromList(listId: string, itemId: string, headers: Record) { + return SELF.fetch(`${ORIGIN}/curatedlists/${listId}/items/${itemId}`, { + method: 'DELETE', + headers, + }) +} + +it('removes an item from the caller’s list on DELETE …/items/:itemId', async () => { + const headers = await bearer('320') + await saveForLater('__SavedForLater_Rooms', '953', headers) + const saved = await (await saveForLater('__SavedForLater_Rooms', '2576', headers)).text() + const listId = /"ListId":(\d+),/.exec(saved)![1] + + const res = await removeFromList(listId, '2576', headers) + expect(res.status).toBe(200) + const body = await res.text() + expect(body).toMatch(new RegExp(`"ListId":${listId},`)) + const { Value, ...envelope } = JSON.parse(body) as { Value: Record } + expect(envelope).toEqual({ Success: true, Error: null, error_id: null }) + const { ListId: _id, ...rest } = Value + expect(rest).toEqual({ + CreatorAccountId: 320, + Name: '__SavedForLater_Rooms', + Description: null, + ImageName: 'DefaultRoomImage.jpg', + Type: 1, + ItemIds: ['953'], + CreatedAt: expect.any(String), + }) + + // Removing the last item keeps the list; removing an absent one answers it unchanged. + await removeFromList(listId, '953', headers) + const again = await removeFromList(listId, '953', headers) + expect(again.status).toBe(200) + expect((await again.json()) as { Value: { ItemIds: string[] } }).toMatchObject({ + Value: { ItemIds: [] }, + }) + expect(await readList(320)).toMatchObject({ ItemIds: [] }) +}) + +it('refuses to remove from another player’s list, or without a token', async () => { + const saved = await ( + await saveForLater('__SavedForLater_Rooms', '953', await bearer('321')) + ).text() + const listId = /"ListId":(\d+),/.exec(saved)![1] + + expect((await removeFromList(listId, '953', await bearer('322'))).status).toBe(403) + expect((await removeFromList(listId, '953', {})).status).toBe(401) + expect((await removeFromList('999999', '953', await bearer('321'))).status).toBe(404) + expect((await readList(321)).ItemIds).toEqual(['953']) +}) diff --git a/packages/domain/src/lists-db.ts b/packages/domain/src/lists-db.ts index c9fe34b..622c7e0 100644 --- a/packages/domain/src/lists-db.ts +++ b/packages/domain/src/lists-db.ts @@ -223,3 +223,41 @@ export async function getPlayerList( if (row === null) return undefined return toCuratedList(row, await getListItems(db, row.list_id)) } + +/** What {@link removePlayerListItem} found: the list as it now stands, or why it can't be. */ +export type RemoveListItemResult = + { ok: true; list: CuratedList } | { ok: false; reason: 'not_found' | 'not_owner' } + +/** + * Remove an item from a list the caller owns — `DELETE /curatedlists/:listId/items/:itemId`, + * which the client calls to un-save a room. Unlike the save, the path names the list by ID, + * so ownership has to be CHECKED rather than implied: the list must be the caller's. + * + * Removing an item the list doesn't hold is not an error — the list already says what the + * caller wants it to — and answers the list unchanged. The list itself is kept when it + * empties: the client created it and expects to find it again by name. + */ +export async function removePlayerListItem( + db: D1Database, + accountId: number, + listId: number, + itemId: string +): Promise { + const row = await db + .prepare( + `SELECT list_id, creator_account_id, list_type, list_name, list_description, + image_name, accessibility, created_at + FROM list WHERE list_id = ?1` + ) + .bind(listId) + .first() + if (row === null) return { ok: false, reason: 'not_found' } + if (row.creator_account_id !== accountId) return { ok: false, reason: 'not_owner' } + + await db + .prepare('DELETE FROM list_item WHERE list_id = ?1 AND item_id = ?2') + .bind(listId, itemId) + .run() + + return { ok: true, list: toCuratedList(row, await getListItems(db, listId)) } +}