Merge pull request #363 from the-luap/feat/upload-redesign-and-auth-loop-fix

feat(upload): async photo processing + fix(auth): /auth/session symmetry (loop fix)
2026-05-02 22:59:03 +02:00
parent f529c9e3d7 b30010eb6f
commit 907bcf1eb2
20 changed files with 2011 additions and 383 deletions
@@ -1,10 +1,12 @@
 import React, { useState } from 'react';
-import { Check, Download, Trash2, Eye, EyeOff, Package, MessageSquare, Star, Video, FolderOpen } from 'lucide-react';
+import { Check, Download, Trash2, Eye, EyeOff, Package, MessageSquare, Star, Video, FolderOpen, Cog, AlertTriangle, RefreshCw } from 'lucide-react';
 import { toast } from 'react-toastify';
+import { useQueryClient } from '@tanstack/react-query';
 import { useTranslation } from 'react-i18next';

 import { AdminPhoto } from '../../services/photos.service';
 import { photosService } from '../../services/photos.service';
+import { uploadsService } from '../../services/uploads.service';
 import { Button } from '../common';
 import { AdminAuthenticatedImage } from './AdminAuthenticatedImage';
 import { BulkCategoryModal } from './BulkCategoryModal';
@@ -32,6 +34,7 @@ export const AdminPhotoGrid: React.FC<AdminPhotoGridProps> = ({
  categories = []
 }) => {
  const { t } = useTranslation();
+  const queryClient = useQueryClient();
  const [selectedPhotos, setSelectedPhotos] = useState<Set<number>>(new Set());
  const [isSelectionMode, setIsSelectionMode] = useState(false);
  const [isDeleting, setIsDeleting] = useState(false);
@@ -298,9 +301,42 @@ export const AdminPhotoGrid: React.FC<AdminPhotoGridProps> = ({
              </div>
            )}

-            {/* Thumbnail */}
+            {/* Thumbnail (or processing placeholder for in-flight photos) */}
            <div className="aspect-square">
-              {photo.thumbnail_url ? (
+              {(photo as any).processing_status === 'pending' ||
+              (photo as any).processing_status === 'processing' ? (
+                <div className="w-full h-full flex flex-col items-center justify-center bg-amber-50 dark:bg-amber-900/20 text-amber-700 dark:text-amber-300 gap-1 px-2 text-center">
+                  <Cog className="w-7 h-7 animate-spin" />
+                  <p className="text-[10px] font-medium leading-tight">
+                    {t('admin.photos.processingStatus', 'Processing…')}
+                  </p>
+                </div>
+              ) : (photo as any).processing_status === 'failed' ? (
+                <div className="w-full h-full flex flex-col items-center justify-center bg-red-50 dark:bg-red-900/20 text-red-700 dark:text-red-300 gap-1 px-2 text-center">
+                  <AlertTriangle className="w-7 h-7" />
+                  <p className="text-[10px] font-medium leading-tight">
+                    {t('admin.photos.processingFailed', 'Failed')}
+                  </p>
+                  <button
+                    onClick={async (e) => {
+                      e.stopPropagation();
+                      try {
+                        await uploadsService.retryPhoto(photo.id);
+                        toast.success(t('admin.photos.retryQueued', 'Retry queued'));
+                        // Refetch grid via React Query so the placeholder
+                        // updates without a full reload.
+                        queryClient.invalidateQueries({ queryKey: ['admin-event-photos'] });
+                      } catch (err: any) {
+                        toast.error(err?.response?.data?.error || 'Retry failed');
+                      }
+                    }}
+                    className="mt-1 px-2 py-0.5 rounded bg-red-200 dark:bg-red-800 text-[10px] inline-flex items-center gap-1"
+                  >
+                    <RefreshCw className="w-2.5 h-2.5" />
+                    {t('upload.retryFailed', 'Retry')}
+                  </button>
+                </div>
+              ) : photo.thumbnail_url ? (
                <AdminAuthenticatedImage
                  src={photo.thumbnail_url}
                  alt={photo.filename}
@@ -1,5 +1,5 @@
-import React, { useState, useRef, useMemo } from 'react';
-import { Upload, X, Image, Loader2 } from 'lucide-react';
+import React, { useState, useRef, useMemo, useEffect } from 'react';
+import { Upload, X, Image, Loader2, Cog } from 'lucide-react';
 import { Button } from '../common';
 import { clsx } from 'clsx';
 import { api } from '../../config/api';
@@ -9,6 +9,7 @@ import { categoriesService } from '../../services/categories.service';
 import { settingsService } from '../../services/settings.service';
 import { useTranslation } from 'react-i18next';
 import { extensionsToMimeTypes, extensionsToAcceptString } from '../../utils/fileTypes';
+import { useUploadProgress } from '../../hooks/useUploadProgress';

 interface PhotoUploadProps {
  eventId: number;
@@ -18,6 +19,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<PhotoUploadProps> = ({ eventId, onUploadComplete }) => {
  const { t } = useTranslation();
  const [isUploading, setIsUploading] = useState(false);
@@ -25,9 +35,18 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
  const [uploadProgress, setUploadProgress] = useState(0);
  const [currentChunk, setCurrentChunk] = useState(0);
  const [totalChunks, setTotalChunks] = useState(0);
+  const [phase, setPhase] = useState<UploadPhase>({ kind: 'idle' });
  const [selectedCategoryId, setSelectedCategoryId] = useState<number | null>(null);
  const [replaceByName, setReplaceByName] = useState(false);
+  // Upload IDs returned from each chunk POST. The processing tracker
+  // 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[]>([]);
  const fileInputRef = useRef<HTMLInputElement>(null);
+
+  const { aggregate: processingAggregate } = useUploadProgress(uploadIds, {
+    enabled: phase.kind === 'processing' && uploadIds.length > 0,
+  });
  
  // Fetch categories for this event
  const { data: categories = [] } = useQuery({
@@ -107,6 +126,7 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl

    setIsUploading(true);
    setUploadProgress(0);
+    setUploadIds([]);

    // For large uploads, chunk the files by both count AND size to prevent memory/network issues
    const MAX_FILES_PER_CHUNK = Math.max(1, Math.min(50, maxFilesPerUpload)); // Max 50 files per chunk
@@ -144,11 +164,11 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ 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,63 +176,138 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ 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),
+                  });
+                }
              }
            },
          });

          totalUploaded += (response.data?.successCount || chunk.length);
          totalReplaced += (response.data?.replacedCount || 0);
+          // Backend returns a per-request upload_id. Track it so the
+          // processing-status hook can poll/stream live progress.
+          if (response.data?.upload_id) {
+            const newId = response.data.upload_id as string;
+            setUploadIds((prev) => (prev.includes(newId) ? prev : [...prev, newId]));
+          }
        } 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;
        }
      }

