diff --git a/src/store/downloadStore.ts b/src/store/downloadStore.ts index 6834d1e..0f5b423 100644 --- a/src/store/downloadStore.ts +++ b/src/store/downloadStore.ts @@ -177,11 +177,13 @@ const startDownloadListeners = async () => { mainStore.updateDownload(payload.id, updates); if (status === 'completed' || status === 'failed' || status === 'paused') { - mainStore.setPendingOrder(mainStore.pendingOrder.filter(id => id !== payload.id)); + useDownloadStore.setState(state => ({ + pendingOrder: state.pendingOrder.filter(id => id !== payload.id) + })); } else if (status === 'queued') { - if (!mainStore.pendingOrder.includes(payload.id)) { - mainStore.setPendingOrder([...mainStore.pendingOrder, payload.id]); - } + useDownloadStore.setState(state => state.pendingOrder.includes(payload.id) + ? {} + : { pendingOrder: [...state.pendingOrder, payload.id] }); } if (status === 'queued' || status === 'downloading' || status === 'processing' || status === 'retrying') { diff --git a/src/store/useDownloadStore.test.ts b/src/store/useDownloadStore.test.ts index adc8a9e..96176c0 100644 --- a/src/store/useDownloadStore.test.ts +++ b/src/store/useDownloadStore.test.ts @@ -523,6 +523,37 @@ describe('useDownloadStore', () => { .toEqual(['00000000-0000-0000-0000-000000000001', '00000000-0000-0000-0000-000000000001']); }); + it('moves persisted paused rows behind runnable rows and assigns contiguous positions', async () => { + vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => { + if (cmd === 'db_get_all_queues') { + return [JSON.stringify({ id: 'queue-a', name: 'Queue A', isMain: false })]; + } + if (cmd === 'db_get_all_downloads') { + return [ + JSON.stringify({ id: 'active', status: 'downloading', queueId: 'queue-a', queuePosition: 0 }), + JSON.stringify({ id: 'queued-one', status: 'queued', queueId: 'queue-a', queuePosition: 1 }), + JSON.stringify({ id: 'paused-one', status: 'paused', queueId: 'queue-a', queuePosition: 2 }), + JSON.stringify({ id: 'queued-two', status: 'queued', queueId: 'queue-a', queuePosition: 3 }), + JSON.stringify({ id: 'legacy-invalid', status: 'queued', queueId: 'queue-a', queuePosition: 'not-a-number' }) + ]; + } + return undefined; + }); + + await useDownloadStore.getState().initDB(); + + expect(useDownloadStore.getState().downloads + .filter(download => download.queueId === 'queue-a') + .sort((left, right) => (left.queuePosition ?? 0) - (right.queuePosition ?? 0)) + .map(download => download.id) + ).toEqual(['active', 'queued-one', 'queued-two', 'legacy-invalid', 'paused-one']); + expect(useDownloadStore.getState().downloads + .filter(download => download.queueId === 'queue-a') + .map(download => download.queuePosition) + .sort((left, right) => (left ?? 0) - (right ?? 0)) + ).toEqual([0, 1, 2, 3, 4]); + }); + it('removes persisted temporary media estimates that are smaller than downloaded bytes', async () => { vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => { if (cmd === 'db_get_all_queues') return []; @@ -1052,17 +1083,44 @@ describe('useDownloadStore', () => { downloads: [ { id: 'queued-one', url: 'http://one', fileName: 'one', status: 'queued', category: 'Other', dateAdded: '', queueId: 'pause-queue' }, { id: 'queued-two', url: 'http://two', fileName: 'two', status: 'queued', category: 'Other', dateAdded: '', queueId: 'pause-queue' }, + { id: 'staged-one', url: 'http://staged', fileName: 'staged', status: 'staged', category: 'Other', dateAdded: '', queueId: 'pause-queue' }, ] as any[], }); vi.mocked(ipc.invokeCommand).mockResolvedValue(undefined as never); - await expect(useDownloadStore.getState().pauseAll()).resolves.toBe(2); + await expect(useDownloadStore.getState().pauseAll()).resolves.toBe(3); expect( vi.mocked(ipc.invokeCommand).mock.calls.filter(([command]) => command === 'pause_download') - ).toHaveLength(2); + ).toHaveLength(3); expect(useDownloadStore.getState().downloads.every(item => item.status === 'paused')).toBe(true); }); + it('moves a paused row behind the remaining runnable queue rows', async () => { + useDownloadStore.setState({ + downloads: [ + { id: 'active', status: 'downloading', queueId: 'ordered-pause', queuePosition: 0 }, + { id: 'queued-one', status: 'queued', queueId: 'ordered-pause', queuePosition: 1 }, + { id: 'pause-target', status: 'queued', queueId: 'ordered-pause', queuePosition: 2 }, + { id: 'queued-two', status: 'queued', queueId: 'ordered-pause', queuePosition: 3 } + ] as any[], + pendingOrder: ['queued-one', 'pause-target', 'queued-two'] + }); + vi.mocked(ipc.invokeCommand).mockResolvedValue(undefined as never); + + await useDownloadStore.getState().pauseDownload('pause-target'); + + const ordered = useDownloadStore.getState().downloads + .filter(download => download.queueId === 'ordered-pause') + .sort((left, right) => (left.queuePosition ?? 0) - (right.queuePosition ?? 0)); + expect(ordered.map(download => download.id)).toEqual([ + 'active', 'queued-one', 'queued-two', 'pause-target' + ]); + expect(ordered.map(download => download.queuePosition)).toEqual([0, 1, 2, 3]); + expect(useDownloadStore.getState().pendingOrder).toEqual(['queued-one', 'queued-two']); + expect(useDownloadStore.getState().downloads.find(download => download.id === 'pause-target')?.status) + .toBe('paused'); + }); + it('does not let a queue pause lose a selected start in flight', async () => { useDownloadStore.setState({ downloads: [ @@ -1143,6 +1201,35 @@ describe('useDownloadStore', () => { expect(ipc.invokeCommand).not.toHaveBeenCalledWith('enqueue_download', expect.anything()); }); + it('inserts a newly staged queue item before paused rows', async () => { + useDownloadStore.setState({ + downloads: [{ + id: 'already-paused', + url: 'https://example.com/paused.bin', + fileName: 'paused.bin', + category: 'Other', + dateAdded: '', + status: 'paused', + queueId: 'queue-b', + queuePosition: 0 + }] as any[] + }); + + await useDownloadStore.getState().addDownload({ + id: 'new-staged', + url: 'https://example.com/new.bin', + fileName: 'new.bin', + category: 'Other', + dateAdded: '' + }, { type: 'add-to-queue', queueId: 'queue-b' }); + + const ordered = useDownloadStore.getState().downloads + .filter(item => item.queueId === 'queue-b') + .sort((left, right) => (left.queuePosition ?? 0) - (right.queuePosition ?? 0)); + expect(ordered.map(item => item.id)).toEqual(['new-staged', 'already-paused']); + expect(ordered.map(item => item.queuePosition)).toEqual([0, 1]); + }); + it('carries a media format estimate into numeric progress state', async () => { await useDownloadStore.getState().addDownload({ id: 'media-estimate', @@ -1766,7 +1853,8 @@ describe('useDownloadStore', () => { useDownloadStore.setState({ downloads: [ { id: 'ready', status: 'ready', queueId: 'old' }, - { id: 'done', status: 'completed', queueId: 'old' } + { id: 'done', status: 'completed', queueId: 'old' }, + { id: 'paused', status: 'paused', queueId: 'new', queuePosition: 0 } ] as any[] }); @@ -1774,6 +1862,11 @@ describe('useDownloadStore', () => { expect(useDownloadStore.getState().downloads.find(item => item.id === 'ready')?.queueId).toBe('new'); expect(useDownloadStore.getState().downloads.find(item => item.id === 'done')?.queueId).toBe('old'); + expect(useDownloadStore.getState().downloads + .filter(item => item.queueId === 'new') + .sort((left, right) => (left.queuePosition ?? 0) - (right.queuePosition ?? 0)) + .map(item => item.id) + ).toEqual(['ready', 'paused']); }); it('does not reassign an item that completes while queue assignment is awaiting cancellation', async () => { diff --git a/src/store/useDownloadStore.ts b/src/store/useDownloadStore.ts index 02068bb..49b74f6 100644 --- a/src/store/useDownloadStore.ts +++ b/src/store/useDownloadStore.ts @@ -126,8 +126,15 @@ const advanceQueueControlGeneration = (queueId: string): number => { const isCurrentQueueControlGeneration = (queueId: string, generation: number): boolean => currentQueueControlGeneration(queueId) === generation; +const comparableQueuePosition = (download: DownloadItem): number => { + const position = download.queuePosition; + return typeof position === 'number' && Number.isFinite(position) && position >= 0 + ? position + : Number.MAX_SAFE_INTEGER; +}; + const queuePositionComparator = (left: DownloadItem, right: DownloadItem): number => - (left.queuePosition ?? Number.MAX_SAFE_INTEGER) - (right.queuePosition ?? Number.MAX_SAFE_INTEGER) || + comparableQueuePosition(left) - comparableQueuePosition(right) || left.id.localeCompare(right.id); const queueItemsForReordering = (downloads: DownloadItem[], queueId: string): DownloadItem[] => @@ -161,6 +168,17 @@ const applyQueueOrder = ( : download); }; +// Paused downloads remain part of the queue, but they must not be interleaved +// with the queue's non-paused rows. Keep the partition in the store so +// the table, persistence, and later queue operations all observe the same +// order instead of each deriving a different one. +const reorderQueueWithPausedAtEnd = (downloads: DownloadItem[], queueId: string): DownloadItem[] => { + const queueItems = queueItemsForReordering(downloads, queueId); + const nonPausedItems = queueItems.filter(download => download.status !== 'paused'); + const pausedItems = queueItems.filter(download => download.status === 'paused'); + return applyQueueOrder(downloads, queueId, [...nonPausedItems, ...pausedItems]); +}; + const advanceDownloadLifecycle = (id: string): bigint => { const nextGeneration = (downloadLifecycleGenerations.get(id) ?? 0n) + 1n; downloadLifecycleGenerations.set(id, nextGeneration); @@ -519,16 +537,27 @@ const effectiveDestinationForItem = async ( const normalizeQueuePositions = (downloads: DownloadItem[]): DownloadItem[] => { const nextPosition = new Map(); - return downloads.map(download => { + const normalized: DownloadItem[] = downloads.map(download => { const queueId = download.queueId || MAIN_QUEUE_ID; const position = nextPosition.get(queueId) || 0; nextPosition.set(queueId, position + 1); + const persistedPosition = download.queuePosition; + const queuePosition = typeof persistedPosition === 'number' && + Number.isFinite(persistedPosition) && persistedPosition >= 0 + ? Math.trunc(persistedPosition) + : position; return { ...download, queueId, - queuePosition: download.queuePosition ?? position + queuePosition }; }); + + let ordered = normalized; + for (const queueId of new Set(normalized.map(download => download.queueId || MAIN_QUEUE_ID))) { + ordered = reorderQueueWithPausedAtEnd(ordered, queueId); + } + return ordered; }; const TEMPORARY_MEDIA_ESTIMATE_MAX_BYTES = 1024; @@ -1294,7 +1323,9 @@ export const useDownloadStore = create((set, get) => { hasBeenDispatched: false }; advanceDownloadLifecycle(item.id); - set((state) => ({ downloads: [...state.downloads, ownedItem] })); + set((state) => ({ + downloads: reorderQueueWithPausedAtEnd([...state.downloads, ownedItem], queueId) + })); if (action.type === 'add-to-queue') { info(`Download ${item.id} added to queue ${action.queueId}`); @@ -1334,19 +1365,32 @@ export const useDownloadStore = create((set, get) => { preemptDispatch ), updateDownload: (id, updates) => { - set((state) => ({ - downloads: state.downloads.map(d => { - if (d.id === id) { - const updated = { - ...d, + set((state) => { + const current = state.downloads.find(download => download.id === id); + if (!current) return { downloads: state.downloads }; + + const downloads = state.downloads.map(d => d.id === id + ? { + ...d, ...updates, fraction: updates.fraction !== undefined ? updates.fraction : d.fraction - }; - return updated; - } - return d; - }) - })); + } + : d + ); + const shouldNormalizeQueue = updates.status === 'paused' || + updates.status === 'queued' || + updates.status === 'staged'; + const updated = downloads.find(download => download.id === id) ?? current; + + return { + downloads: shouldNormalizeQueue + ? reorderQueueWithPausedAtEnd(downloads, updated.queueId || MAIN_QUEUE_ID) + : downloads, + ...(updates.status === 'paused' + ? { pendingOrder: state.pendingOrder.filter(value => value !== id) } + : {}) + }; + }); if (updates.status && ['completed', 'failed', 'paused'].includes(updates.status)) { info(`Download ${id} status changed to ${updates.status}`); @@ -1753,8 +1797,8 @@ export const useDownloadStore = create((set, get) => { ); const maxPos = queueItems.reduce((max, d) => Math.max(max, d.queuePosition ?? 0), -1); const nextPosition = maxPos + 1; - set(state => ({ - downloads: state.downloads.map(item => + set(state => { + const downloads = state.downloads.map(item => movableIds.has(item.id) && item.status !== 'completed' ? { ...item, @@ -1764,8 +1808,9 @@ export const useDownloadStore = create((set, get) => { hasBeenDispatched: false } : item - ) - })); + ); + return { downloads: reorderQueueWithPausedAtEnd(downloads, queueId) }; + }); }); }, setDownloadSpeedLimit: (id, limit) => runDownloadLifecycleOperation( @@ -2094,11 +2139,11 @@ export const useDownloadStore = create((set, get) => { // Reset interrupted active downloads to queued. set((state) => ({ - downloads: state.downloads.map(d => + downloads: normalizeQueuePositions(state.downloads.map(d => isActiveDownloadStatus(d.status) && d.status !== 'queued' ? { ...d, status: 'queued' as const } : d - ) + )) })); } catch (e) { diff --git a/src/utils/downloadActions.test.ts b/src/utils/downloadActions.test.ts index 02104a0..78a7150 100644 --- a/src/utils/downloadActions.test.ts +++ b/src/utils/downloadActions.test.ts @@ -12,13 +12,18 @@ import { } from './downloadActions'; describe('download action policy', () => { - it('keeps start and pause actions mutually exclusive', () => { + it('allows queued items to be paused before their first dispatch', () => { + expect(canStartDownload('staged')).toBe(true); + expect(canPauseDownload('staged')).toBe(true); + for (const status of ['ready', 'paused', 'failed'] as const) { expect(canStartDownload(status)).toBe(true); expect(canPauseDownload(status)).toBe(false); } - for (const status of ['queued', 'downloading', 'processing', 'retrying'] as const) { + for (const status of ['staged', 'queued', 'downloading', 'processing', 'retrying'] as const) { expect(canPauseDownload(status)).toBe(true); + } + for (const status of ['queued', 'downloading', 'processing', 'retrying'] as const) { expect(canStartDownload(status)).toBe(false); } }); @@ -37,9 +42,10 @@ describe('download action policy', () => { expect(getPauseResumeAction('retrying')).toBe('pause'); expect(getPauseResumeAction('paused')).toBe('resume'); - for (const status of ['ready', 'staged', 'completed', 'failed'] as const) { + for (const status of ['ready', 'completed', 'failed'] as const) { expect(getPauseResumeAction(status)).toBeNull(); } + expect(getPauseResumeAction('staged')).toBe('pause'); }); it('provides consistent labels and edit locks', () => { @@ -62,7 +68,7 @@ describe('download action policy', () => { { status: 'completed' }, ]); - expect(counts).toEqual({ pause: 2, resume: 4 }); + expect(counts).toEqual({ pause: 3, resume: 4 }); }); it('keeps large action badges compact without changing the accessible count', () => { diff --git a/src/utils/downloadActions.ts b/src/utils/downloadActions.ts index 718b5b4..5b0d486 100644 --- a/src/utils/downloadActions.ts +++ b/src/utils/downloadActions.ts @@ -8,6 +8,7 @@ const STARTABLE_STATUSES: ReadonlySet = new Set([ ]); const PAUSABLE_STATUSES: ReadonlySet = new Set([ + 'staged', 'queued', 'downloading', 'processing',