✨ Surface which files failed during photo upload
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.
This commit is contained in:
@@ -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<PhotoUploadProps> = ({ eventId, onUploadComplete }) => {
|
||||
const { t } = useTranslation();
|
||||
const [isUploading, setIsUploading] = useState(false);
|
||||
@@ -42,11 +57,28 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ 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<string[]>([]);
|
||||
// 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<UploadFailure[]>([]);
|
||||
const [failuresDismissed, setFailuresDismissed] = useState(false);
|
||||
const fileInputRef = useRef<HTMLInputElement>(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<UploadFailure[]>(() => {
|
||||
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<PhotoUploadProps> = ({ 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<PhotoUploadProps> = ({ 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<PhotoUploadProps> = ({ 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<PhotoUploadProps> = ({ 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<PhotoUploadProps> = ({ 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<PhotoUploadProps> = ({ eventId, onUploadCompl
|
||||
</Button>
|
||||
</div>
|
||||
|
||||
{/* 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 && (
|
||||
<div
|
||||
data-testid="upload-failure-report"
|
||||
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">
|
||||
<div className="flex items-center gap-2 text-amber-800 dark:text-amber-300">
|
||||
<AlertTriangle className="w-5 h-5 flex-shrink-0" />
|
||||
<p className="text-sm font-medium">
|
||||
{t('upload.failures.title', '{{count}} file(s) could not be uploaded', {
|
||||
count: failures.length,
|
||||
})}
|
||||
</p>
|
||||
</div>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setFailuresDismissed(true)}
|
||||
aria-label={t('common.dismiss', 'Dismiss')}
|
||||
className="p-1 -m-1 text-amber-700 dark:text-amber-300 hover:bg-amber-100 dark:hover:bg-amber-800/40 rounded"
|
||||
>
|
||||
<X className="w-4 h-4" />
|
||||
</button>
|
||||
</div>
|
||||
<ul className="mt-3 max-h-48 overflow-y-auto space-y-1.5">
|
||||
{failures.map((f, i) => (
|
||||
<li key={`${f.kind}-${f.filename}-${i}`} className="flex items-start gap-2 text-xs">
|
||||
<span
|
||||
className={clsx(
|
||||
'flex-shrink-0 mt-0.5 px-1.5 py-0.5 rounded font-medium whitespace-nowrap',
|
||||
f.kind === 'rejected' && 'bg-red-100 text-red-700 dark:bg-red-900/40 dark:text-red-300',
|
||||
f.kind === 'transfer' && 'bg-orange-100 text-orange-700 dark:bg-orange-900/40 dark:text-orange-300',
|
||||
f.kind === 'processing' && 'bg-purple-100 text-purple-700 dark:bg-purple-900/40 dark:text-purple-300'
|
||||
)}
|
||||
>
|
||||
{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')}
|
||||
</span>
|
||||
<span className="min-w-0">
|
||||
<span className="font-medium text-neutral-800 dark:text-neutral-200 break-all">
|
||||
{f.filename}
|
||||
</span>
|
||||
<span className="text-neutral-500 dark:text-neutral-400"> — {f.reason}</span>
|
||||
</span>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* 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
|
||||
|
||||
@@ -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<typeof import('react-i18next')>('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(<QueryClientProvider client={queryClient}>{ui}</QueryClientProvider>);
|
||||
};
|
||||
|
||||
async function uploadOneFile(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.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(<PhotoUpload eventId={1} />);
|
||||
|
||||
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(<PhotoUpload eventId={1} />);
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user