🐛 Address review: fix spinner hang + show processing failures

Blocker: the "every file rejected" reset never fired because the backend
returns `upload_id` unconditionally (with count 0), so `anyQueued` was
always true and the completion effect (gated on total > 0) never ran —
modal spun forever. Gate `anyQueued` on `count > 0` so a zero-photo
response takes the terminal reset path.

Concern 1: processing-stage failures were invisible — the modal
auto-closed on clean transfer before the worker reported them. Defer the
settle/close decision to the completion effect (combining transfer +
processing failures), and persist failed photos into `processingFailures`
state before `uploadIds` is cleared, so the rows don't vanish the instant
they appear.

Also: report card gets role="status"/aria-live (nit), and tests now cover
the whole-chunk transfer failure, the clean-settle path, and the
onUploadSettled contract from the real component.

frontend/src/components/admin/PhotoUpload.tsx

@@ -68,6 +68,11 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
   // validation rejections (response.errors) and whole-chunk failures.
   // Processing failures are merged in from the progress hook below.
   const [transferFailures, setTransferFailures] = useState<UploadFailure[]>([]);
+  // Processing failures are captured into state (not read live) because the
+  // completion effect clears uploadIds, which empties the progress hook's
+  // failedPhotos — reading live would make the rows vanish the instant they
+  // appear.
+  const [processingFailures, setProcessingFailures] = useState<UploadFailure[]>([]);
   const [failuresDismissed, setFailuresDismissed] = useState(false);
   const fileInputRef = useRef<HTMLInputElement>(null);
 
@@ -76,16 +81,12 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
   });
 
   // Single source of truth for the "which files failed" report: transfer
-  // stage failures (collected during handleUpload) plus processing
+  // stage failures (collected during handleUpload) plus processing failures
-  // failures (live, from the progress hook). Both carry filename + reason.
+  // (captured on completion). Both carry filename + reason.
-  const failures = useMemo<UploadFailure[]>(() => {
+  const failures = useMemo<UploadFailure[]>(
-    const processing: UploadFailure[] = processingAggregate.failedPhotos.map((p) => ({
+    () => [...transferFailures, ...processingFailures],
-      filename: p.filename,
+    [transferFailures, processingFailures]
-      reason: p.error || t('upload.failures.unknownReason', 'Unknown error'),
+  );
-      kind: 'processing',
-    }));
-    return [...transferFailures, ...processing];
-  }, [transferFailures, processingAggregate.failedPhotos, t]);
   
   // Fetch categories for this event
   const { data: categories = [] } = useQuery({
@@ -203,6 +204,7 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
     setUploadIds([]);
     // Clear any prior failure report before this run.
     setTransferFailures([]);
+    setProcessingFailures([]);
     setFailuresDismissed(false);
 
     // For large uploads, chunk the files by both count AND size to prevent memory/network issues.
@@ -317,9 +319,12 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
               });
             }
           }
