From 9a5816a45818be55de3231a2b98d1c817b33ce2a Mon Sep 17 00:00:00 2001 From: sripwoud Date: Tue, 18 Aug 2026 17:53:19 +0200 Subject: [PATCH] fix(web): import all selected files, not just the first The file picker allows multi-select but the import mutation only ever uploaded files[0], silently dropping the rest. upload-import is one-file-per-call and async: it returns Pending immediately and the real counts only exist at /import-progress/:id, so summing upload responses would aggregate zeros. importFiles() loops the selection sequentially: upload, poll to a terminal status, merge counts and failed_details into an aggregate. A file that fails upload or polling becomes a synthetic failure entry (labelled with the file name) instead of aborting the remaining uploads; only when every upload transport-fails does it throw so the existing toast-and-reset path still handles total failure. Upload progress is byte-weighted across the whole selection so the bar never resets between files. An AbortSignal wired to component unmount replaces the deleted setInterval cleanup so polling cannot outlive the page. Fixes #2 --- .../import/__tests__/import-files.test.ts | 297 ++++++++++++++++++ web/src/features/import/import-files.ts | 192 +++++++++++ web/src/features/import/index.tsx | 68 ++-- 3 files changed, 514 insertions(+), 43 deletions(-) create mode 100644 web/src/features/import/__tests__/import-files.test.ts create mode 100644 web/src/features/import/import-files.ts diff --git a/web/src/features/import/__tests__/import-files.test.ts b/web/src/features/import/__tests__/import-files.test.ts new file mode 100644 index 0000000..fd70b1d --- /dev/null +++ b/web/src/features/import/__tests__/import-files.test.ts @@ -0,0 +1,297 @@ +import { describe, it, expect, vi } from 'vitest' +import type { ImportProgress } from '@/api/import/api' +import { importFiles, type ImportFilesDeps } from '../import-files' + +const makeFile = (name: string, size: number) => + new File([new Uint8Array(size)], name) + +const prog = (over: Partial = {}): ImportProgress => ({ + import_id: 'imp_1', + status: 'Completed', + format: 'eml', + total: 1, + success: 1, + duplicates: 0, + failed: 0, + failed_details: [], + ...over, +}) + +const makeDeps = (over: Partial = {}): ImportFilesDeps => ({ + upload: vi.fn(async () => prog()), + getProgress: vi.fn(async () => prog()), + onUploadPct: vi.fn(), + onProgress: vi.fn(), + onPhase: vi.fn(), + pollIntervalMs: 0, + ...over, +}) + +describe('importFiles', () => { + it('uploads every selected file, in order', async () => { + const files = [ + makeFile('a.eml', 10), + makeFile('b.eml', 10), + makeFile('c.eml', 10), + ] + const upload = vi.fn(async (file: File) => prog({ import_id: file.name })) + const deps = makeDeps({ upload }) + + await importFiles(files, deps) + + expect(upload).toHaveBeenCalledTimes(3) + expect(upload.mock.calls.map(([f]) => f.name)).toEqual([ + 'a.eml', + 'b.eml', + 'c.eml', + ]) + }) + + it('reports uploading then processing for each file', async () => { + const files = [makeFile('a.eml', 10), makeFile('b.eml', 10)] + const deps = makeDeps({ + upload: vi.fn(async () => + prog({ status: 'Pending', total: 0, success: 0 }) + ), + }) + + await importFiles(files, deps) + + expect(vi.mocked(deps.onPhase).mock.calls.map(([p]) => p)).toEqual([ + 'uploading', + 'processing', + 'uploading', + 'processing', + ]) + }) + + it('aggregates counts across files', async () => { + const files = [makeFile('a.mbox', 10), makeFile('b.mbox', 10)] + const results: Record = { + 'a.mbox': prog({ + import_id: 'a', + total: 3, + success: 2, + failed: 1, + duplicates: 1, + }), + 'b.mbox': prog({ import_id: 'b', total: 2, success: 2 }), + } + const deps = makeDeps({ + upload: vi.fn(async (file: File) => results[file.name]), + }) + + const result = await importFiles(files, deps) + + expect(result.total).toBe(5) + expect(result.success).toBe(4) + expect(result.failed).toBe(1) + expect(result.duplicates).toBe(1) + expect(result.status).toBe('Completed') + }) + + it('polls until the import reaches a terminal status', async () => { + const files = [makeFile('a.mbox', 10)] + const polls = [ + prog({ status: 'Processing', total: 5, success: 2 }), + prog({ status: 'Completed', total: 5, success: 5 }), + ] + const getProgress = vi.fn(async () => polls.shift()!) + const deps = makeDeps({ + upload: vi.fn(async () => + prog({ status: 'Pending', total: 0, success: 0 }) + ), + getProgress, + }) + + const result = await importFiles(files, deps) + + expect(getProgress).toHaveBeenCalledTimes(2) + expect(result.success).toBe(5) + const seen = vi.mocked(deps.onProgress).mock.calls.map(([p]) => p.success) + expect(seen).toContain(2) + }) + + it('continues past a failed upload and surfaces its error', async () => { + const files = [makeFile('bad.eml', 10), makeFile('good.eml', 10)] + const upload = vi.fn(async (file: File) => { + if (file.name === 'bad.eml') { + throw { response: { data: { message: 'not a valid email file' } } } + } + return prog({ import_id: 'good' }) + }) + const deps = makeDeps({ upload }) + + const result = await importFiles(files, deps) + + expect(upload).toHaveBeenCalledTimes(2) + expect(result.total).toBe(2) + expect(result.success).toBe(1) + expect(result.failed).toBe(1) + expect(result.failed_details).toHaveLength(1) + expect(result.failed_details[0].error_message).toContain('bad.eml') + expect(result.failed_details[0].error_message).toContain( + 'not a valid email file' + ) + expect(result.status).toBe('Completed') + }) + + it('throws when every upload fails, so the caller can toast and reset', async () => { + const files = [makeFile('a.eml', 10), makeFile('b.eml', 10)] + const deps = makeDeps({ + upload: vi.fn(async () => { + throw { response: { data: { message: 'server unreachable' } } } + }), + }) + + await expect(importFiles(files, deps)).rejects.toThrow('server unreachable') + }) + + it('reports cumulative upload progress that never resets across files', async () => { + const files = [makeFile('a.eml', 100), makeFile('b.eml', 300)] + const deps = makeDeps({ + upload: vi.fn(async (_file: File, onPct: (pct: number) => void) => { + onPct(50) + onPct(100) + return prog() + }), + }) + + await importFiles(files, deps) + + const seen = vi.mocked(deps.onUploadPct).mock.calls.map(([pct]) => pct) + expect(seen.length).toBeGreaterThan(0) + for (let i = 1; i < seen.length; i++) { + expect(seen[i]).toBeGreaterThanOrEqual(seen[i - 1]) + } + expect(seen[seen.length - 1]).toBe(100) + }) + + it('labels failed details with the file name only for multi-file selections', async () => { + const failing = (id: string) => + prog({ + import_id: id, + total: 2, + success: 1, + failed: 1, + failed_details: [{ index: 0, error_message: 'bad message' }], + }) + + const multi = await importFiles( + [makeFile('a.mbox', 10), makeFile('b.mbox', 10)], + makeDeps({ upload: vi.fn(async (file: File) => failing(file.name)) }) + ) + expect(multi.failed_details.map((d) => d.error_message)).toEqual([ + 'a.mbox: bad message', + 'b.mbox: bad message', + ]) + + const single = await importFiles( + [makeFile('a.mbox', 10)], + makeDeps({ upload: vi.fn(async () => failing('a')) }) + ) + expect(single.failed_details.map((d) => d.error_message)).toEqual([ + 'bad message', + ]) + }) + + it('gives up on a file after repeated poll errors and continues', async () => { + const files = [makeFile('a.mbox', 10), makeFile('b.eml', 10)] + const upload = vi.fn(async (file: File) => + file.name === 'a.mbox' + ? prog({ import_id: 'a', status: 'Pending', total: 0, success: 0 }) + : prog({ import_id: 'b' }) + ) + const getProgress = vi.fn(async () => { + throw new Error('network down') + }) + const deps = makeDeps({ upload, getProgress }) + + const result = await importFiles(files, deps) + + expect(upload).toHaveBeenCalledTimes(2) + expect(result.success).toBe(1) + expect(result.failed).toBe(1) + expect(result.failed_details[0].error_message).toContain('a.mbox') + expect(result.status).toBe('Completed') + }) + + it('records the file position as the index of a synthetic failure', async () => { + const files = [ + makeFile('a.eml', 10), + makeFile('bad.eml', 10), + makeFile('c.eml', 10), + ] + const deps = makeDeps({ + upload: vi.fn(async (file: File) => { + if (file.name === 'bad.eml') throw new Error('boom') + return prog() + }), + }) + + const result = await importFiles(files, deps) + + expect(result.failed_details).toHaveLength(1) + expect(result.failed_details[0].index).toBe(1) + }) + + it('is Failed when the server reports a fatal failure with zero counts', async () => { + const files = [makeFile('a.mbox', 10)] + const deps = makeDeps({ + upload: vi.fn(async () => + prog({ + status: 'Failed', + total: 0, + success: 0, + failed: 0, + failed_details: [{ index: 0, error_message: 'mailbox not found' }], + }) + ), + }) + + const result = await importFiles(files, deps) + + expect(result.status).toBe('Failed') + }) + + it('stops uploading and polling once aborted', async () => { + const controller = new AbortController() + const files = [makeFile('a.mbox', 10), makeFile('b.mbox', 10)] + const upload = vi.fn(async () => + prog({ status: 'Pending', total: 0, success: 0 }) + ) + const getProgress = vi.fn(async () => { + controller.abort() + return prog({ status: 'Processing', total: 5, success: 1 }) + }) + const deps = makeDeps({ upload, getProgress, signal: controller.signal }) + + await importFiles(files, deps) + + expect(upload).toHaveBeenCalledTimes(1) + expect(getProgress).toHaveBeenCalledTimes(1) + }) + + it('is Failed when nothing succeeded', async () => { + const files = [makeFile('a.mbox', 10)] + const deps = makeDeps({ + upload: vi.fn(async () => + prog({ + status: 'Failed', + total: 2, + success: 0, + failed: 2, + failed_details: [ + { index: 0, error_message: 'parse error' }, + { index: 1, error_message: 'parse error' }, + ], + }) + ), + }) + + const result = await importFiles(files, deps) + + expect(result.status).toBe('Failed') + expect(result.failed).toBe(2) + }) +}) diff --git a/web/src/features/import/import-files.ts b/web/src/features/import/import-files.ts new file mode 100644 index 0000000..708c934 --- /dev/null +++ b/web/src/features/import/import-files.ts @@ -0,0 +1,192 @@ +// +// Copyright (c) 2025-2026 rustmailer.com (https://rustmailer.com) +// +// This file is part of the Bichon Email Archiving Project +import type { ImportProgress } from '@/api/import/api' + +export interface ImportFilesDeps { + upload: (file: File, onPct: (pct: number) => void) => Promise + getProgress: (importId: string) => Promise + onUploadPct: (pct: number) => void + onProgress: (progress: ImportProgress) => void + onPhase: (phase: 'uploading' | 'processing') => void + signal?: AbortSignal + pollIntervalMs?: number +} + +const DEFAULT_POLL_INTERVAL_MS = 1000 +const MAX_POLL_ERRORS = 5 + +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) + +const isTerminal = (status: ImportProgress['status']) => + status === 'Completed' || status === 'Failed' + +function emptyProgress(): ImportProgress { + return { + import_id: '', + status: 'Completed', + format: '', + total: 0, + success: 0, + duplicates: 0, + failed: 0, + failed_details: [], + } +} + +export function errorMessage(err: unknown): string { + if (err && typeof err === 'object') { + const maybe = err as { + response?: { data?: { message?: unknown } } + message?: unknown + } + const serverMessage = maybe.response?.data?.message + if (typeof serverMessage === 'string' && serverMessage) return serverMessage + if (typeof maybe.message === 'string' && maybe.message) return maybe.message + } + return String(err) +} + +function mergeProgress( + aggregate: ImportProgress, + fileProgress: ImportProgress, + label: string | null +): ImportProgress { + return { + ...aggregate, + import_id: fileProgress.import_id || aggregate.import_id, + format: fileProgress.format || aggregate.format, + total: aggregate.total + fileProgress.total, + success: aggregate.success + fileProgress.success, + duplicates: aggregate.duplicates + fileProgress.duplicates, + failed: aggregate.failed + fileProgress.failed, + failed_details: [ + ...aggregate.failed_details, + ...fileProgress.failed_details.map((d) => + label ? { ...d, error_message: `${label}: ${d.error_message}` } : d + ), + ], + } +} + +function fileFailure(fileIndex: number, message: string): ImportProgress { + return { + ...emptyProgress(), + status: 'Failed', + total: 1, + failed: 1, + failed_details: [{ index: fileIndex, error_message: message }], + } +} + +async function waitForTerminal( + initial: ImportProgress, + getProgress: (importId: string) => Promise, + pollIntervalMs: number, + signal: AbortSignal | undefined, + onTick: (progress: ImportProgress) => void +): Promise { + let current = initial + let consecutiveErrors = 0 + while (!isTerminal(current.status)) { + await sleep(pollIntervalMs) + if (signal?.aborted) return current + try { + current = await getProgress(initial.import_id) + consecutiveErrors = 0 + onTick(current) + } catch (err) { + consecutiveErrors++ + if (consecutiveErrors > MAX_POLL_ERRORS) { + throw new Error( + `lost track of import progress (the import may still be running, check import history): ${errorMessage(err)}` + ) + } + } + } + return current +} + +export async function importFiles( + files: File[], + deps: ImportFilesDeps +): Promise { + const { upload, getProgress, onUploadPct, onProgress, onPhase, signal } = deps + const pollIntervalMs = deps.pollIntervalMs ?? DEFAULT_POLL_INTERVAL_MS + const totalBytes = files.reduce((sum, f) => sum + f.size, 0) + const label = files.length > 1 ? (file: File) => file.name : () => null + + let aggregate = emptyProgress() + let uploadedBytes = 0 + let transportFailures = 0 + let sawFailure = false + + for (const [index, file] of files.entries()) { + if (signal?.aborted) break + + const startBytes = uploadedBytes + const reportPct = (filePct: number) => { + if (totalBytes === 0) return + const bytes = startBytes + (filePct / 100) * file.size + onUploadPct(Math.min(100, Math.round((bytes / totalBytes) * 100))) + } + + onPhase('uploading') + let initial: ImportProgress + try { + initial = await upload(file, reportPct) + } catch (err) { + transportFailures++ + sawFailure = true + aggregate = mergeProgress( + aggregate, + fileFailure(index, `${file.name}: ${errorMessage(err)}`), + null + ) + uploadedBytes = startBytes + file.size + reportPct(100) + onProgress(aggregate) + continue + } + uploadedBytes = startBytes + file.size + reportPct(100) + + onPhase('processing') + let final: ImportProgress + try { + final = await waitForTerminal( + initial, + getProgress, + pollIntervalMs, + signal, + (current) => onProgress(mergeProgress(aggregate, current, label(file))) + ) + } catch (err) { + sawFailure = true + aggregate = mergeProgress( + aggregate, + fileFailure(index, `${file.name}: ${errorMessage(err)}`), + null + ) + onProgress(aggregate) + continue + } + if (final.status === 'Failed' || final.failed > 0) sawFailure = true + aggregate = mergeProgress(aggregate, final, label(file)) + onProgress(aggregate) + } + + if (files.length > 0 && transportFailures === files.length) { + throw new Error( + aggregate.failed_details[0]?.error_message ?? 'Upload failed' + ) + } + + const result: ImportProgress = { + ...aggregate, + status: aggregate.success === 0 && sawFailure ? 'Failed' : 'Completed', + } + onProgress(result) + return result +} diff --git a/web/src/features/import/index.tsx b/web/src/features/import/index.tsx index 0376a35..fdeac07 100644 --- a/web/src/features/import/index.tsx +++ b/web/src/features/import/index.tsx @@ -45,6 +45,7 @@ import { import { get_system_configurations } from '@/api/system/api'; import { list_mailboxes } from '@/api/mailbox/api'; import { extractFolderHint, type FolderHint } from './folder-hint'; +import { importFiles, errorMessage } from './import-files'; const MAX_EML = 100 * 1024 * 1024; // 100 MB (hardcoded) const DEFAULT_MAX_MBOX = 1024 * 1024 * 1024; // 1 GB (fallback; actual limit from server settings) @@ -104,7 +105,6 @@ export default function ImportPage() { const [folder, setFolder] = useState('INBOX'); const [files, setFiles] = useState([]); const [dragging, setDragging] = useState(false); - // const [importId, setImportId] = useState(null); const [progress, setProgress] = useState(null); const [uploadPct, setUploadPct] = useState(0); const [phase, setPhase] = useState<'idle' | 'uploading' | 'processing' | 'done'>('idle'); @@ -117,7 +117,11 @@ export default function ImportPage() { // Combobox state for account selection const [accountOpen, setAccountOpen] = useState(false); - const pollRef = useRef | null>(null); + const abortRef = useRef(null); + + useEffect(() => { + return () => { abortRef.current?.abort(); }; + }, []); const { data: accounts = [] } = useQuery({ queryKey: ['nosync-accounts'], @@ -168,33 +172,6 @@ export default function ImportPage() { } })(); - const startPolling = useCallback((id: string) => { - if (pollRef.current) clearInterval(pollRef.current); - let retries = 0; - pollRef.current = setInterval(async () => { - try { - const p = await get_import_progress(id); - setProgress(p); - retries = 0; - if (p.status === 'Completed' || p.status === 'Failed') { - if (pollRef.current) clearInterval(pollRef.current); - setPhase('done'); - refetchHistory(); - } - } catch { - retries++; - if (retries > 5) { - if (pollRef.current) clearInterval(pollRef.current); - setPhase('idle'); - } - } - }, 1000); - }, [refetchHistory]); - - useEffect(() => { - return () => { if (pollRef.current) clearInterval(pollRef.current); }; - }, []); - const handleFiles = useCallback(async (newFiles: FileList | File[]) => { const arr = Array.from(newFiles) as File[]; const queued: QueuedFile[] = arr.map((f) => { @@ -209,7 +186,6 @@ export default function ImportPage() { setFiles(queued); setPhase('idle'); setProgress(null); - //setImportId(null); // Extract folder hint from the first valid file. // PST files are binary (OLE2) — headers can't be extracted in-browser. @@ -277,26 +253,32 @@ export default function ImportPage() { const importMutation = useMutation({ mutationFn: async () => { if (!accountId || !files.length) return; - const file = files[0].file; + const controller = new AbortController(); + abortRef.current = controller; setPhase('uploading'); setUploadPct(0); - const result = await upload_import( - Number(accountId), - effectiveFolder, - file.name, - file, - (pct) => setUploadPct(pct), + setProgress(null); + await importFiles( + files.map((q) => q.file), + { + upload: (file, onPct) => + upload_import(Number(accountId), effectiveFolder, file.name, file, onPct), + getProgress: get_import_progress, + onUploadPct: setUploadPct, + onProgress: setProgress, + onPhase: setPhase, + signal: controller.signal, + }, ); - //setImportId(result.import_id); - setProgress(result); - setPhase('processing'); - startPolling(result.import_id); + setPhase('done'); + refetchHistory(); }, - onError: (err: any) => { + onError: (err: unknown) => { setPhase('idle'); + setProgress(null); toast({ title: t('common.failed'), - description: err?.response?.data?.message || err.message, + description: errorMessage(err), variant: 'destructive', }); },