From cf09a71be6117f92bc1553926f7b5dab1f72fbab Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Sat, 24 Oct 2026 23:11:40 +0000 Subject: [PATCH] fix(cover): keep a cover staged in a slot that is locked again before save A cover staged after unlocking a locked slot, then re-locked before saving, was silently dropped. The save now leaves that slot open, writes the cover, then locks it again, and holds the page refetch until the locks are final. Closes #1915 --- .../detail/tabs/EditMetadataTab.vue | 46 +++++++++++--- .../tabs/__tests__/EditMetadataTab.spec.ts | 62 +++++++++++++++++++ .../__tests__/useMetadataLocks.spec.ts | 30 +++++++++ .../book/composables/useMetadataLocks.ts | 24 +++++-- 4 files changed, 146 insertions(+), 16 deletions(-) diff --git a/client/src/features/book/components/detail/tabs/EditMetadataTab.vue b/client/src/features/book/components/detail/tabs/EditMetadataTab.vue index 963bbd5f5..995954cbe 100644 --- a/client/src/features/book/components/detail/tabs/EditMetadataTab.vue +++ b/client/src/features/book/components/detail/tabs/EditMetadataTab.vue @@ -177,6 +177,7 @@ const { isLocked, isUpdating: isUpdatingLock, replace: replaceLocks, + persist: persistLocks, toggle, lockAll, unlockAll, @@ -343,30 +344,54 @@ const communityRatingLines = computed(() => form.communityRatings.map((rating) => formatCommunityRatingLine(rating, availableMetadataProviders.value ?? [])), ) +// Held while a save runs, so the parent's refetch reads the book after every write, locks included. +let heldCoverChanges: (CoverMedium | null)[] | null = null + +async function submit() { + if (submitDisabled.value) return + heldCoverChanges = [] + try { + await saveFormAndCovers() + } finally { + const held = new Set(heldCoverChanges) + heldCoverChanges = null + for (const medium of held) emit('coverChanged', medium) + } +} + /** * Locks here save with the form, so a pending cover whose slot is locked on the server can only be * written after the form has unlocked it. Every other pending cover is written first, and a failure - * there stops the form save. + * there stops the form save. A cover staged while its slot was unlocked here, then locked again, is + * still written: the form save leaves that slot open, and it is locked again once the write ends. */ -async function submit() { - if (submitDisabled.value) return +async function saveFormAndCovers() { const panel = coverPanel.value const pendingMedia = panel?.pendingMedia ?? [] const lockedOnServer = (medium: CoverMedium | null) => props.book.lockedFields.includes(coverLockField(medium)) if (panel && !(await panel.confirm(pendingMedia.filter((medium) => !lockedOnServer(medium))))) return + const writeAfterSave = pendingMedia.filter(lockedOnServer) + const relockFields: BookMetadataLockField[] = writeAfterSave.map(coverLockField).filter((field) => isLocked(field)) const locksChanged = locksDirty.value - const result = await save(props.book.id, lockedFields.value) + const result = await save( + props.book.id, + lockedFields.value.filter((field) => !relockFields.includes(field)), + ) if (!result) return markLocksPersisted(result.book.lockedFields) + if (relockFields.length > 0) await replaceLocks(props.book.id, [...result.book.lockedFields, ...relockFields], 'all') emit('saved', result.book) - if (locksChanged) emit('locksChanged', result.book.lockedFields) showSaveResultToast(result.write, result.libraryAutoWriteEnabled) - const unlockedBySave = pendingMedia.filter((medium) => lockedOnServer(medium) && !result.book.lockedFields.includes(coverLockField(medium))) - if (!panel || unlockedBySave.length === 0) return - await nextTick() - await panel.confirm(unlockedBySave) + const unlockedBySave = writeAfterSave.filter((medium) => !result.book.lockedFields.includes(coverLockField(medium))) + if (panel && unlockedBySave.length > 0) { + await nextTick() + await panel.confirm(unlockedBySave) + } + const relocked = relockFields.length > 0 ? await persistLocks(props.book.id) : null + if (relocked) emit('saved', relocked) + if (locksChanged) emit('locksChanged', (relocked ?? result.book).lockedFields) } function handleReset() { @@ -874,7 +899,8 @@ async function handleUnlockAll() { } function handleCoverChanged(medium: CoverMedium | null) { - emit('coverChanged', medium) + if (heldCoverChanges) heldCoverChanges.push(medium) + else emit('coverChanged', medium) } diff --git a/client/src/features/book/components/detail/tabs/__tests__/EditMetadataTab.spec.ts b/client/src/features/book/components/detail/tabs/__tests__/EditMetadataTab.spec.ts index ca59a9bf9..7264f58a8 100644 --- a/client/src/features/book/components/detail/tabs/__tests__/EditMetadataTab.spec.ts +++ b/client/src/features/book/components/detail/tabs/__tests__/EditMetadataTab.spec.ts @@ -412,6 +412,68 @@ describe('EditMetadataTab cover tiles', () => { expect(JSON.parse(String(patchCalls()[0]![1]!.body)).lockedFields).toEqual([]) }) + it('writes a cover staged while its locked slot was opened here, then locks the slot again', async () => { + const wrapper = mountTab(makeBook({ lockedFields: ['title', 'audioCover'] })) + await flushPromises() + const coverPanelStub = wrapper.getComponent(CoverEditorPanelStub) + coverPanelStub.vm.$emit('toggleLock', 'audioCover') + await flushPromises() + panel.hasPending.value = true + panel.pendingMedia.value = ['audio'] + coverPanelStub.vm.$emit('toggleLock', 'audioCover') + await flushPromises() + + const order: string[] = [] + panel.confirm.mockImplementation(async (media) => { + if (!media?.length) return true + order.push(`covers:${media.join(',')}`) + coverPanelStub.vm.$emit('coverChanged', 'audio') + return true + }) + const relockedBook = makeBook({ lockedFields: ['title', 'audioCover'] }) + vi.mocked(api).mockImplementation(async (url, init) => { + if (init?.method !== 'PATCH') return json([]) + const lockedFields = JSON.parse(String(init.body)).lockedFields + if (String(url).endsWith('/metadata-locks')) { + order.push(`locks:${lockedFields.join(',')}`) + return json(relockedBook) + } + order.push(`form:${lockedFields.join(',')}`) + return json({ book: makeBook({ lockedFields: ['title'] }), write: null, libraryAutoWriteEnabled: false }) + }) + + await saveButton(wrapper).trigger('click') + await flushPromises() + + expect(order).toEqual(['form:title', 'covers:audio', 'locks:title,audioCover']) + expect(wrapper.emitted('saved')!.at(-1)).toEqual([relockedBook]) + expect(wrapper.emitted('coverChanged')).toEqual([['audio']]) + expect(coverPanelStub.props('lockedFields')).toEqual(['title', 'audioCover']) + }) + + it('locks the slot again when the cover staged in it fails to save', async () => { + const wrapper = mountTab(makeBook({ lockedFields: ['audioCover'] })) + await flushPromises() + const coverPanelStub = wrapper.getComponent(CoverEditorPanelStub) + coverPanelStub.vm.$emit('toggleLock', 'audioCover') + await flushPromises() + panel.hasPending.value = true + panel.pendingMedia.value = ['audio'] + coverPanelStub.vm.$emit('toggleLock', 'audioCover') + await flushPromises() + panel.confirm.mockImplementation(async (media) => !media?.length) + savedBook = makeBook({ lockedFields: [] }) + + await saveButton(wrapper).trigger('click') + await flushPromises() + + expect(panel.confirm).toHaveBeenLastCalledWith(['audio']) + expect(patchCalls().map(([url, init]) => [String(url).split('/').at(-1), JSON.parse(String(init!.body)).lockedFields])).toEqual([ + ['metadata-and-locks', []], + ['metadata-locks', ['audioCover']], + ]) + }) + it('clears the unsaved covers with the rest of the form', async () => { const wrapper = mountTab(makeBook()) await flushPromises() diff --git a/client/src/features/book/composables/__tests__/useMetadataLocks.spec.ts b/client/src/features/book/composables/__tests__/useMetadataLocks.spec.ts index 5f3b98dc8..dfe51c5ad 100644 --- a/client/src/features/book/composables/__tests__/useMetadataLocks.spec.ts +++ b/client/src/features/book/composables/__tests__/useMetadataLocks.spec.ts @@ -174,6 +174,36 @@ describe('useMetadataLocks', () => { expect(locksDirty.value).toBe(false) }) + it('writes staged lock changes at once when asked, even in deferred mode', async () => { + const { load, toggle, persist, lockedFields, locksDirty } = useMetadataLocks({ deferred: true }) + load(makeBook()) + await toggle(1, 'cover') + apiMock.mockResolvedValueOnce({ ok: true, json: async () => makeBook({ lockedFields: ['cover'] }) }) + + const result = await persist(1) + + expect(apiMock).toHaveBeenCalledWith( + '/api/v1/books/1/metadata-locks', + expect.objectContaining({ method: 'PATCH', body: JSON.stringify({ lockedFields: ['cover'] }) }), + ) + expect(result?.lockedFields).toEqual(['cover']) + expect(lockedFields.value).toEqual(['cover']) + expect(locksDirty.value).toBe(false) + }) + + it('keeps staged lock changes when writing them at once fails', async () => { + const { load, toggle, persist, lockedFields, locksDirty, error } = useMetadataLocks({ deferred: true }) + load(makeBook()) + await toggle(1, 'cover') + apiMock.mockResolvedValueOnce({ ok: false, status: 500 }) + + expect(await persist(1)).toBeNull() + + expect(error.value).toBe('HTTP 500') + expect(lockedFields.value).toEqual(['cover']) + expect(locksDirty.value).toBe(true) + }) + it('marks deferred lock changes as persisted after a successful combined save', async () => { const { load, toggle, markPersisted, lockedFields, locksDirty } = useMetadataLocks({ deferred: true }) load(makeBook()) diff --git a/client/src/features/book/composables/useMetadataLocks.ts b/client/src/features/book/composables/useMetadataLocks.ts index 8729422d0..6c86f451b 100644 --- a/client/src/features/book/composables/useMetadataLocks.ts +++ b/client/src/features/book/composables/useMetadataLocks.ts @@ -53,19 +53,30 @@ export function useMetadataLocks(options: UseMetadataLocksOptions = {}) { bookId: number, nextLockedFields: readonly BookMetadataLockField[], scope: BookMetadataLockField | 'all', + ): Promise { + if (!options.deferred) return send(bookId, nextLockedFields, scope) + lockedFields.value = normalizeLockedFields(nextLockedFields) + error.value = null + return null + } + + /** Writes the staged locks now, for a caller whose locks otherwise wait for its form save. */ + function persist(bookId: number): Promise { + return send(bookId, lockedFields.value, 'all') + } + + async function send( + bookId: number, + nextLockedFields: readonly BookMetadataLockField[], + scope: BookMetadataLockField | 'all', ): Promise { updatingField.value = scope error.value = null try { - const normalized = normalizeLockedFields(nextLockedFields) - if (options.deferred) { - lockedFields.value = normalized - return null - } const res = await api(`/api/v1/books/${bookId}/metadata-locks`, { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ lockedFields: normalized }), + body: JSON.stringify({ lockedFields: normalizeLockedFields(nextLockedFields) }), }) if (!res.ok) throw new Error(`HTTP ${res.status}`) const updated: BookDetail = await res.json() @@ -108,6 +119,7 @@ export function useMetadataLocks(options: UseMetadataLocksOptions = {}) { isLocked, isUpdating, replace, + persist, toggle, lockAll, unlockAll,