fix(collections): read the created list off the response the server actually sends

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.
This commit is contained in:
Maurice
2026-09-05 12:53:39 +02:00
committed by Maurice
parent 3e532aab99
commit f75e9e9af8
4 changed files with 33 additions and 12 deletions
+8 -5
View File
@@ -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 () => {
+6 -2
View File
@@ -53,9 +53,13 @@ export const collectionsApi = {
ax.get(base).then((r: AxiosResponse) => r.data),
get: (id: number): Promise<CollectionDetailResponse> =>
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<Collection> =>
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<Collection> =>
ax.patch(`${base}/${id}`, body satisfies CollectionUpdateRequest).then((r: AxiosResponse) => r.data),
uploadCover: (id: number, formData: FormData): Promise<Collection> =>
postMultipart(`${base}/${id}/cover`, formData),
+14 -3
View File
@@ -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 })
+5 -2
View File
@@ -210,9 +210,12 @@ export const useCollectionStore = create<CollectionState>((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) => {