From 3f4d4b8643c7207d6a9afc233dd05cc249b52fca Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Mon, 6 Jul 2026 02:01:43 +0900 Subject: [PATCH] fix(transfer): persist queue when clearing completed/failed/all (#4947) Clear Completed, Clear Failed and Clear All mutated the Zustand store directly, so the cleared items were never written back to localStorage. On the next load the persisted queue restored them and they reappeared in the Transfer Queue panel. Route these clears through transferManager (like clearPending already does) so each calls persistQueue() after mutating the store. Co-authored-by: Claude Opus 4.8 (1M context) --- .../services/transfer-manager.test.ts | 84 +++++++++++++++++++ .../readest-app/src/hooks/useTransferQueue.ts | 6 +- .../src/services/transferManager.ts | 15 ++++ 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/apps/readest-app/src/__tests__/services/transfer-manager.test.ts b/apps/readest-app/src/__tests__/services/transfer-manager.test.ts index 140cfe5b..d94c55e0 100644 --- a/apps/readest-app/src/__tests__/services/transfer-manager.test.ts +++ b/apps/readest-app/src/__tests__/services/transfer-manager.test.ts @@ -527,6 +527,90 @@ describe('TransferManager', () => { }); }); + // ── clearCompleted ─────────────────────────────────────────────── + describe('clearCompleted', () => { + test('removes completed transfers, keeps others, and persists the queue', async () => { + const book1 = makeBook({ hash: 'h1', title: 'B1' }); + const book2 = makeBook({ hash: 'h2', title: 'B2' }); + const appService = makeAppService(); + await transferManager.initialize( + appService as never, + () => [book1, book2], + vi.fn(), + translationFn, + ); + + transferManager.pauseQueue(); + const id1 = transferManager.queueUpload(book1)!; + const id2 = transferManager.queueDownload(book2)!; + useTransferStore.getState().setTransferStatus(id1, 'completed'); + + transferManager.clearCompleted(); + + expect(useTransferStore.getState().transfers[id1]).toBeUndefined(); + expect(useTransferStore.getState().transfers[id2]).toBeDefined(); + + const stored = JSON.parse(localStorage.getItem('readest_transfer_queue')!); + expect(stored.transfers[id1]).toBeUndefined(); + expect(stored.transfers[id2]).toBeDefined(); + }); + }); + + // ── clearFailed ────────────────────────────────────────────────── + describe('clearFailed', () => { + test('removes failed/cancelled transfers, keeps others, and persists the queue', async () => { + const book1 = makeBook({ hash: 'h1', title: 'B1' }); + const book2 = makeBook({ hash: 'h2', title: 'B2' }); + const appService = makeAppService(); + await transferManager.initialize( + appService as never, + () => [book1, book2], + vi.fn(), + translationFn, + ); + + transferManager.pauseQueue(); + const id1 = transferManager.queueUpload(book1)!; + const id2 = transferManager.queueDownload(book2)!; + useTransferStore.getState().setTransferStatus(id1, 'failed', 'boom'); + + transferManager.clearFailed(); + + expect(useTransferStore.getState().transfers[id1]).toBeUndefined(); + expect(useTransferStore.getState().transfers[id2]).toBeDefined(); + + const stored = JSON.parse(localStorage.getItem('readest_transfer_queue')!); + expect(stored.transfers[id1]).toBeUndefined(); + expect(stored.transfers[id2]).toBeDefined(); + }); + }); + + // ── clearAll ───────────────────────────────────────────────────── + describe('clearAll', () => { + test('removes every transfer and persists the empty queue', async () => { + const book1 = makeBook({ hash: 'h1', title: 'B1' }); + const book2 = makeBook({ hash: 'h2', title: 'B2' }); + const appService = makeAppService(); + await transferManager.initialize( + appService as never, + () => [book1, book2], + vi.fn(), + translationFn, + ); + + transferManager.pauseQueue(); + transferManager.queueUpload(book1); + transferManager.queueDownload(book2); + + transferManager.clearAll(); + + expect(Object.keys(useTransferStore.getState().transfers)).toHaveLength(0); + + const stored = JSON.parse(localStorage.getItem('readest_transfer_queue')!); + expect(Object.keys(stored.transfers)).toHaveLength(0); + }); + }); + // ── Queue processing (integration-style) ───────────────────────── describe('queue processing', () => { test('successful upload calls appService.uploadBook and updates book', async () => { diff --git a/apps/readest-app/src/hooks/useTransferQueue.ts b/apps/readest-app/src/hooks/useTransferQueue.ts index 609e4963..9601f279 100644 --- a/apps/readest-app/src/hooks/useTransferQueue.ts +++ b/apps/readest-app/src/hooks/useTransferQueue.ts @@ -67,11 +67,11 @@ export function useTransferQueue(libraryLoaded = true, delayInit = 0) { }, []); const clearCompleted = useCallback(() => { - useTransferStore.getState().clearCompleted(); + transferManager.clearCompleted(); }, []); const clearFailed = useCallback(() => { - useTransferStore.getState().clearFailed(); + transferManager.clearFailed(); }, []); const clearPending = useCallback(() => { @@ -79,7 +79,7 @@ export function useTransferQueue(libraryLoaded = true, delayInit = 0) { }, []); const clearAll = useCallback(() => { - useTransferStore.getState().clearAll(); + transferManager.clearAll(); }, []); const getTransferProgress = useCallback((bookHash: string, type: TransferType) => { diff --git a/apps/readest-app/src/services/transferManager.ts b/apps/readest-app/src/services/transferManager.ts index ff1918e2..b2e77a94 100644 --- a/apps/readest-app/src/services/transferManager.ts +++ b/apps/readest-app/src/services/transferManager.ts @@ -259,6 +259,21 @@ class TransferManager { this.persistQueue(); } + clearCompleted(): void { + useTransferStore.getState().clearCompleted(); + this.persistQueue(); + } + + clearFailed(): void { + useTransferStore.getState().clearFailed(); + this.persistQueue(); + } + + clearAll(): void { + useTransferStore.getState().clearAll(); + this.persistQueue(); + } + private async processQueue(): Promise { if (this.isProcessing) return;