feat(upload): two-state UI + temp dir cleanup (PR-A of async processing)

Phase 1 of the upload-progress redesign. Two changes that ship UX wins
without any architectural surgery — they're a stepping stone for the
full async-processing rework that follows in subsequent commits.

1. Two-state progress bar (PhotoUpload.tsx, UserPhotoUpload.tsx)

   When axios.onUploadProgress reports loaded === total, the request is
   on the server and the bytes have left the browser. Today the bar sits
   at 100% for the chunk while the backend runs sharp/ffmpeg/EXIF (often
   minutes on NFS-backed storage) and users assume the upload froze.

   The component now distinguishes two phases:
   - 'transferring' — bytes-on-wire, determinate progress bar.
   - 'processing'   — bytes done, waiting for response. Indeterminate
                      spinner + an explanatory hint that the backend is
                      generating thumbnails / reading metadata and the
                      user can leave the page.

   Same pattern in UserPhotoUpload (gallery): the per-file checkmark
   icon is replaced by a Loader2 spinner while the request is in flight
   after bytes-on-wire finished.

2. Temp directory cleanup (adminPhotos.js)

   Multer creates temp/upload_<ts>_<rand>/ per request. Files inside it
   are individually unlinked after they're moved to storage on the
   success path, but the empty directory was never removed. On error
   paths three different inline blocks each tried to clean up; the
   success path was missed entirely. Result: the orphan-empty-dirs
   accumulation reported in the issue (70+ on the affected instance).

   Replace the inline cleanup blocks with a single idempotent
   cleanupTempDir() registered on res.finish + res.close, so it fires
   exactly once on every exit path (validation 4xx, server 5xx, multer
   error, success).