-          // Backend returns a per-request upload_id. Track it so the
+          // Track the per-request upload_id so the processing hook can poll
-          // processing-status hook can poll/stream live progress.
+          // for live progress — but only when photos were actually queued
-          if (response.data?.upload_id) {
+          // (count > 0). The backend returns an upload_id even when every
+          // file was rejected (count 0); tracking it there would make us wait
+          // for a processing phase that never starts, hanging the spinner.
+          if (response.data?.upload_id && (response.data?.count ?? 0) > 0) {
             anyQueued = true;
             const newId = response.data.upload_id as string;
             setUploadIds((prev) => (prev.includes(newId) ? prev : [...prev, newId]));
@@ -371,25 +376,20 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
         onUploadComplete();
       }
 
-      // Tell the host the transfer stage settled. A clean upload lets the
+      // Settling (and the modal's close decision) is deferred until we know
-      // modal auto-close (unchanged behaviour); a partial failure keeps it
+      // the WHOLE outcome, including background processing. If nothing was
-      // open so the failure report below stays visible.
+      // queued (every file rejected, or a pre-async backend), the transfer
-      onUploadSettled?.({ hasFailures: collected.length > 0 });
+      // stage is already terminal — settle now and reset the UI. Otherwise
-
+      // the processing effect below settles once the worker finishes, so
-      // Nothing was queued for background processing (every chunk failed,
+      // processing failures are included before the modal decides to close.
-      // or a pre-async backend) — there's no processing phase to wait on,
-      // so reset the transfer UI here. The failure report persists.
       if (!anyQueued) {
+        onUploadSettled?.({ hasFailures: collected.length > 0 });
         setIsUploading(false);
         setUploadProgress(0);
         setCurrentChunk(0);
         setTotalChunks(0);
         setPhase({ kind: 'idle' });
       }
-
-      // If the backend never returned an upload_id (e.g. only failures
-      // or pre-async-backend deployment), we have nothing to wait for —
-      // fall through to the finally cleanup which resets state.
     } catch (error: any) {
       console.error('Upload error:', error);
       toast.error(error.response?.data?.error || t('toast.uploadError'));
@@ -410,6 +410,15 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
     if (!processingAggregate.isComplete) return;
 
     if (processingAggregate.failed > 0) {
+      // Persist the failed photos into the report before uploadIds is cleared
+      // below (which would otherwise empty the progress hook's failedPhotos).
+      setProcessingFailures(
+        processingAggregate.failedPhotos.map((p) => ({
+          filename: p.filename,
+          reason: p.error || t('upload.failures.unknownReason', 'Unknown error'),
+          kind: 'processing' as const,
+        }))
+      );
       toast.warning(
         t('upload.processingFailed', { count: processingAggregate.failed }) ||
           `${processingAggregate.failed} photo(s) failed to process`
@@ -421,6 +430,12 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
     }
 
     if (onUploadComplete) onUploadComplete();
+    // Now the whole outcome is known — settle. The modal auto-closes only
+    // when nothing failed at either stage; any transfer OR processing
+    // failure keeps it open so the report (which lists both) stays visible.
+    onUploadSettled?.({
+      hasFailures: transferFailures.length > 0 || processingAggregate.failed > 0,
+    });
     setIsUploading(false);
     setUploadProgress(0);
     setCurrentChunk(0);
@@ -578,6 +593,8 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
       {!failuresDismissed && failures.length > 0 && (
         <div
           data-testid="upload-failure-report"
+          role="status"
+          aria-live="polite"
           className="rounded-lg border border-amber-300 dark:border-amber-700/60 bg-amber-50 dark:bg-amber-900/20 p-4"
         >
           <div className="flex items-start justify-between gap-3">

frontend/src/components/admin/__tests__/PhotoUpload.failureReport.test.tsx

@@ -3,12 +3,14 @@
  *
  * Before this, a partial upload only showed a count ("some files failed"),
  * and the backend's per-file `errors[]` were dropped entirely. These tests
- * pin that every failure stage is now named with its reason:
+ * pin that every failure stage is named with its reason:
  *   - rejected:   from the upload response's `errors: [{filename, error}]`
+ *   - transfer:   from a whole-chunk POST failure (the `catch` path)
  *   - processing: from useUploadProgress's `failedPhotos`
- * and that the report can be dismissed.
+ * plus the settle contract (hasFailures true/false) the modal relies on to
+ * decide whether to auto-close, and dismissal.
  */
-import { render, screen, within } from '@testing-library/react';
+import { render, screen, waitFor, within } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
@@ -30,26 +32,26 @@ vi.mock('react-toastify', () => ({
   toast: { warning: vi.fn(), info: vi.fn(), error: vi.fn(), success: vi.fn() },
 }));
 
-// The upload POST: 202 Accepted, one file queued, one rejected per-file.
+const postMock = vi.fn();
-const postMock = vi.fn().mockResolvedValue({
-  data: {
-    successCount: 1,
-    upload_id: 'u1',
-    errors: [{ filename: 'too-big.png', error: 'File too large' }],
-  },
-});
 vi.mock('../../../config/api', () => ({ api: { post: (...a: any[]) => postMock(...a), get: vi.fn() } }));
 
-// One photo failed in the background worker.
+// Mutable processing aggregate, swapped per test. Returned only once photos
+// are queued (uploadIds non-empty), mirroring the real hook flipping from
+// "nothing to track" to "complete" — the transition that fires the settle
+// effect.
+const clean = () => ({
+  total: 0, pending: 0, processing: 0, complete: 0, failed: 0,
+  failedPhotos: [] as { id: number; filename: string; error: string | null }[],
+  isComplete: false, isReady: true,
+});
+const hoisted = vi.hoisted(() => ({ aggregate: null as any }));
 vi.mock('../../../hooks/useUploadProgress', () => ({
-  useUploadProgress: () => ({
+  useUploadProgress: (ids: string[]) => ({
     snapshots: {},
     error: null,
-    aggregate: {
+    aggregate: ids && ids.length > 0
-      total: 2, pending: 0, processing: 0, complete: 1, failed: 1,
+      ? hoisted.aggregate
-      failedPhotos: [{ id: 5, filename: 'corrupt.jpg', error: 'Unsupported format' }],
+      : { total: 0, pending: 0, processing: 0, complete: 0, failed: 0, failedPhotos: [], isComplete: false, isReady: true },
-      isComplete: true, isReady: true,
-    },
   }),
 }));
 
@@ -65,40 +67,83 @@ const renderWithClient = (ui: ReactElement) => {
   return render(<QueryClientProvider client={queryClient}>{ui}</QueryClientProvider>);
 };
 
-async function uploadOneFile(container: HTMLElement, user: ReturnType<typeof userEvent.setup>) {
+async function uploadFile(container: HTMLElement, user: ReturnType<typeof userEvent.setup>) {
   const fileInput = container.querySelector('input[type="file"]') as HTMLInputElement;
-  await user.upload(fileInput, new File([new Uint8Array([1, 2, 3])], 'too-big.png', { type: 'image/png' }));
+  await user.upload(fileInput, new File([new Uint8Array([1, 2, 3])], 'good-photo.png', { type: 'image/png' }));
   await user.click(screen.getByRole('button', { name: /common\.upload/ }));
 }
 
 describe('PhotoUpload failure report', () => {
-  beforeEach(() => postMock.mockClear());
+  beforeEach(() => {
+    postMock.mockReset();
+    hoisted.aggregate = clean();
+  });
   afterEach(() => vi.clearAllMocks());
 
-  it('names each failed file with its reason and failure stage', async () => {
+  it('names rejected + processing failures and settles with hasFailures', async () => {
+    postMock.mockResolvedValue({
+      data: { successCount: 1, count: 1, upload_id: 'u1', errors: [{ filename: 'too-big.png', error: 'File too large' }] },
+    });
+    hoisted.aggregate = {
+      total: 2, pending: 0, processing: 0, complete: 1, failed: 1,
+      failedPhotos: [{ id: 5, filename: 'corrupt.jpg', error: 'Unsupported format' }],
+      isComplete: true, isReady: true,
+    };
+    const onUploadSettled = vi.fn();
     const user = userEvent.setup();
-    const { container } = renderWithClient(<PhotoUpload eventId={1} />);
+    const { container } = renderWithClient(<PhotoUpload eventId={1} onUploadSettled={onUploadSettled} />);
 
-    await uploadOneFile(container, user);
+    await uploadFile(container, user);
 
     const report = await screen.findByTestId('upload-failure-report');
-
-    // Rejected file (from the response errors[] that used to be dropped)
     expect(within(report).getByText('too-big.png')).toBeInTheDocument();
     expect(within(report).getByText(/File too large/)).toBeInTheDocument();
     expect(within(report).getByText('Rejected')).toBeInTheDocument();
-
-    // Processing failure (from useUploadProgress.failedPhotos)
     expect(within(report).getByText('corrupt.jpg')).toBeInTheDocument();
-    expect(within(report).getByText(/Unsupported format/)).toBeInTheDocument();
     expect(within(report).getByText('Processing failed')).toBeInTheDocument();
+
+    // Contract with the modal: the real component fires onUploadSettled and,
+    // because something failed, asks the host NOT to auto-close.
+    await waitFor(() => expect(onUploadSettled).toHaveBeenCalledWith({ hasFailures: true }));
   });
 
-  it('can be dismissed', async () => {
+  it('reports a whole-chunk transfer failure by name', async () => {
+    postMock.mockRejectedValue({ response: { data: { error: 'Network error' } } });
     const user = userEvent.setup();
     const { container } = renderWithClient(<PhotoUpload eventId={1} />);
 
-    await uploadOneFile(container, user);
+    await uploadFile(container, user);
+
+    const report = await screen.findByTestId('upload-failure-report');
+    expect(within(report).getByText('good-photo.png')).toBeInTheDocument();
+    expect(within(report).getByText(/Network error/)).toBeInTheDocument();
+    expect(within(report).getByText('Transfer failed')).toBeInTheDocument();
+  });
+
+  it('settles clean when nothing fails, so the modal can auto-close', async () => {
+    postMock.mockResolvedValue({ data: { successCount: 1, count: 1, upload_id: 'u1', errors: [] } });
+    hoisted.aggregate = {
+      total: 1, pending: 0, processing: 0, complete: 1, failed: 0,
+      failedPhotos: [], isComplete: true, isReady: true,
+    };
+    const onUploadSettled = vi.fn();
+    const user = userEvent.setup();
+    const { container } = renderWithClient(<PhotoUpload eventId={1} onUploadSettled={onUploadSettled} />);
+
+    await uploadFile(container, user);
+
+    await waitFor(() => expect(onUploadSettled).toHaveBeenCalledWith({ hasFailures: false }));
+    expect(screen.queryByTestId('upload-failure-report')).not.toBeInTheDocument();
+  });
+
+  it('can be dismissed', async () => {
+    postMock.mockResolvedValue({
+      data: { successCount: 0, count: 0, errors: [{ filename: 'too-big.png', error: 'File too large' }] },
+    });
+    const user = userEvent.setup();
+    const { container } = renderWithClient(<PhotoUpload eventId={1} />);
+
+    await uploadFile(container, user);
     const report = await screen.findByTestId('upload-failure-report');
 
     await user.click(within(report).getByRole('button', { name: /Dismiss/i }));