From a70c2eb9e028ea8b38960ae01dd57e56dd7489eb Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 5 Sep 2026 12:53:39 +0200 Subject: [PATCH] fix(collections): read the created list off the response the server actually sends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/addons/collections answers with the collection itself — the controller returns the service's `Collection` straight through, and the e2e suite pins that shape. The client declared a `{ collection }` envelope and read `.collection` off it, so `createCollection()` resolved `undefined` and handed back `null` every single time. The caller needs that id. In the list editor and its phone sheet the id is what the cover upload and `onCreated` hang off, so creating a list with a cover picked in the same dialog silently dropped the image, the new list was never opened or selected afterwards, and the retry guard that exists to stop a failed cover step from minting a duplicate was dead. The list itself still appeared, because `loadAll()` runs first — which is why this went unnoticed. Nothing caught it because both test layers stubbed the shape the client believed in rather than the one the server sends: the api suite's MSW handlers wrapped the collection in an envelope, and the store suite's mock resolved one too. Both now answer the way the controller does, `update()` included — its declared type was wrong for the same reason, though no caller reads its result. The fix is on the client. That wire shape has shipped for as long as Collections has existed and the server e2e test pins it, so changing that side would be the breaking one. --- client/src/api/collections.test.ts | 13 ++++++++----- client/src/api/collections.ts | 8 ++++++-- client/src/store/collectionStore.test.ts | 17 ++++++++++++++--- client/src/store/collectionStore.ts | 7 +++++-- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/client/src/api/collections.test.ts b/client/src/api/collections.test.ts index 8bb4875e4..dd0ff7f34 100644 --- a/client/src/api/collections.test.ts +++ b/client/src/api/collections.test.ts @@ -59,23 +59,26 @@ describe('collectionsApi', () => { expect(res.collection.name).toBe('Tokyo') }) - it('FE-API-COLLECTIONS-003: create() posts the create payload', async () => { - server.use(http.post(BASE, record({ collection }))) + // Both handlers answer with the BARE collection, the way the controller does — + // stubbing a { collection } envelope here is what let the client keep reading + // `.collection` off a response that never had one. + it('FE-API-COLLECTIONS-003: create() posts the create payload and returns the list itself', async () => { + server.use(http.post(BASE, record(collection))) const res = await collectionsApi.create({ name: 'Tokyo', color: '#111827' }) expect(requestBody).toEqual({ name: 'Tokyo', color: '#111827' }) - expect(res.collection.id).toBe(1) + expect(res.id).toBe(1) }) it('FE-API-COLLECTIONS-004: update() patches the list by id', async () => { - server.use(http.patch(`${BASE}/:id`, record({ collection }))) + server.use(http.patch(`${BASE}/:id`, record(collection))) const res = await collectionsApi.update(1, { name: 'Tokyo 2026' }) expect(requestUrl).toContain(`${BASE}/1`) expect(requestBody).toEqual({ name: 'Tokyo 2026' }) - expect(res.collection).toEqual(collection) + expect(res).toEqual(collection) }) it('FE-API-COLLECTIONS-005: uploadCover() posts multipart to the cover endpoint', async () => { diff --git a/client/src/api/collections.ts b/client/src/api/collections.ts index be4b4c085..da110e20c 100644 --- a/client/src/api/collections.ts +++ b/client/src/api/collections.ts @@ -53,9 +53,13 @@ export const collectionsApi = { ax.get(base).then((r: AxiosResponse) => r.data), get: (id: number): Promise => ax.get(`${base}/${id}`).then((r: AxiosResponse) => r.data), - create: (body: CollectionCreateRequest): Promise<{ collection: Collection }> => + // Both answer with the bare collection, not a { collection } envelope — the + // controller returns the service's `Collection` straight through, and the e2e + // suite pins that shape. Declaring the envelope made createCollection() resolve + // undefined for every caller. + create: (body: CollectionCreateRequest): Promise => ax.post(base, body satisfies CollectionCreateRequest).then((r: AxiosResponse) => r.data), - update: (id: number, body: CollectionUpdateRequest): Promise<{ collection: Collection }> => + update: (id: number, body: CollectionUpdateRequest): Promise => ax.patch(`${base}/${id}`, body satisfies CollectionUpdateRequest).then((r: AxiosResponse) => r.data), uploadCover: (id: number, formData: FormData): Promise => postMultipart(`${base}/${id}/cover`, formData), diff --git a/client/src/store/collectionStore.test.ts b/client/src/store/collectionStore.test.ts index 8db449557..ec7858780 100644 --- a/client/src/store/collectionStore.test.ts +++ b/client/src/store/collectionStore.test.ts @@ -69,8 +69,10 @@ beforeEach(() => { vi.clearAllMocks() vi.mocked(collectionsApi.list).mockResolvedValue(listResponse()) vi.mocked(collectionsApi.get).mockResolvedValue(detail()) - vi.mocked(collectionsApi.create).mockResolvedValue({ collection: listA }) - vi.mocked(collectionsApi.update).mockResolvedValue({ collection: listA }) + // The bare collection is what POST /addons/collections actually answers with; + // mocking a { collection } envelope here is what hid the bug this pins. + vi.mocked(collectionsApi.create).mockResolvedValue(listA) + vi.mocked(collectionsApi.update).mockResolvedValue(listA) vi.mocked(collectionsApi.uploadCover).mockResolvedValue(listA) vi.mocked(collectionsApi.remove).mockResolvedValue({ success: true }) vi.mocked(collectionsApi.reorder).mockResolvedValue({ success: true }) @@ -289,11 +291,20 @@ describe('collectionStore — list CRUD', () => { }) it('FE-STORE-COLLECTION-014: createCollection() returns null when the response carries no list', async () => { - vi.mocked(collectionsApi.create).mockResolvedValue({} as unknown as { collection: Collection }) + vi.mocked(collectionsApi.create).mockResolvedValue(undefined as unknown as Collection) expect(await store().createCollection({ name: 'Tokyo' })).toBeNull() }) + // The API declared a { collection } envelope the server never sends, so the id + // never reached the caller: a cover picked while creating a list was dropped + // and the new list never opened. + it('FE-STORE-COLLECTION-014b: createCollection() hands back the id the server sent', async () => { + const created = await store().createCollection({ name: 'Tokyo' }) + + expect(created?.id).toBe(1) + }) + it('FE-STORE-COLLECTION-015: updateCollection() reloads the detail when the list is active', async () => { useCollectionStore.setState({ activeId: 1 }) diff --git a/client/src/store/collectionStore.ts b/client/src/store/collectionStore.ts index 3b7772912..265c558fa 100644 --- a/client/src/store/collectionStore.ts +++ b/client/src/store/collectionStore.ts @@ -210,9 +210,12 @@ export const useCollectionStore = create((set, get) => ({ }, createCollection: async (payload) => { - const data = await collectionsApi.create(payload) + // POST answers with the collection itself. Reading `.collection` off it + // returned undefined every time, so the caller never got the new id: a cover + // picked while creating a list was dropped and the list never opened. + const created = await collectionsApi.create(payload) await get().loadAll() - return data.collection ?? null + return created ?? null }, updateCollection: async (id, updates) => {