New translation keys (en/de): upload.transferring, upload.processing,
upload.processingHint, upload.processingProgress, upload.processingFailed,
upload.retryFailed.
This commit is contained in:
Paul Nothaft
2026-05-02 22:56:04 +02:00
parent f905f7e733
commit 86dfcc4f11
5 changed files with 141 additions and 62 deletions
+20 -35
View File
@@ -150,6 +150,24 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r
next(); next();
}); });
}, validateUploadContent, validateUploadedFiles, async (req, res) => { }, validateUploadContent, validateUploadedFiles, async (req, res) => {
// Single cleanup site for the multer temp directory — runs on every
// exit path (success, validation 4xx, server 5xx, multer error). The
// previous code had three inline cleanup blocks for individual early
// returns and missed the success path entirely, leaving an empty
// per-request directory behind on every successful upload (#357 review).
let tempCleanupDone = false;
const cleanupTempDir = async () => {
if (tempCleanupDone || !req.tempUploadPath) return;
tempCleanupDone = true;
try {
await fs.rm(req.tempUploadPath, { recursive: true, force: true });
} catch (e) {
console.error('Failed to clean up temp upload directory:', e);
}
};
res.on('finish', cleanupTempDir);
res.on('close', cleanupTempDir);
try { try {
const { eventId } = req.params; const { eventId } = req.params;
const { category_id, replace_by_name } = req.body; const { category_id, replace_by_name } = req.body;
@@ -165,14 +183,6 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r
const event = await db('events').where({ id: eventId }).first(); const event = await db('events').where({ id: eventId }).first();
if (!event) { if (!event) {
console.error('Event not found:', eventId); console.error('Event not found:', eventId);
// Clean up temp files
if (req.tempUploadPath) {
try {
await fs.rm(req.tempUploadPath, { recursive: true, force: true });
} catch (e) {
console.error('Failed to clean up temp path:', e);
}
}
return res.status(404).json({ error: 'Event not found' }); return res.status(404).json({ error: 'Event not found' });
} }
@@ -192,14 +202,6 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r
} }
} }
if (currentCount + newFilesCount > event.photo_cap) { if (currentCount + newFilesCount > event.photo_cap) {
// Clean up temp files
if (req.tempUploadPath) {
try {
await fs.rm(req.tempUploadPath, { recursive: true, force: true });
} catch (e) {
console.error('Failed to clean up temp path:', e);
}
}
return res.status(400).json({ return res.status(400).json({
error: `Photo cap exceeded. This event allows a maximum of ${event.photo_cap} photos. Currently ${currentCount} photos exist, and you are trying to upload ${newFilesCount} more.` error: `Photo cap exceeded. This event allows a maximum of ${event.photo_cap} photos. Currently ${currentCount} photos exist, and you are trying to upload ${newFilesCount} more.`
}); });
@@ -209,14 +211,6 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r
if (!req.files || req.files.length === 0) { if (!req.files || req.files.length === 0) {
console.error('No files in request. req.files:', req.files); console.error('No files in request. req.files:', req.files);
console.error('Request body keys:', Object.keys(req.body)); console.error('Request body keys:', Object.keys(req.body));
// Clean up temp files
if (req.tempUploadPath) {
try {
await fs.rm(req.tempUploadPath, { recursive: true, force: true });
} catch (e) {
console.error('Failed to clean up temp path:', e);
}
}
return res.status(400).json({ error: 'No files uploaded' }); return res.status(400).json({ error: 'No files uploaded' });
} }
@@ -597,17 +591,8 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r
res.json(response); res.json(response);
} catch (error) { } catch (error) {
console.error('Error uploading photos:', error); console.error('Error uploading photos:', error);
// Temp directory cleanup is handled by the response finish/close
// Clean up temp upload directory on error // listeners above, regardless of which exit path fires.
if (req.tempUploadPath) {
try {
await fs.rm(req.tempUploadPath, { recursive: true, force: true });
console.log(`Cleaned up temp upload directory after error: ${req.tempUploadPath}`);
} catch (e) {
console.error('Failed to clean up temp upload directory:', e);
}
}
res.status(500).json({ error: 'Failed to upload photos' }); res.status(500).json({ error: 'Failed to upload photos' });
} }
}); });
+83 -20
View File
@@ -1,5 +1,5 @@
import React, { useState, useRef, useMemo } from 'react'; import React, { useState, useRef, useMemo } from 'react';
import { Upload, X, Image, Loader2 } from 'lucide-react'; import { Upload, X, Image, Loader2, Cog } from 'lucide-react';
import { Button } from '../common'; import { Button } from '../common';
import { clsx } from 'clsx'; import { clsx } from 'clsx';
import { api } from '../../config/api'; import { api } from '../../config/api';
@@ -18,6 +18,15 @@ interface PhotoUploadProps {
const DEFAULT_MAX_FILES_PER_UPLOAD = 500; const DEFAULT_MAX_FILES_PER_UPLOAD = 500;
const MAX_FILES_PER_UPLOAD_LIMIT = 2000; const MAX_FILES_PER_UPLOAD_LIMIT = 2000;
// Upload phase machine. The user perceives "frozen" during 'processing'
// because the bytes are already on the server and we're waiting for
// thumbnail/EXIF/etc. work — the explicit phase + hint message kills
// that perception (#352 / contributor analysis on issue 357 review).
type UploadPhase =
| { kind: 'idle' }
| { kind: 'transferring'; chunkIndex: number; totalChunks: number; bytePct: number }
| { kind: 'processing'; chunkIndex: number; totalChunks: number; filesInChunk: number };
export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadComplete }) => { export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadComplete }) => {
const { t } = useTranslation(); const { t } = useTranslation();
const [isUploading, setIsUploading] = useState(false); const [isUploading, setIsUploading] = useState(false);
@@ -25,6 +34,7 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
const [uploadProgress, setUploadProgress] = useState(0); const [uploadProgress, setUploadProgress] = useState(0);
const [currentChunk, setCurrentChunk] = useState(0); const [currentChunk, setCurrentChunk] = useState(0);
const [totalChunks, setTotalChunks] = useState(0); const [totalChunks, setTotalChunks] = useState(0);
const [phase, setPhase] = useState<UploadPhase>({ kind: 'idle' });
const [selectedCategoryId, setSelectedCategoryId] = useState<number | null>(null); const [selectedCategoryId, setSelectedCategoryId] = useState<number | null>(null);
const [replaceByName, setReplaceByName] = useState(false); const [replaceByName, setReplaceByName] = useState(false);
const fileInputRef = useRef<HTMLInputElement>(null); const fileInputRef = useRef<HTMLInputElement>(null);
@@ -156,14 +166,44 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
formData.append('replace_by_name', 'true'); formData.append('replace_by_name', 'true');
} }
setPhase({
kind: 'transferring',
chunkIndex,
totalChunks: chunks.length,
bytePct: 0,
});
try { try {
const response = await api.post(`/admin/events/${eventId}/upload`, formData, { const response = await api.post(`/admin/events/${eventId}/upload`, formData, {
onUploadProgress: (progressEvent) => { onUploadProgress: (progressEvent) => {
if (progressEvent.total) { if (progressEvent.total) {
// Calculate overall progress across all chunks
const chunkProgress = progressEvent.loaded / progressEvent.total; const chunkProgress = progressEvent.loaded / progressEvent.total;
const overallProgress = ((chunkIndex + chunkProgress) / chunks.length) * 100; const overallProgress = ((chunkIndex + chunkProgress) / chunks.length) * 100;
setUploadProgress(Math.round(overallProgress)); setUploadProgress(Math.round(overallProgress));
// Once bytes have all left the browser, the request is
// sitting in the backend processing pipeline. Flip to
// 'processing' so the UI explains the wait instead of
// looking frozen at the chunk's max progress.
if (chunkProgress >= 1) {
setPhase((prev) =>
prev.kind === 'transferring' && prev.chunkIndex === chunkIndex
? {
kind: 'processing',
chunkIndex,
totalChunks: chunks.length,
filesInChunk: chunk.length,
}
: prev
);
} else {
setPhase({
kind: 'transferring',
chunkIndex,
totalChunks: chunks.length,
bytePct: Math.round(chunkProgress * 100),
});
}
} }
}, },
}); });
@@ -210,6 +250,7 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
setUploadProgress(0); setUploadProgress(0);
setCurrentChunk(0); setCurrentChunk(0);
setTotalChunks(0); setTotalChunks(0);
setPhase({ kind: 'idle' });
} }
}; };
@@ -344,26 +385,48 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
</Button> </Button>
</div> </div>
{/* Progress Bar */} {/* 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
assume the upload froze. */}
{isUploading && ( {isUploading && (
<div className="mt-4"> <div className="mt-4">
<div className="flex justify-between text-sm text-neutral-600 dark:text-neutral-400 mb-1"> {phase.kind === 'processing' ? (
<span> <div className="rounded-lg border border-amber-200 dark:border-amber-800 bg-amber-50 dark:bg-amber-900/20 p-4">
{t('upload.uploading')} <div className="flex items-start gap-3">
{totalChunks > 1 && ` (${t('common.chunk')} ${currentChunk}/${totalChunks})`} <Cog className="w-5 h-5 text-amber-600 dark:text-amber-400 animate-spin shrink-0 mt-0.5" />
</span> <div className="flex-1 min-w-0">
<span>{uploadProgress}%</span> <p className="text-sm font-medium text-amber-900 dark:text-amber-100">
</div> {t('upload.processing')}
<div className="w-full bg-neutral-200 dark:bg-neutral-700 rounded-full h-2"> {phase.totalChunks > 1 && ` (${t('common.chunk')} ${phase.chunkIndex + 1}/${phase.totalChunks})`}
<div </p>
className="bg-primary-600 h-2 rounded-full transition-all duration-300" <p className="text-xs text-amber-800 dark:text-amber-200 mt-1">
style={{ width: `${uploadProgress}%` }} {t('upload.processingHint')}
/> </p>
</div> </div>
{totalChunks > 1 && ( </div>
<p className="text-xs text-neutral-500 dark:text-neutral-400 mt-1"> </div>
{t('upload.uploadingChunks', { count: selectedFiles.length, total: totalChunks })} ) : (
</p> <>
<div className="flex justify-between text-sm text-neutral-600 dark:text-neutral-400 mb-1">
<span>
{t('upload.transferring')}
{totalChunks > 1 && ` (${t('common.chunk')} ${currentChunk}/${totalChunks})`}
</span>
<span>{uploadProgress}%</span>
</div>
<div className="w-full bg-neutral-200 dark:bg-neutral-700 rounded-full h-2">
<div
className="bg-primary-600 h-2 rounded-full transition-all duration-300"
style={{ width: `${uploadProgress}%` }}
/>
</div>
{totalChunks > 1 && (
<p className="text-xs text-neutral-500 dark:text-neutral-400 mt-1">
{t('upload.uploadingChunks', { count: selectedFiles.length, total: totalChunks })}
</p>
)}
</>
)} )}
</div> </div>
)} )}
@@ -1,5 +1,5 @@
import React, { useState, useMemo } from 'react'; import React, { useState, useMemo } from 'react';
import { Upload, X, CheckCircle } from 'lucide-react'; import { Upload, X, CheckCircle, Loader2 } from 'lucide-react';
import { useTranslation } from 'react-i18next'; import { useTranslation } from 'react-i18next';
import { toast } from 'react-toastify'; import { toast } from 'react-toastify';
import { Button } from '../common'; import { Button } from '../common';
@@ -24,6 +24,10 @@ export const UserPhotoUpload: React.FC<UserPhotoUploadProps> = ({
const [files, setFiles] = useState<File[]>([]); const [files, setFiles] = useState<File[]>([]);
const [uploading, setUploading] = useState(false); const [uploading, setUploading] = useState(false);
const [uploadProgress, setUploadProgress] = useState<{ [key: string]: number }>({}); const [uploadProgress, setUploadProgress] = useState<{ [key: string]: number }>({});
// Per-file processing state — flips to true once axios reports
// bytes-on-wire for that file, so the UI can show "Processing…"
// instead of a static 100% bar while the backend works.
const [processingFiles, setProcessingFiles] = useState<{ [key: string]: boolean }>({});
const { data: publicSettings } = usePublicSettings(); const { data: publicSettings } = usePublicSettings();
@@ -87,9 +91,18 @@ export const UserPhotoUpload: React.FC<UserPhotoUploadProps> = ({
...prev, ...prev,
[file.name]: progress, [file.name]: progress,
})); }));
if (progress >= 100) {
setProcessingFiles(prev => ({ ...prev, [file.name]: true }));
}
} }
}, },
}); });
// Request resolved → file fully processed by backend.
setProcessingFiles(prev => {
const next = { ...prev };
delete next[file.name];
return next;
});
successCount++; successCount++;
} catch (error: any) { } catch (error: any) {
// Upload error handled - user notified via UI // Upload error handled - user notified via UI
@@ -185,7 +198,13 @@ export const UserPhotoUpload: React.FC<UserPhotoUploadProps> = ({
</div> </div>
{uploadProgress[file.name] !== undefined ? ( {uploadProgress[file.name] !== undefined ? (
<div className="flex items-center gap-2"> <div className="flex items-center gap-2">
{uploadProgress[file.name] === 100 ? ( {processingFiles[file.name] ? (
// Bytes are on the server; the request hasn't
// resolved yet because the backend is still
// generating thumbnails / reading EXIF. Show
// a spinner so it doesn't look stuck at 100%.
<Loader2 className="w-5 h-5 text-amber-600 animate-spin" />
) : uploadProgress[file.name] === 100 ? (
<CheckCircle className="w-5 h-5 text-green-600" /> <CheckCircle className="w-5 h-5 text-green-600" />
) : ( ) : (
<div className="w-20"> <div className="w-20">
+6
View File
@@ -131,6 +131,12 @@
"unsupportedFiles": "Einige Dateien wurden übersprungen, da das Format nicht unterstützt wird (JPEG/PNG/WebP/MP4/MOV/WEBM verwenden).", "unsupportedFiles": "Einige Dateien wurden übersprungen, da das Format nicht unterstützt wird (JPEG/PNG/WebP/MP4/MOV/WEBM verwenden).",
"selectedFiles": "Ausgewählte Dateien", "selectedFiles": "Ausgewählte Dateien",
"uploading": "Wird hochgeladen...", "uploading": "Wird hochgeladen...",
"transferring": "Übertragung",
"processing": "Fotos werden verarbeitet...",
"processingHint": "Dateien sind hochgeladen. PicPeak erstellt jetzt Thumbnails und liest Metadaten. Sie können diese Seite verlassen — die Verarbeitung läuft im Hintergrund weiter.",
"processingProgress": "{{complete}} von {{total}} fertig",
"processingFailed": "{{count}} Foto(s) konnten nicht verarbeitet werden",
"retryFailed": "Fehlgeschlagene erneut versuchen",
"uploadComplete": "Upload abgeschlossen!", "uploadComplete": "Upload abgeschlossen!",
"uploadFailed": "Upload fehlgeschlagen", "uploadFailed": "Upload fehlgeschlagen",
"someFilesFailed": "Einige Dateien konnten nicht hochgeladen werden", "someFilesFailed": "Einige Dateien konnten nicht hochgeladen werden",
+6
View File
@@ -131,6 +131,12 @@
"unsupportedFiles": "Some files were skipped because the format is not supported (use JPEG/PNG/WebP/MP4/MOV/WEBM).", "unsupportedFiles": "Some files were skipped because the format is not supported (use JPEG/PNG/WebP/MP4/MOV/WEBM).",
"selectedFiles": "Selected files", "selectedFiles": "Selected files",
"uploading": "Uploading...", "uploading": "Uploading...",
"transferring": "Transferring",
"processing": "Processing photos...",
"processingHint": "Files are uploaded. PicPeak is now generating thumbnails and reading metadata. You can leave this page — work continues in the background.",
"processingProgress": "{{complete}} of {{total}} done",
"processingFailed": "{{count}} photo(s) failed to process",
"retryFailed": "Retry failed",
"uploadComplete": "Upload complete!", "uploadComplete": "Upload complete!",
"uploadFailed": "Upload failed", "uploadFailed": "Upload failed",
"someFilesFailed": "Some files failed to upload", "someFilesFailed": "Some files failed to upload",