From 1b0100cfad2be169d0b28664dd4b3ea89b391c20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Deuerling?= Date: Tue, 30 Jun 2026 21:08:02 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Surface=20which=20files=20failed=20?= =?UTF-8?q?during=20photo=20upload?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A partial upload only told the admin "some files failed" with no way to find out which ones — even though the data existed. The backend already returns per-file rejections (response.errors: [{filename, error}]) and the progress hook already exposes failedPhotos, but both were dropped. Add a dismissible failure report to the upload modal listing every file that didn't make it into the gallery, grouped by stage with its reason: - rejected: per-file validation rejections from the upload response (previously discarded entirely) - transfer: whole-chunk request failures (now captured with the error, not just the filename) - processing: background-worker failures from useUploadProgress.failedPhotos Replace the count-only "some files failed" toast with one that points at the list. Add en/de keys under upload.failures.* and a component test covering the rejected + processing rows and dismissal. --- frontend/src/components/admin/PhotoUpload.tsx | 128 ++++++++++++++++-- .../PhotoUpload.failureReport.test.tsx | 107 +++++++++++++++ frontend/src/i18n/locales/de.json | 10 ++ frontend/src/i18n/locales/en.json | 10 ++ 4 files changed, 247 insertions(+), 8 deletions(-) create mode 100644 frontend/src/components/admin/__tests__/PhotoUpload.failureReport.test.tsx diff --git a/frontend/src/components/admin/PhotoUpload.tsx b/frontend/src/components/admin/PhotoUpload.tsx index 2cae80c8..57ff78fd 100644 --- a/frontend/src/components/admin/PhotoUpload.tsx +++ b/frontend/src/components/admin/PhotoUpload.tsx @@ -1,5 +1,5 @@ import React, { useState, useRef, useMemo, useEffect } from 'react'; -import { Upload, X, Image, Loader2, Cog } from 'lucide-react'; +import { Upload, X, Image, Loader2, Cog, AlertTriangle } from 'lucide-react'; import { Button } from '../common'; import { clsx } from 'clsx'; import { api } from '../../config/api'; @@ -28,6 +28,21 @@ type UploadPhase = | { kind: 'transferring'; chunkIndex: number; totalChunks: number; bytePct: number } | { kind: 'processing'; chunkIndex: number; totalChunks: number; filesInChunk: number }; +// Why a file didn't make it into the gallery. Each maps to a distinct +// stage so the user knows whether to re-pick the file (rejected), retry +// the network (transfer), or check the source image (processing). +// - rejected: validation/queueing refused it (bad type, too large, +// corrupt) — returned per-file in the upload response. +// - transfer: the whole chunk request failed (timeout, 5xx, network). +// - processing: stored fine, but the background worker couldn't process +// it (from useUploadProgress's failedPhotos). +type UploadFailureKind = 'rejected' | 'transfer' | 'processing'; +interface UploadFailure { + filename: string; + reason: string; + kind: UploadFailureKind; +} + export const PhotoUpload: React.FC = ({ eventId, onUploadComplete }) => { const { t } = useTranslation(); const [isUploading, setIsUploading] = useState(false); @@ -42,11 +57,28 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl // hook merges status across all of them so the user sees one unified // progress count even when the upload spans multiple HTTP requests. const [uploadIds, setUploadIds] = useState([]); + // Per-file failures surfaced from the transfer stage (chunk POSTs): + // validation rejections (response.errors) and whole-chunk failures. + // Processing failures are merged in from the progress hook below. + const [transferFailures, setTransferFailures] = useState([]); + const [failuresDismissed, setFailuresDismissed] = useState(false); const fileInputRef = useRef(null); const { aggregate: processingAggregate } = useUploadProgress(uploadIds, { enabled: phase.kind === 'processing' && uploadIds.length > 0, }); + + // Single source of truth for the "which files failed" report: transfer + // stage failures (collected during handleUpload) plus processing + // failures (live, from the progress hook). Both carry filename + reason. + const failures = useMemo(() => { + const processing: UploadFailure[] = processingAggregate.failedPhotos.map((p) => ({ + filename: p.filename, + 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({ @@ -162,6 +194,9 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl setIsUploading(true); setUploadProgress(0); setUploadIds([]); + // Clear any prior failure report before this run. + setTransferFailures([]); + setFailuresDismissed(false); // For large uploads, chunk the files by both count AND size to prevent memory/network issues. // #509: the per-chunk byte cap MUST be tunable so users behind Cloudflare Tunnel and other @@ -195,9 +230,10 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl } setTotalChunks(chunks.length); - let totalUploaded = 0; let totalReplaced = 0; - let failedFiles = []; + // Accumulates transfer-stage failures (per-file rejections + whole-chunk + // failures) with their reasons, so the report can name each one. + const collected: UploadFailure[] = []; try { for (let chunkIndex = 0; chunkIndex < chunks.length; chunkIndex++) { @@ -258,8 +294,20 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl }, }); - totalUploaded += (response.data?.successCount || chunk.length); totalReplaced += (response.data?.replacedCount || 0); + // The backend accepts the request (202) but may reject individual + // files (bad type, too large, corrupt) and reports them in + // `errors: [{ filename, error }]`. Surface each one by name. + const rejected = response.data?.errors; + if (Array.isArray(rejected)) { + for (const r of rejected) { + collected.push({ + filename: r?.filename || t('upload.failures.unknownFile', 'Unknown file'), + reason: r?.error || t('upload.failures.unknownReason', 'Unknown error'), + kind: 'rejected', + }); + } + } // Backend returns a per-request upload_id. Track it so the // processing-status hook can poll/stream live progress. if (response.data?.upload_id) { @@ -268,7 +316,13 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl } } catch (error: any) { console.error(`Error uploading chunk ${chunkIndex + 1}:`, error); - failedFiles.push(...chunk.map(f => f.name)); + const reason = + error?.response?.data?.error || + error?.message || + t('upload.failures.transferReason', 'Transfer failed'); + collected.push( + ...chunk.map((f) => ({ filename: f.name, reason, kind: 'transfer' as const })) + ); // Continue with next chunk even if one fails continue; @@ -288,10 +342,15 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl if (totalReplaced > 0) { toast.info(t('upload.replacedFiles', { count: totalReplaced }) || `${totalReplaced} photo(s) replaced`); } - if (failedFiles.length > 0) { + // Publish transfer-stage failures to the report (rendered below with + // each filename + reason). The toast is just the headline; the list + // is where the user finds out *which* files failed. + setTransferFailures(collected); + if (collected.length > 0) { toast.warning( - t('upload.someFilesFailed') || - `Transferred ${totalUploaded} files. ${failedFiles.length} files failed to transfer.` + t('upload.failures.toast', '{{count}} file(s) could not be uploaded — see the list below.', { + count: collected.length, + }) ); } @@ -487,6 +546,59 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl + {/* Failure report — names every file that didn't make it into the + gallery, grouped by failure stage, so the user can act on each. + Persists until dismissed or a new upload starts. */} + {!failuresDismissed && failures.length > 0 && ( +
+
+
+ +

