From a09d71ed12c02672e54ea14c908d2dad92d3d90e Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 17:34:56 +0200 Subject: [PATCH] fix(storage): a phone migration that fails to start drops the rest of the queue and says so On the phone storage panel, when the operator accepted several category moves and the first migration failed to start (for example the server was busy), only that error was toasted. The other accepted moves stayed in a hidden queue with no hint, and started on their own later whenever the panel state happened to refresh. The desktop drops the rest of the queue, names it in a storage.migrate.queueDropped toast and refreshes the state, so the operator sees what did not move and saves again. Cause: useStoragePanel only dropped the queue when the dropQueueOnFailure option was set, and only the desktop set it. Fix: the option is gone and useStoragePanel always drops the rest of the queue on a failed start, toasts it and refreshes, for both shells. Red test: FE-ADMIN-STOR-PANEL-011 (useStoragePanel.test.ts) failed with "AssertionError: expected "vi.fn()" to be called 2 times, but got 1 times". Changed assertions in src/components/Admin/storage/useStoragePanel.test.ts: - FE-ADMIN-STOR-PANEL-011 (renamed from "by default a failed migration start only toasts and keeps the rest queued"): toast.error was called once, now twice; migrationQueue had length 1, now 0; refreshState was not called, now it is. - FE-ADMIN-STOR-PANEL-012: no assertion changed; it no longer passes the removed dropQueueOnFailure option. --- .../components/Admin/storage/AdminStoragePanel.tsx | 2 +- .../Admin/storage/useStoragePanel.test.ts | 12 ++++++------ .../components/Admin/storage/useStoragePanel.ts | 14 ++------------ 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/client/src/components/Admin/storage/AdminStoragePanel.tsx b/client/src/components/Admin/storage/AdminStoragePanel.tsx index 275af1c5a..b567e1cc8 100644 --- a/client/src/components/Admin/storage/AdminStoragePanel.tsx +++ b/client/src/components/Admin/storage/AdminStoragePanel.tsx @@ -96,7 +96,7 @@ export default function AdminStoragePanel(): React.ReactElement { const { admin, editing, setEditing, confirmRemove, setConfirmRemove, syncPrompt, setSyncPrompt, migratePrompt, closeMigratePrompt, migrationQueue, handleCancelMigration, handleRefreshStats, handleStartBackfill, handleCancelBackfill, loaded, - } = useStoragePanel({ dropQueueOnFailure: true }) + } = useStoragePanel() // Open/closed only: the render below and moveAndSave both recompute // computeMigrationCandidates(draft, state) fresh rather than reading the // candidates the prompt opened with, which go stale the moment the operator diff --git a/client/src/components/Admin/storage/useStoragePanel.test.ts b/client/src/components/Admin/storage/useStoragePanel.test.ts index b446e18e6..e1216908d 100644 --- a/client/src/components/Admin/storage/useStoragePanel.test.ts +++ b/client/src/components/Admin/storage/useStoragePanel.test.ts @@ -188,7 +188,7 @@ describe('useStoragePanel', () => { expect(result.current.migratePrompt).toBeNull(); }); - it('FE-ADMIN-STOR-PANEL-011: by default a failed migration start only toasts and keeps the rest queued', async () => { + it('FE-ADMIN-STOR-PANEL-011: a failed migration start toasts, drops the rest of the queue, and refreshes', async () => { fake.admin = makeAdmin(baseState(), movingDraft('files', 'covers')); vi.mocked(fake.admin.startMigration).mockResolvedValueOnce('busy'); const { result } = renderHook(() => useStoragePanel()); @@ -196,15 +196,15 @@ describe('useStoragePanel', () => { await result.current.loaded!.moveAndSave(); }); await waitFor(() => expect(toast.error).toHaveBeenCalledWith('busy')); - expect(toast.error).toHaveBeenCalledTimes(1); - expect(result.current.migrationQueue).toHaveLength(1); - expect(fake.admin.refreshState).not.toHaveBeenCalled(); + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(2)); + expect(result.current.migrationQueue).toHaveLength(0); + expect(fake.admin.refreshState).toHaveBeenCalled(); }); - it('FE-ADMIN-STOR-PANEL-012: with dropQueueOnFailure a failed start drops the rest, names it, and refreshes', async () => { + it('FE-ADMIN-STOR-PANEL-012: a failed start drops the rest, names it, and refreshes', async () => { fake.admin = makeAdmin(baseState(), movingDraft('files', 'covers')); vi.mocked(fake.admin.startMigration).mockResolvedValueOnce('busy'); - const { result } = renderHook(() => useStoragePanel({ dropQueueOnFailure: true })); + const { result } = renderHook(() => useStoragePanel()); await act(async () => { await result.current.loaded!.moveAndSave(); }); diff --git a/client/src/components/Admin/storage/useStoragePanel.ts b/client/src/components/Admin/storage/useStoragePanel.ts index 7f5955e7d..460d0badd 100644 --- a/client/src/components/Admin/storage/useStoragePanel.ts +++ b/client/src/components/Admin/storage/useStoragePanel.ts @@ -64,15 +64,6 @@ interface StorageEditing { mirror: { candidates: string[]; initialTargets: string[] }; } -interface UseStoragePanelOptions { - /** - * What a failed migration start does to the rest of the queue. When set, the - * remaining candidates are dropped, named in a second toast, and the state is - * refreshed; otherwise only the error is toasted and the rest stay queued. - */ - dropQueueOnFailure?: boolean; -} - /** * The storage admin panel behind both shells (the desktop sections and the phone cards * render their own markup over it): the backend editor, the removal confirm, the @@ -80,7 +71,7 @@ interface UseStoragePanelOptions { * accepted category migrations one at a time. Everything that needs the loaded state * and draft sits under `loaded`, which is null until both exist. */ -export function useStoragePanel({ dropQueueOnFailure = false }: UseStoragePanelOptions = {}) { +export function useStoragePanel() { const { t } = useTranslation(); const toast = useToast(); const admin = useStorageAdmin(t('common.error'), t('storage.saveConflict')); @@ -126,7 +117,7 @@ export function useStoragePanel({ dropQueueOnFailure = false }: UseStoragePanelO // rule spans backfills and migrations alike, so starting while one runs // would 409, and the queued candidate would be lost (no retry). // - // With dropQueueOnFailure, a failed startMigration POST does not strand + // A failed startMigration POST does not strand // `rest`: it was already dequeued into local state before the POST, so on // failure nothing would change `migrationQueue` or `admin.state` again and // the remaining candidates would sit invisibly, never migrated. The whole @@ -145,7 +136,6 @@ export function useStoragePanel({ dropQueueOnFailure = false }: UseStoragePanelO migrationStartInFlight.current = false; if (error) { toast.error(error); - if (!dropQueueOnFailure) return; if (rest.length > 0) { toast.error( t('storage.migrate.queueDropped', {