-      // Clear selected files
+      // Bytes are all on the server. Clear the file picker so the
+      // user can queue another batch — but DON'T dismiss the upload
+      // UI yet; we'll watch the processing aggregate (useEffect below)
+      // to know when the backend has finished generating thumbnails
+      // and metadata.
      setSelectedFiles([]);
      if (fileInputRef.current) {
        fileInputRef.current.value = '';
      }

-      // Show appropriate message
      if (totalReplaced > 0) {
        toast.info(t('upload.replacedFiles', { count: totalReplaced }) || `${totalReplaced} photo(s) replaced`);
      }
-      if (failedFiles.length === 0) {
-        toast.success(t('upload.uploadComplete') || `Successfully uploaded ${totalUploaded} files`);
-      } else {
+      if (failedFiles.length > 0) {
        toast.warning(
-          t('upload.someFilesFailed') || 
-          `Uploaded ${totalUploaded} files. ${failedFiles.length} files failed.`
+          t('upload.someFilesFailed') ||
+          `Transferred ${totalUploaded} files. ${failedFiles.length} files failed to transfer.`
        );
      }
-      
-      // Call callback
+
+      // Refresh the grid early so the user sees their photos appearing
+      // as the worker processes them. The processing-aggregate effect
+      // below will refresh again on completion.
      if (onUploadComplete) {
        onUploadComplete();
      }
+
+      // 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'));
-    } finally {
      setIsUploading(false);
      setUploadProgress(0);
      setCurrentChunk(0);
      setTotalChunks(0);
+      setPhase({ kind: 'idle' });
+      setUploadIds([]);
    }
  };

