diff --git a/apps/api/src/images-db.ts b/apps/api/src/images-db.ts index 2481cf4..4a24c83 100644 --- a/apps/api/src/images-db.ts +++ b/apps/api/src/images-db.ts @@ -118,12 +118,16 @@ export async function createImage(db: D1Database, input: NewImage): Promise { const row = await db - .prepare('SELECT COUNT(*) AS n FROM image_interaction WHERE saved_image_id = ?1 AND cheered = 1') + .prepare( + 'SELECT COUNT(*) AS n FROM image_interaction WHERE saved_image_id = ?1 AND cheered = 1' + ) .bind(savedImageId) .first<{ n: number }>() const count = row?.n ?? 0 await db - .prepare("UPDATE image SET data = json_set(data, '$.CheerCount', CAST(?2 AS INTEGER)) WHERE id = ?1") + .prepare( + "UPDATE image SET data = json_set(data, '$.CheerCount', CAST(?2 AS INTEGER)) WHERE id = ?1" + ) .bind(savedImageId, count) .run() return count @@ -223,9 +227,7 @@ export async function getImagesByRoom( if (filter > 0) images = images.filter((img) => img.Type === filter) - images.sort( - sort === 1 ? (a, b) => b.CheerCount - a.CheerCount || newestFirst(a, b) : newestFirst - ) + images.sort(sort === 1 ? (a, b) => b.CheerCount - a.CheerCount || newestFirst(a, b) : newestFirst) return images.slice(skip, skip + take) } @@ -257,6 +259,46 @@ export async function getImagesByPlayer( .slice(skip, skip + take) } +/** + * The client-facing projection of a saved image for the player photo lists (the + * reference's `ImagesPlayer`). Same data as the stored record, but the id and type + * are renamed — `Id` → `SavedImageId`, `Type` → `SavedImageType` — and the tagged + * player ids aren't part of it. The client deserializes into this shape, so a raw + * SavedImage leaves it without an image id and its thumbnails come up blank. + */ +export interface ImagesPlayer { + Accessibility: number + AccessibilityLocked: boolean + CheerCount: number + CommentCount: number + CreatedAt: string + Description: string | null + ImageName: string + PlayerEventId: number | null + PlayerId: number + RoomId: number | null + SavedImageId: number + SavedImageType: number +} + +/** Project a stored image to the client's ImagesPlayer shape. */ +export function toImagesPlayer(img: SavedImage): ImagesPlayer { + return { + Accessibility: img.Accessibility, + AccessibilityLocked: img.AccessibilityLocked, + CheerCount: img.CheerCount, + CommentCount: img.CommentCount, + CreatedAt: img.CreatedAt, + Description: img.Description, + ImageName: img.ImageName, + PlayerEventId: img.PlayerEventId, + PlayerId: img.PlayerId, + RoomId: img.RoomId, + SavedImageId: img.Id, + SavedImageType: img.Type, + } +} + /** Default number of recent images the slideshow feed returns. */ export const SLIDESHOW_LIMIT = 130 diff --git a/apps/api/src/routes/images.ts b/apps/api/src/routes/images.ts index 22cfee0..75b0dc6 100644 --- a/apps/api/src/routes/images.ts +++ b/apps/api/src/routes/images.ts @@ -1,5 +1,6 @@ import { Hono } from 'hono' +import { authedId, unauthorized } from '../http' import { createImage, deleteImage, @@ -11,8 +12,8 @@ import { getSlideshowImages, SavedImageType, setImageCheer, + toImagesPlayer, } from '../images-db' -import { authedId, unauthorized } from '../http' import type { App } from '../context' @@ -139,12 +140,14 @@ export const imageRoutes = new Hono({ strict: false }) }) // A player's photos — the public images that player has taken, newest first. - // Paginated via skip/take (take defaults to 100). Returns a bare array. + // Paginated via skip/take (take defaults to 100). Returns a bare array of the + // client's ImagesPlayer projection (SavedImageId/SavedImageType, not Id/Type). .get('/api/images/v4/player/:playerId{[0-9]+}', async (c) => { const playerId = Number.parseInt(c.req.param('playerId'), 10) const skip = Number.parseInt(c.req.query('skip') ?? '0', 10) || 0 const take = Number.parseInt(c.req.query('take') ?? '100', 10) || 100 - return c.json(await getImagesByPlayer(c.env.DB, playerId, 0, skip, take)) + const images = await getImagesByPlayer(c.env.DB, playerId, 0, skip, take) + return c.json(images.map(toImagesPlayer)) }) // A player's photos with a sort option. `sort` orders the list (1 = most @@ -154,16 +157,19 @@ export const imageRoutes = new Hono({ strict: false }) const sort = Number.parseInt(c.req.query('sort') ?? '0', 10) || 0 const skip = Number.parseInt(c.req.query('skip') ?? '0', 10) || 0 const take = Number.parseInt(c.req.query('take') ?? '100', 10) || 100 - return c.json(await getImagesByPlayer(c.env.DB, playerId, sort, skip, take)) + const images = await getImagesByPlayer(c.env.DB, playerId, sort, skip, take) + return c.json(images.map(toImagesPlayer)) }) // A player's photo feed — the public images they took plus ones they're tagged - // in, newest first. Paginated via skip/take (take defaults to 100). Bare array. + // in, newest first. Paginated via skip/take (take defaults to 100). Bare array of + // the same ImagesPlayer projection the player photo lists use. .get('/api/images/v3/feed/player/:playerId{[0-9]+}', async (c) => { const playerId = Number.parseInt(c.req.param('playerId'), 10) const skip = Number.parseInt(c.req.query('skip') ?? '0', 10) || 0 const take = Number.parseInt(c.req.query('take') ?? '100', 10) || 100 - return c.json(await getPlayerFeed(c.env.DB, playerId, skip, take)) + const images = await getPlayerFeed(c.env.DB, playerId, skip, take) + return c.json(images.map(toImagesPlayer)) }) // Global slideshow feed — the most recent publicly-listable ShareCamera photos @@ -214,5 +220,7 @@ export const imageRoutes = new Hono({ strict: false }) .map((raw) => Number.parseInt(raw.trim(), 10)) .filter((imageId) => !Number.isNaN(imageId)) ?? [] const cheered = await getCheeredImageIds(c.env.DB, id, ids) - return c.json(ids.map((imageId) => ({ SavedImageId: imageId, IsCheered: cheered.has(imageId) }))) + return c.json( + ids.map((imageId) => ({ SavedImageId: imageId, IsCheered: cheered.has(imageId) })) + ) }) diff --git a/apps/api/src/test/integration/api.test.ts b/apps/api/src/test/integration/api.test.ts index 7d8ddb6..df5b950 100644 --- a/apps/api/src/test/integration/api.test.ts +++ b/apps/api/src/test/integration/api.test.ts @@ -1140,7 +1140,8 @@ describe('images', () => { test('POST /api/images/v1/cheer persists, syncs CheerCount, and the bulk lookup reflects it', async () => { // Seed an image to cheer. - const img = await createImage(env.DB, { imageName: 'cheerme.jpg', playerId: 700 }) + // Its own player id: 700's photos are asserted on exactly in the player-list test. + const img = await createImage(env.DB, { imageName: 'cheerme.jpg', playerId: 7001 }) const cheerBody = JSON.stringify({ SavedImageId: img.Id, Cheer: true }) // No token → 401. @@ -1465,23 +1466,47 @@ describe('images', () => { seed({ Id: 205, PlayerId: 999, TaggedPlayerIds: [111] }), ]) + // The lists serve the client's ImagesPlayer projection: the id and type are + // SavedImageId/SavedImageType, and TaggedPlayerIds isn't part of it. + type ImagesPlayer = { SavedImageId: number; SavedImageType: number; ImageName: string } + // v4/player → only photos 700 *took*, public, newest first. const mine = (await ( await exports.default.fetch(`${ORIGIN}/api/images/v4/player/700`) - ).json()) as SavedImage[] - expect(mine.map((i) => i.Id)).toEqual([202, 201]) + ).json()) as ImagesPlayer[] + expect(mine.map((i) => i.SavedImageId)).toEqual([202, 201]) + expect(mine[0]).toEqual({ + Accessibility: 1, + AccessibilityLocked: false, + CheerCount: 0, + CommentCount: 0, + CreatedAt: '2026-04-01T00:00:00.000Z', + Description: null, + ImageName: 'p202.jpg', + PlayerEventId: null, + PlayerId: 700, + RoomId: null, + SavedImageId: 202, + SavedImageType: 1, + }) // take paginates. const one = (await ( await exports.default.fetch(`${ORIGIN}/api/images/v4/player/700?take=1`) - ).json()) as SavedImage[] - expect(one.map((i) => i.Id)).toEqual([202]) + ).json()) as ImagesPlayer[] + expect(one.map((i) => i.SavedImageId)).toEqual([202]) + + // v5/player is the same list with a sort option (0 = newest first). + const sorted = (await ( + await exports.default.fetch(`${ORIGIN}/api/images/v5/player/700?sort=0`) + ).json()) as ImagesPlayer[] + expect(sorted.map((i) => i.SavedImageId)).toEqual([202, 201]) // v3/feed/player → photos taken *or* tagged in, newest first (204 is newest). const feed = (await ( await exports.default.fetch(`${ORIGIN}/api/images/v3/feed/player/700?take=100`) - ).json()) as SavedImage[] - expect(feed.map((i) => i.Id)).toEqual([204, 202, 201]) + ).json()) as ImagesPlayer[] + expect(feed.map((i) => i.SavedImageId)).toEqual([204, 202, 201]) // A player with no photos → empty array on both. expect( diff --git a/apps/clubs/src/clubs-db.ts b/apps/clubs/src/clubs-db.ts index d18c05c..cb7742b 100644 --- a/apps/clubs/src/clubs-db.ts +++ b/apps/clubs/src/clubs-db.ts @@ -130,9 +130,9 @@ interface StoredClub extends Club { CreatedAt: string CustomTags?: string[] /** - * The club's gallery images, by slot (the client PUTs to - * `/additionalimage/{index}`). Positional, so a cleared middle slot stays as an - * empty string rather than shifting the images after it. + * The club's gallery image names, in order (the client PUTs to + * `/additionalimage/{index}`). Packed, never sparse: removing one shifts the rest + * up, so the list is always the images the club actually has. */ AdditionalImages?: string[] } @@ -673,39 +673,32 @@ function dedupeTags(tags: string[]): string[] { return out } -/** - * A club's gallery images (stored on the blob). Trailing empty slots are trimmed, so - * a club with nothing set reads as `[]` while a club with only slot 1 filled still - * reports `['', 'name.jpg']` — the index a client PUT to is the index it reads back. - */ +/** A club's gallery image names, in order (stored on the blob; `[]` when it has none). */ export async function getClubAdditionalImages(db: D1Database, clubId: number): Promise { const row = await db .prepare('SELECT data FROM club WHERE club_id = ?1') .bind(clubId) .first() - const images = row === null ? [] : ((JSON.parse(row.data) as StoredClub).AdditionalImages ?? []) - let end = images.length - while (end > 0 && images[end - 1] === '') end-- - return images.slice(0, end) + return row === null ? [] : ((JSON.parse(row.data) as StoredClub).AdditionalImages ?? []) } /** - * A club's gallery as the client reads it: the image record behind each filled slot, - * in slot order. Empty slots are left out (the records carry no index, so a hole - * would just be a blank image), and a name whose metadata row is missing falls back - * to a placeholder record so the picture still renders. + * A club's gallery as the client reads it: the image record behind each name, in + * order. A name whose metadata row is missing falls back to a placeholder record so + * the picture still renders. */ export async function getClubGallery(db: D1Database, clubId: number): Promise { - const names = (await getClubAdditionalImages(db, clubId)).filter((n) => n !== '') + const names = await getClubAdditionalImages(db, clubId) if (names.length === 0) return [] const records = await getSavedImagesByNames(db, names) return names.map((name) => records.get(name) ?? placeholderSavedImage(name)) } /** - * Set (or clear, with an empty `imageName`) one of a club's gallery image slots. - * Returns null when the club doesn't exist. The slot must be in range — callers - * validate the index before getting here. + * Set (or remove, with an empty `imageName`) one of a club's gallery images. The list + * stays packed: removing an image shifts the ones after it up, and setting an index + * past the end appends rather than leaving a gap. Returns null when the club doesn't + * exist; the caller validates the index is in range. */ export async function setClubAdditionalImage( db: D1Database, @@ -720,11 +713,15 @@ export async function setClubAdditionalImage( if (row === null) return null const stored = JSON.parse(row.data) as StoredClub - // Pad rather than assign past the end: a sparse array would serialize its holes as - // nulls, and the client expects strings in every slot it reads. const images = [...(stored.AdditionalImages ?? [])] - while (images.length <= index) images.push('') - images[index] = imageName + if (imageName === '') { + // Removing past the end is a no-op, not an error: the image is already gone. + if (index < images.length) images.splice(index, 1) + } else if (index < images.length) { + images[index] = imageName + } else if (images.length < MAX_ADDITIONAL_IMAGES) { + images.push(imageName) + } const updated: StoredClub = { ...stored, AdditionalImages: images } await db diff --git a/apps/clubs/src/clubs.app.ts b/apps/clubs/src/clubs.app.ts index 180afa4..95b5c0a 100644 --- a/apps/clubs/src/clubs.app.ts +++ b/apps/clubs/src/clubs.app.ts @@ -574,15 +574,15 @@ const app = new Hono() }) }) - // One of the club's gallery images, by slot (`/additionalimage/{index}`, 0-based — - // the client PUTs the first image to 0, the second to 1). Takes the same - // `imageName` the `storage` worker handed back; an empty one clears that slot. - // Co-owner or above, like the main image. The slots are positional, so clearing - // one doesn't shift the others; they come back on `value.AdditionalImages`. + // One of the club's gallery images, by position (`/additionalimage/{index}`, 0-based + // — the client PUTs the first image to 0, the second to 1). Takes the same + // `imageName` the `storage` worker handed back. Co-owner or above, like the main + // image. The list is packed: a PUT past the end appends rather than leaving a gap, + // and the images come back in order on `value.AdditionalImages`. // - // DELETE removes that slot's image — same thing as PUTting an empty name, with the - // intent spelled out. It ignores any body, so it can't accidentally set one, and - // deleting an already-empty slot is a no-op rather than an error. + // DELETE removes that position's image and shifts the rest up, so there's never a + // blank slot in the gallery. It ignores any body, so it can't accidentally set an + // image instead, and deleting a position that holds nothing is a no-op. .on(['PUT', 'DELETE'], '/club/:clubId{[0-9]+}/additionalimage/:index{[0-9]+}', async (c) => { const id = await authedId(c) if (id === null) return c.body(null, 401) diff --git a/apps/clubs/src/test/integration/api.test.ts b/apps/clubs/src/test/integration/api.test.ts index 190adb0..2af4e67 100644 --- a/apps/clubs/src/test/integration/api.test.ts +++ b/apps/clubs/src/test/integration/api.test.ts @@ -634,29 +634,30 @@ describe('clubs endpoints', () => { expect.objectContaining({ Id: 500, ImageName: first, PlayerId: 7100 }), ]) - // Slots are positional: filling 2 while 1 is empty keeps them in that order, and - // the empty slot isn't served as a blank image. + // The list stays packed: a PUT past the end appends rather than leaving a gap. const third = (await (await setImage(2, 'c.jpg')).json()) as Details expect(names(third.value.AdditionalImages)).toEqual([first, 'c.jpg']) - const second = (await (await setImage(1, 'b.jpg')).json()) as Details - expect(names(second.value.AdditionalImages)).toEqual([first, 'b.jpg', 'c.jpg']) + const second = (await (await setImage(2, 'b.jpg')).json()) as Details + expect(names(second.value.AdditionalImages)).toEqual([first, 'c.jpg', 'b.jpg']) - // Re-PUTting a slot replaces just that image; an empty name clears it. A name with - // no image row still renders, as a placeholder record. + // Re-PUTting a position replaces just that image. A name with no image row still + // renders, as a placeholder record. const replaced = (await (await setImage(0, 'a2.jpg')).json()) as Details - expect(names(replaced.value.AdditionalImages)).toEqual(['a2.jpg', 'b.jpg', 'c.jpg']) + expect(names(replaced.value.AdditionalImages)).toEqual(['a2.jpg', 'c.jpg', 'b.jpg']) expect(replaced.value.AdditionalImages[0]).toMatchObject({ Id: 0, ImageName: 'a2.jpg' }) + + // An empty name removes that image and shifts the rest up — no blank left behind. const cleared = (await (await setImage(1, '')).json()) as Details - expect(names(cleared.value.AdditionalImages)).toEqual(['a2.jpg', 'c.jpg']) + expect(names(cleared.value.AdditionalImages)).toEqual(['a2.jpg', 'b.jpg']) // They're on the club's details payload, for everyone reading the club. const details = (await ( await exports.default.fetch(`${ORIGIN}/club/${clubId}/details`) ).json()) as { AdditionalImages: Image[] } - expect(names(details.AdditionalImages)).toEqual(['a2.jpg', 'c.jpg']) + expect(names(details.AdditionalImages)).toEqual(['a2.jpg', 'b.jpg']) - // DELETE removes a slot's image, ignoring any body, and repeats are no-ops. The - // other slots keep their positions, so slot 2 is still slot 2. + // DELETE removes that position's image and shifts the rest up, ignoring any body. + // Deleting a position that holds nothing is a no-op. const deleteImage = async (index: number, sub = '7100', body?: string) => exports.default.fetch(`${ORIGIN}/club/${clubId}/additionalimage/${index}`, { method: 'DELETE', @@ -665,12 +666,14 @@ describe('clubs endpoints', () => { }) const dropped = (await (await deleteImage(0, '7100', 'imageName=sneaky.jpg')).json()) as Details expect(dropped).toMatchObject({ error: '', success: true }) - expect(names(dropped.value.AdditionalImages)).toEqual(['c.jpg']) + expect(names(dropped.value.AdditionalImages)).toEqual(['b.jpg']) + // Position 1 holds nothing now, so deleting it changes nothing. expect( - names(((await (await deleteImage(0)).json()) as Details).value.AdditionalImages) - ).toEqual(['c.jpg']) - const refilled = (await (await setImage(0, 'a3.jpg')).json()) as Details - expect(names(refilled.value.AdditionalImages)).toEqual(['a3.jpg', 'c.jpg']) + names(((await (await deleteImage(1)).json()) as Details).value.AdditionalImages) + ).toEqual(['b.jpg']) + // A PUT past the end appends, so the club is back to two images. + const refilled = (await (await setImage(1, 'a3.jpg')).json()) as Details + expect(names(refilled.value.AdditionalImages)).toEqual(['b.jpg', 'a3.jpg']) // Same gate as the PUT. expect((await deleteImage(0, '7101')).status).toBe(403)