diff --git a/backend/src/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js index 46ec4661..1a577260 100644 --- a/backend/src/routes/adminPhotos.js +++ b/backend/src/routes/adminPhotos.js @@ -150,29 +150,39 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r next(); }); }, 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 { const { eventId } = req.params; const { category_id, replace_by_name } = req.body; const replaceByName = replace_by_name === 'true' || replace_by_name === true; - + console.log('Upload request received for event:', eventId); console.log('Body:', req.body); console.log('Files:', req.files ? req.files.length : 'none'); console.log('File details:', req.files?.map(f => ({ name: f.originalname, size: f.size, mimetype: f.mimetype }))); console.log('Category ID received:', category_id); - + // Verify event exists and admin has access const event = await db('events').where({ id: eventId }).first(); if (!event) { 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' }); } @@ -192,14 +202,6 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r } } 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({ 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) { console.error('No files in request. req.files:', req.files); 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' }); } @@ -597,17 +591,8 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r res.json(response); } catch (error) { console.error('Error uploading photos:', error); - - // Clean up temp upload directory on error - 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); - } - } - + // Temp directory cleanup is handled by the response finish/close + // listeners above, regardless of which exit path fires. res.status(500).json({ error: 'Failed to upload photos' }); } }); diff --git a/frontend/src/components/admin/PhotoUpload.tsx b/frontend/src/components/admin/PhotoUpload.tsx index 326d8a2c..2d664cf6 100644 --- a/frontend/src/components/admin/PhotoUpload.tsx +++ b/frontend/src/components/admin/PhotoUpload.tsx @@ -1,5 +1,5 @@ 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 { clsx } from 'clsx'; import { api } from '../../config/api'; @@ -18,6 +18,15 @@ interface PhotoUploadProps { const DEFAULT_MAX_FILES_PER_UPLOAD = 500; 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 = ({ eventId, onUploadComplete }) => { const { t } = useTranslation(); const [isUploading, setIsUploading] = useState(false); @@ -25,6 +34,7 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl const [uploadProgress, setUploadProgress] = useState(0); const [currentChunk, setCurrentChunk] = useState(0); const [totalChunks, setTotalChunks] = useState(0); + const [phase, setPhase] = useState({ kind: 'idle' }); const [selectedCategoryId, setSelectedCategoryId] = useState(null); const [replaceByName, setReplaceByName] = useState(false); const fileInputRef = useRef(null); @@ -144,11 +154,11 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl setCurrentChunk(chunkIndex + 1); const chunk = chunks[chunkIndex]; const formData = new FormData(); - + chunk.forEach((file) => { formData.append('photos', file); }); - + if (selectedCategoryId) { formData.append('category_id', selectedCategoryId.toString()); } @@ -156,14 +166,44 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl formData.append('replace_by_name', 'true'); } + setPhase({ + kind: 'transferring', + chunkIndex, + totalChunks: chunks.length, + bytePct: 0, + }); + try { const response = await api.post(`/admin/events/${eventId}/upload`, formData, { onUploadProgress: (progressEvent) => { if (progressEvent.total) { - // Calculate overall progress across all chunks const chunkProgress = progressEvent.loaded / progressEvent.total; const overallProgress = ((chunkIndex + chunkProgress) / chunks.length) * 100; 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), + }); + } } }, }); @@ -173,7 +213,7 @@ 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)); - + // Continue with next chunk even if one fails continue; } @@ -210,6 +250,7 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl setUploadProgress(0); setCurrentChunk(0); setTotalChunks(0); + setPhase({ kind: 'idle' }); } }; @@ -344,26 +385,48 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl - {/* 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 && (
-
- - {t('upload.uploading')} - {totalChunks > 1 && ` (${t('common.chunk')} ${currentChunk}/${totalChunks})`} - - {uploadProgress}% -
-
-
-
- {totalChunks > 1 && ( -

- {t('upload.uploadingChunks', { count: selectedFiles.length, total: totalChunks })} -

+ {phase.kind === 'processing' ? ( +
+
+ +
+

+ {t('upload.processing')} + {phase.totalChunks > 1 && ` (${t('common.chunk')} ${phase.chunkIndex + 1}/${phase.totalChunks})`} +

+

+ {t('upload.processingHint')} +

+
+
+
+ ) : ( + <> +
+ + {t('upload.transferring')} + {totalChunks > 1 && ` (${t('common.chunk')} ${currentChunk}/${totalChunks})`} + + {uploadProgress}% +
+
+
+
+ {totalChunks > 1 && ( +

+ {t('upload.uploadingChunks', { count: selectedFiles.length, total: totalChunks })} +

+ )} + )}
)} diff --git a/frontend/src/components/gallery/UserPhotoUpload.tsx b/frontend/src/components/gallery/UserPhotoUpload.tsx index fa45e2ff..6423f157 100644 --- a/frontend/src/components/gallery/UserPhotoUpload.tsx +++ b/frontend/src/components/gallery/UserPhotoUpload.tsx @@ -1,5 +1,5 @@ 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 { toast } from 'react-toastify'; import { Button } from '../common'; @@ -24,6 +24,10 @@ export const UserPhotoUpload: React.FC = ({ const [files, setFiles] = useState([]); const [uploading, setUploading] = useState(false); 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(); @@ -87,9 +91,18 @@ export const UserPhotoUpload: React.FC = ({ ...prev, [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++; } catch (error: any) { // Upload error handled - user notified via UI @@ -185,7 +198,13 @@ export const UserPhotoUpload: React.FC = ({
{uploadProgress[file.name] !== undefined ? (
- {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%. + + ) : uploadProgress[file.name] === 100 ? ( ) : (
diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index 891a6c2d..2456c406 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -131,6 +131,12 @@ "unsupportedFiles": "Einige Dateien wurden übersprungen, da das Format nicht unterstützt wird (JPEG/PNG/WebP/MP4/MOV/WEBM verwenden).", "selectedFiles": "Ausgewählte Dateien", "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!", "uploadFailed": "Upload fehlgeschlagen", "someFilesFailed": "Einige Dateien konnten nicht hochgeladen werden", diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index d2d3ba87..0c571243 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -131,6 +131,12 @@ "unsupportedFiles": "Some files were skipped because the format is not supported (use JPEG/PNG/WebP/MP4/MOV/WEBM).", "selectedFiles": "Selected files", "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!", "uploadFailed": "Upload failed", "someFilesFailed": "Some files failed to upload",