+  // When the background worker finishes processing every photo from
+  // this upload, dismiss the upload UI and surface the result.
+  useEffect(() => {
+    if (!isUploading) return;
+    if (uploadIds.length === 0) return;
+    if (!processingAggregate.isComplete) return;
+
+    if (processingAggregate.failed > 0) {
+      toast.warning(
+        t('upload.processingFailed', { count: processingAggregate.failed }) ||
+          `${processingAggregate.failed} photo(s) failed to process`
+      );
+    } else {
+      toast.success(
+        t('upload.uploadComplete') || `Successfully uploaded ${processingAggregate.complete} photo(s)`
+      );
+    }
+
+    if (onUploadComplete) onUploadComplete();
+    setIsUploading(false);
+    setUploadProgress(0);
+    setCurrentChunk(0);
+    setTotalChunks(0);
+    setPhase({ kind: 'idle' });
+    setUploadIds([]);
+    // We intentionally only react to processingAggregate.isComplete /
+    // .failed — the rest of the deps either don't move during this
+    // effect's lifetime or are stable callbacks.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [processingAggregate.isComplete, processingAggregate.failed, isUploading]);
+
  const formatFileSize = (bytes: number) => {
    if (bytes < 1024) return bytes + ' B';
    if (bytes < 1024 * 1024) return (bytes / 1024).toFixed(1) + ' KB';
@@ -344,26 +439,73 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
        </Button>
      </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 && (
        <div className="mt-4">
-          <div className="flex justify-between text-sm text-neutral-600 dark:text-neutral-400 mb-1">
-            <span>
-              {t('upload.uploading')}
-              {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>
+          {phase.kind === 'processing' ? (
+            <div className="rounded-lg border border-amber-200 dark:border-amber-800 bg-amber-50 dark:bg-amber-900/20 p-4">
+              <div className="flex items-start gap-3">
+                <Cog className="w-5 h-5 text-amber-600 dark:text-amber-400 animate-spin shrink-0 mt-0.5" />
+                <div className="flex-1 min-w-0">
+                  <p className="text-sm font-medium text-amber-900 dark:text-amber-100">
+                    {t('upload.processing')}
+                  </p>
+                  {processingAggregate.total > 0 && (
+                    <>
+                      <p className="text-xs text-amber-900 dark:text-amber-100 font-medium mt-2">
+                        {t('upload.processingProgress', {
+                          complete: processingAggregate.complete + processingAggregate.failed,
+                          total: processingAggregate.total,
+                        })}
+                      </p>
+                      <div className="w-full bg-amber-100 dark:bg-amber-900/40 rounded-full h-2 mt-1">
+                        <div
+                          className="bg-amber-600 dark:bg-amber-500 h-2 rounded-full transition-all duration-300"
+                          style={{
+                            width: `${
+                              processingAggregate.total === 0
+                                ? 0
+                                : Math.round(
+                                    ((processingAggregate.complete + processingAggregate.failed) /
+                                      processingAggregate.total) *
+                                      100
+                                  )
+                            }%`,
+                          }}
+                        />
+                      </div>
+                    </>
+                  )}
+                  <p className="text-xs text-amber-800 dark:text-amber-200 mt-2">
+                    {t('upload.processingHint')}
+                  </p>
+                </div>
+              </div>
+            </div>
+          ) : (
+            <>
+              <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>
      )}
@@ -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<UserPhotoUploadProps> = ({
  const [files, setFiles] = useState<File[]>([]);
  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<UserPhotoUploadProps> = ({
                ...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<UserPhotoUploadProps> = ({
                    </div>
                    {uploadProgress[file.name] !== undefined ? (
                      <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" />
                        ) : (
                          <div className="w-20">