+ {t('upload.failures.title', '{{count}} file(s) could not be uploaded', { + count: failures.length, + })} +

+
+ +
+
    + {failures.map((f, i) => ( +
  • + + {f.kind === 'rejected' && t('upload.failures.kindRejected', 'Rejected')} + {f.kind === 'transfer' && t('upload.failures.kindTransfer', 'Transfer failed')} + {f.kind === 'processing' && t('upload.failures.kindProcessing', 'Processing failed')} + + + + {f.filename} + + — {f.reason} + +
    • + ))} +
+
+ )} + {/* Progress display — two distinct phases. Bytes-on-wire ('transferring') drives the determinate bar; the post-bytes wait ('processing') swaps in an indeterminate spinner with an explanatory hint so users don't diff --git a/frontend/src/components/admin/__tests__/PhotoUpload.failureReport.test.tsx b/frontend/src/components/admin/__tests__/PhotoUpload.failureReport.test.tsx new file mode 100644 index 00000000..b6bd9ce3 --- /dev/null +++ b/frontend/src/components/admin/__tests__/PhotoUpload.failureReport.test.tsx @@ -0,0 +1,107 @@ +/** + * Coverage for the upload failure report — the "which files failed" list. + * + * 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: + * - rejected: from the upload response's `errors: [{filename, error}]` + * - processing: from useUploadProgress's `failedPhotos` + * and that the report can be dismissed. + */ +import { render, screen, 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'; +import type { ReactElement } from 'react'; + +import { PhotoUpload } from '../PhotoUpload'; + +vi.mock('react-i18next', async () => { + const actual = await vi.importActual('react-i18next'); + return { + ...actual, + useTranslation: () => ({ + t: (_key: string, fallback?: any) => (typeof fallback === 'string' ? fallback : _key), + }), + }; +}); + +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().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. +vi.mock('../../../hooks/useUploadProgress', () => ({ + useUploadProgress: () => ({ + snapshots: {}, + error: null, + aggregate: { + total: 2, pending: 0, processing: 0, complete: 1, failed: 1, + failedPhotos: [{ id: 5, filename: 'corrupt.jpg', error: 'Unsupported format' }], + isComplete: true, isReady: true, + }, + }), +})); + +vi.mock('../../../services/categories.service', () => ({ + categoriesService: { getEventCategories: vi.fn().mockResolvedValue([]) }, +})); +vi.mock('../../../services/settings.service', () => ({ + settingsService: { getAllSettings: vi.fn().mockResolvedValue({}) }, +})); + +const renderWithClient = (ui: ReactElement) => { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render({ui}); +}; + +async function uploadOneFile(container: HTMLElement, user: ReturnType) { + 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.click(screen.getByRole('button', { name: /common\.upload/ })); +} + +describe('PhotoUpload failure report', () => { + beforeEach(() => postMock.mockClear()); + afterEach(() => vi.clearAllMocks()); + + it('names each failed file with its reason and failure stage', async () => { + const user = userEvent.setup(); + const { container } = renderWithClient(); + + await uploadOneFile(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(); + }); + + it('can be dismissed', async () => { + const user = userEvent.setup(); + const { container } = renderWithClient(); + + await uploadOneFile(container, user); + const report = await screen.findByTestId('upload-failure-report'); + + await user.click(within(report).getByRole('button', { name: /Dismiss/i })); + expect(screen.queryByTestId('upload-failure-report')).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index fabc4e7e..3f00380b 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -169,6 +169,16 @@ "retryFailed": "Fehlgeschlagene erneut versuchen", "uploadComplete": "Upload abgeschlossen!", "someFilesFailed": "Einige Dateien konnten nicht hochgeladen werden", + "failures": { + "title": "{{count}} Datei(en) konnten nicht hochgeladen werden", + "toast": "{{count}} Datei(en) konnten nicht hochgeladen werden – siehe Liste unten.", + "kindRejected": "Abgelehnt", + "kindTransfer": "Übertragung fehlgeschlagen", + "kindProcessing": "Verarbeitung fehlgeschlagen", + "transferReason": "Übertragung fehlgeschlagen", + "unknownReason": "Unbekannter Fehler", + "unknownFile": "Unbekannte Datei" + }, "replaceByName": "Vorhandene Fotos mit gleichem Namen ersetzen", "uploadPhotos": "Fotos hochladen", "uploadMedia": "Fotos & Videos hochladen", diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 8daea867..e2a5adc2 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -169,6 +169,16 @@ "retryFailed": "Retry failed", "uploadComplete": "Upload complete!", "someFilesFailed": "Some files failed to upload", + "failures": { + "title": "{{count}} file(s) could not be uploaded", + "toast": "{{count}} file(s) could not be uploaded — see the list below.", + "kindRejected": "Rejected", + "kindTransfer": "Transfer failed", + "kindProcessing": "Processing failed", + "transferReason": "Transfer failed", + "unknownReason": "Unknown error", + "unknownFile": "Unknown file" + }, "replaceByName": "Replace existing photos with same name", "uploadPhotos": "Upload Photos", "uploadMedia": "Upload Photos & Videos",