diff --git a/backend/__tests__/routes/galleryUploadStatus.test.js b/backend/__tests__/routes/galleryUploadStatus.test.js new file mode 100644 index 00000000..6e72afb0 --- /dev/null +++ b/backend/__tests__/routes/galleryUploadStatus.test.js @@ -0,0 +1,235 @@ +/** + * Guest-facing upload processing status + cache headers on the gallery router + * (testplan REPORT.md B6 / B7). + * + * B7: a guest upload is queued — POST /gallery/:eventId/upload answers 202 and + * /gallery/:slug/photos only returns rows that reached 'complete'. Without a + * status signal the gallery has to poll the photo list blind and cannot tell a + * slow worker from a photo that failed outright. The contract that matters + * most here is the authorization scope: the endpoint is keyed on an opaque + * upload_id, so it must never report on an upload belonging to a gallery the + * caller's token did not unlock. + * + * B6: the private per-guest JSON on this router carried no Cache-Control at + * all and fell back to heuristic freshness. The media routes must keep their + * own caching — the point of the change is that it is per route, not global. + */ + +const request = require('supertest'); +const express = require('express'); +const cookieParser = require('cookie-parser'); +const jwt = require('jsonwebtoken'); + +const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb'); + +process.env.JWT_SECRET = process.env.JWT_SECRET || 'upload-status-test-secret'; + +const SLUG_A = 'upload-status-a'; +const SLUG_B = 'upload-status-b'; + +// Shape of a real guest upload id: crypto.randomBytes(16).toString('hex'). +const UPLOAD_A = 'a1b2c3d4e5f60718293a4b5c6d7e8f90'; +const UPLOAD_A2 = '0f1e2d3c4b5a69788796a5b4c3d2e1f0'; +const UPLOAD_B = 'ffeeddccbbaa99887766554433221100'; + +describe('gallery upload status + cache headers (B6/B7)', () => { + let db; + let cleanup; + let app; + let eventA; + let eventB; + + const galleryToken = (eventId, slug, extra = {}) => jwt.sign( + { eventId, eventSlug: slug, type: 'gallery', ...extra }, + process.env.JWT_SECRET, + { expiresIn: '1h', issuer: 'picpeak-auth' } + ); + + const createEvent = async (slug, name) => { + const inserted = await db('events').insert({ + slug, + event_type: 'wedding', + event_name: name, + event_date: '2026-08-01', + host_email: 'host@example.com', + admin_email: 'admin@example.com', + password_hash: 'x', + share_link: `/gallery/${slug}/share`, + expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(), + is_active: 1, + is_archived: 0, + is_draft: 0, + allow_user_uploads: 1, + created_at: new Date().toISOString(), + }).returning('id'); + return inserted[0]?.id ?? inserted[0]; + }; + + // `status: undefined` writes no processing_status at all, so the column + // default ('complete', migration 085) applies — the shape a row imported by + // a path that predates async processing has. + const addPhoto = async (eventId, filename, uploadId, status) => { + await db('photos').insert({ + event_id: eventId, + filename, + path: `events/${filename}`, + type: 'individual', + upload_id: uploadId, + ...(status ? { processing_status: status } : {}), + uploaded_at: new Date().toISOString(), + }); + }; + + const status = (slug, token, query) => request(app) + .get(`/api/gallery/${slug}/uploads/status`) + .query(query) + .set('Authorization', `Bearer ${token}`); + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + await seedMinimal(db); + + eventA = await createEvent(SLUG_A, 'Upload Status A'); + eventB = await createEvent(SLUG_B, 'Upload Status B'); + + // Gallery A: one settled group (complete + failed) and one still queued. + await addPhoto(eventA, 'a-complete.jpg', UPLOAD_A, 'complete'); + await addPhoto(eventA, 'a-failed.jpg', UPLOAD_A, 'failed'); + await addPhoto(eventA, 'a-pending.jpg', UPLOAD_A2, 'pending'); + await addPhoto(eventA, 'a-processing.jpg', UPLOAD_A2, 'processing'); + // Row written without an explicit status — takes the column default. + await addPhoto(eventA, 'a-legacy.jpg', UPLOAD_A2, undefined); + + // Gallery B: the group a guest of A must never be able to read. + await addPhoto(eventB, 'b-pending.jpg', UPLOAD_B, 'pending'); + + app = express(); + app.use(express.json()); + app.use(cookieParser()); + app.use('/api/gallery', require('../../src/routes/gallery')); + }, 180000); + + afterAll(async () => { + if (cleanup) await cleanup(); + }); + + describe('B7 — processing status', () => { + it('summarises the caller\'s own upload group', async () => { + const res = await status(SLUG_A, galleryToken(eventA, SLUG_A), { ids: UPLOAD_A }); + expect(res.status).toBe(200); + expect(res.body).toEqual({ total: 2, pending: 0, processing: 0, complete: 1, failed: 1 }); + }); + + it('reports a batch of upload ids in one request', async () => { + const res = await status(SLUG_A, galleryToken(eventA, SLUG_A), { + ids: `${UPLOAD_A},${UPLOAD_A2}`, + }); + expect(res.status).toBe(200); + expect(res.body).toEqual({ total: 5, pending: 1, processing: 1, complete: 2, failed: 1 }); + }); + + it('rejects a missing, malformed or oversized id list', async () => { + const token = galleryToken(eventA, SLUG_A); + expect((await status(SLUG_A, token, {})).status).toBe(400); + expect((await status(SLUG_A, token, { ids: '' })).status).toBe(400); + expect((await status(SLUG_A, token, { ids: 'not a valid id' })).status).toBe(400); + expect((await status(SLUG_A, token, { ids: `${UPLOAD_A},oops!` })).status).toBe(400); + // 51 well-formed ids — one over the cap that bounds the IN-list. + const tooMany = Array.from({ length: 51 }, (_, i) => UPLOAD_A.slice(0, 30) + String(i % 10) + '0').join(','); + expect((await status(SLUG_A, token, { ids: tooMany })).status).toBe(400); + }); + + it('never reports on another gallery\'s upload group', async () => { + // A valid guest of gallery B, asking about gallery A's upload id. + const res = await status(SLUG_B, galleryToken(eventB, SLUG_B), { ids: UPLOAD_A }); + expect(res.status).toBe(200); + // Filtered on event_id, so the rows simply do not exist for this caller — + // no counts, and no "this id exists elsewhere" oracle either. + expect(res.body).toEqual({ total: 0, pending: 0, processing: 0, complete: 0, failed: 0 }); + }); + + it('cannot be reached by pointing a gallery-B token at gallery A\'s slug', async () => { + const res = await status(SLUG_A, galleryToken(eventB, SLUG_B), { ids: UPLOAD_A }); + expect(res.status).toBe(403); + }); + + it('requires a gallery token and refuses display-only slideshow tokens', async () => { + const anon = await request(app) + .get(`/api/gallery/${SLUG_A}/uploads/status`) + .query({ ids: UPLOAD_A }); + expect(anon.status).toBe(401); + + const kiosk = await status(SLUG_A, galleryToken(eventA, SLUG_A, { accessLevel: 'slideshow' }), { + ids: UPLOAD_A, + }); + expect(kiosk.status).toBe(403); + }); + }); + + describe('B6 — cache headers', () => { + const noStore = (res) => { + expect(res.headers['cache-control']).toBe('no-store, no-cache, must-revalidate, private'); + expect(res.headers.pragma).toBe('no-cache'); + }; + + it('marks the private per-guest JSON routes no-store', async () => { + const token = galleryToken(eventA, SLUG_A); + noStore(await status(SLUG_A, token, { ids: UPLOAD_A })); + + const photos = await request(app) + .get(`/api/gallery/${SLUG_A}/photos`) + .set('Authorization', `Bearer ${token}`); + expect(photos.status).toBe(200); + noStore(photos); + + const stats = await request(app) + .get(`/api/gallery/${SLUG_A}/stats`) + .set('Authorization', `Bearer ${token}`); + expect(stats.status).toBe(200); + noStore(stats); + + const people = await request(app) + .get(`/api/gallery/${SLUG_A}/people`) + .set('Authorization', `Bearer ${token}`); + expect(people.status).toBe(200); + noStore(people); + }); + + it('still lets /photos answer a conditional request with a 304', async () => { + // The post-upload poll depends on revalidation staying correct: no-store + // stops the browser retaining the body, it must not stop express from + // agreeing that an unchanged payload is unchanged. + const token = galleryToken(eventA, SLUG_A); + const first = await request(app) + .get(`/api/gallery/${SLUG_A}/photos`) + .set('Authorization', `Bearer ${token}`); + expect(first.headers.etag).toBeTruthy(); + + const second = await request(app) + .get(`/api/gallery/${SLUG_A}/photos`) + .set('Authorization', `Bearer ${token}`) + .set('If-None-Match', first.headers.etag); + expect(second.status).toBe(304); + }); + + it('leaves the cacheable routes alone', async () => { + // noStoreCache is mounted per route, not on the router, precisely so the + // media/asset routes keep their own long-lived caching. + await db('events').where({ id: eventA }).update({ css_template_id: null }); + const css = await request(app).get(`/api/gallery/${SLUG_A}/css-template`); + expect(css.headers['cache-control']).toBeUndefined(); + + const [tpl] = await db('css_templates').insert({ + name: 'Upload Status Test', + slot_number: 99, + css_content: 'body { color: red; }', + is_enabled: 1, + }).returning('id'); + await db('events').where({ id: eventA }).update({ css_template_id: tpl?.id ?? tpl }); + + const cached = await request(app).get(`/api/gallery/${SLUG_A}/css-template`); + expect(cached.status).toBe(200); + expect(cached.headers['cache-control']).toBe('public, max-age=3600'); + }); + }); +}); diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index dab40a64..6504ba2e 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -30,6 +30,14 @@ const { verifyGalleryAccess, denySlideshowToken, isAdminPreview } = require('../ // fall back to the draft/password gate and 404 the derivative. const withPreview = (req, url) => (req.isAdminPreview ? `${url}${url.includes('?') ? '&' : '?'}admin_preview=1` : url); const { resolveGuest } = require('../middleware/guestAuth'); +// Private, per-guest JSON on this router carries no explicit Cache-Control, so +// browsers fall back to heuristic freshness and may serve a stale body from +// disk for a session-scoped surface. `noStoreCache` is mounted per route rather +// than on the whole router on purpose: the image/thumbnail/hero/preview and +// css-template routes below set their own long-lived caching headers and MUST +// keep them — re-fetching every derivative on every scroll is the reason those +// headers exist. +const { noStoreCache } = require('../middleware/noStoreCache'); const { generateGuestIdentifier } = require('../middleware/feedbackRateLimit'); const { COLOR_LABELS, SHARED_COLOR_LABEL_IDENTITY } = require('../constants/colorLabels'); const secureImageService = require('../services/secureImageService'); @@ -235,8 +243,9 @@ router.get('/resolve/:identifier', handleAsync(async (req, res) => { }); })); -// Verify share token -router.get('/:slug/verify-token/:token', handleAsync(async (req, res) => { +// Verify share token. no-store: this is an authorization decision — a cached +// `{ valid: true }` would keep answering for a token the admin has rotated. +router.get('/:slug/verify-token/:token', noStoreCache, handleAsync(async (req, res) => { const { slug, token } = req.params; const event = await db('events') @@ -607,7 +616,9 @@ async function slideshowQrDataUrl(event, req) { // JWT scoped to `accessLevel:'slideshow'` (treated as a guest by the photo / // image endpoints → visible photos only, no client-only/hidden). The page // stores this token and the existing axios interceptor injects it. -router.get('/:slug/show/:token/session', handleAsync(async (req, res) => { +// no-store: this response *is* a credential (it mints a gallery JWT and sets +// the per-slug auth cookie), so it must never be retained anywhere. +router.get('/:slug/show/:token/session', noStoreCache, handleAsync(async (req, res) => { const { slug, token } = req.params; const event = await resolveSlideshow(slug, token); if (!event) { @@ -649,7 +660,7 @@ router.get('/:slug/show/:token/session', handleAsync(async (req, res) => { // current settings + the visible photo count. The page diffs photo_count to // decide when to refetch the full list, and re-reads settings so admin changes // take effect live. A dead/disabled link 404s here → the projector stops. -router.get('/:slug/show/:token/state', handleAsync(async (req, res) => { +router.get('/:slug/show/:token/state', noStoreCache, handleAsync(async (req, res) => { const { slug, token } = req.params; const event = await resolveSlideshow(slug, token); if (!event) { @@ -665,8 +676,16 @@ router.get('/:slug/show/:token/state', handleAsync(async (req, res) => { }); })); -// Get all photos -router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) => { +// Get all photos. +// +// no-store (B6): the payload is private and per-guest — it carries the +// viewer's own likes/favorites/ratings and, for a client token, photos hidden +// from plain guests. With no Cache-Control at all a browser applies heuristic +// freshness and may reuse a body it stored on disk, on a shared device, for a +// gallery whose password has since been rotated. Express still computes its +// weak ETag, so a caller that does revalidate (React Query's own in-memory +// cache is unaffected either way) still gets a correct 304. +router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, noStoreCache, async (req, res) => { try { // Get filter and sort parameters from query // `guest_id` is deliberately NOT read from the query string: the viewer's @@ -1423,7 +1442,9 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) * Counts and cover faces are computed against the caller's own visibility * scope inside facePeopleService; nothing here reads face_count_total. */ -router.get('/:slug/people', verifyGalleryAccess, resolveGuest, async (req, res) => { +// no-store for the same reason as /photos: the people list and its scan +// progress are scoped to what THIS viewer may see. +router.get('/:slug/people', verifyGalleryAccess, resolveGuest, noStoreCache, async (req, res) => { try { const isClient = req.accessLevel === 'client'; const { isEnabledForEvent, areFacesVisibleToGuests, getThresholds } = @@ -2333,7 +2354,9 @@ router.post('/:slug/download-jobs', verifyGalleryAccess, denySlideshowToken, blo // Poll. The token is unguessable, but it is never sufficient on its own — // verifyGalleryAccess still runs and the job must belong to THIS event. -router.get('/:slug/download-jobs/:token', verifyGalleryAccess, denySlideshowToken, blockHiddenGallery, async (req, res) => { +// no-store: a cached 'preparing' would strand the caller in a poll that can +// never observe the job finishing. +router.get('/:slug/download-jobs/:token', verifyGalleryAccess, denySlideshowToken, blockHiddenGallery, noStoreCache, async (req, res) => { try { const job = await downloadJobService.getStatus(req.params.token); if (!job || job.event_id !== req.event.id) { @@ -3043,8 +3066,9 @@ router.get('/:slug/preview/:photoId', // (#655) from the guest payload, so the gallery could never render the // favorite/like limits or their counters (#1030). -// Get photo stats -router.get('/:slug/stats', verifyGalleryAccess, blockHiddenGallery, async (req, res) => { +// Get photo stats. no-store: view/download/visitor counters are private +// gallery analytics and change on every request. +router.get('/:slug/stats', verifyGalleryAccess, blockHiddenGallery, noStoreCache, async (req, res) => { try { const totalPhotos = await db('photos') .where('event_id', req.event.id) @@ -3211,6 +3235,70 @@ router.post('/:eventId/upload', verifyGalleryAccess, denySlideshowToken, async ( } }); +// A guest upload_id is `crypto.randomBytes(16).toString('hex')` +// (photoProcessor.js). The pattern is deliberately a little wider than that so +// an id-format change does not silently 400, but narrow enough that the value +// can only ever be an opaque token. +const UPLOAD_ID_PATTERN = /^[A-Za-z0-9_-]{8,64}$/; +// The guest UI uploads one file per request, so a batch of N files yields N +// upload ids. Batching them into a single poll keeps the request rate flat +// regardless of batch size; the cap bounds the IN-list. +const MAX_UPLOAD_STATUS_IDS = 50; + +/** + * GET /:slug/uploads/status?ids=[,…] + * + * Guest-facing processing status for the guest's own uploads (B7). + * + * The upload route answers 202 and queues the files, and /photos only returns + * rows that reached `processing_status: 'complete'`. Without this the gallery + * had to poll /photos blind, could not say "processing…", and could not tell a + * slow worker from a photo that failed outright — the guest just watched their + * upload not appear. + * + * Authorization: `verifyGalleryAccess` already resolved `req.event` from the + * caller's gallery token, and the query is filtered on `event_id = req.event.id` + * as well as the ids. An id belonging to another gallery therefore matches no + * row rather than being reported as forbidden — no cross-event read, and no + * existence oracle either. Slideshow tokens are denied because a kiosk never + * uploads. + * + * The response is counts only. The guest already knows which files they sent; + * anything more (filenames, `processing_error` strings, which can carry + * internal paths) would be leaking beyond "how far along is my upload". + */ +router.get('/:slug/uploads/status', verifyGalleryAccess, denySlideshowToken, noStoreCache, async (req, res) => { + try { + const ids = String(req.query.ids || '') + .split(',') + .map((id) => id.trim()) + .filter(Boolean); + + if (ids.length === 0 || ids.length > MAX_UPLOAD_STATUS_IDS || !ids.every((id) => UPLOAD_ID_PATTERN.test(id))) { + return res.status(400).json({ error: 'Invalid upload ids' }); + } + + const rows = await db('photos') + .where('event_id', req.event.id) + .whereIn('upload_id', ids) + .select('processing_status'); + + const summary = { total: rows.length, pending: 0, processing: 0, complete: 0, failed: 0 }; + for (const row of rows) { + // NULL is a pre-async-migration row, treated as complete exactly as the + // /photos filter treats it. + const status = row.processing_status || 'complete'; + if (Object.prototype.hasOwnProperty.call(summary, status) && status !== 'total') { + summary[status] += 1; + } + } + + res.json(summary); + } catch (error) { + errorResponse(res, error, 500, 'Failed to read upload status'); + } +}); + /** * GET /:slug/css-template * Get custom CSS template for gallery (public endpoint) diff --git a/frontend/src/components/gallery/GalleryView.tsx b/frontend/src/components/gallery/GalleryView.tsx index cf3f93da..56b0c6a8 100644 --- a/frontend/src/components/gallery/GalleryView.tsx +++ b/frontend/src/components/gallery/GalleryView.tsx @@ -35,7 +35,8 @@ import type { FilterType, FeedbackFilterType } from './GalleryFilter'; import { analyticsService } from '../../services/analytics.service'; import { useDevToolsProtection } from '../../hooks/useDevToolsProtection'; import { api } from '../../config/api'; -import { Upload, Menu, Eye, EyeOff, Shield, X, Download, ChevronLeft } from 'lucide-react'; +import { Upload, Menu, Eye, EyeOff, Shield, X, Download, ChevronLeft, Loader2 } from 'lucide-react'; +import { toast } from 'react-toastify'; import { galleryService } from '../../services/gallery.service'; import { feedbackService, type ColorLabel } from '../../services/feedback.service'; import { useWatermarkSettings } from '../../hooks/useWatermarkSettings'; @@ -255,9 +256,16 @@ export const GalleryView: React.FC = ({ slug, event, requiresP // the photo list only returns completed rows. A single immediate refetch // therefore comes back with a byte-identical payload (which the browser is // answered with a 304), so the guest saw their upload silently vanish until - // they hard-reloaded. Poll for a short while until the queued photos finish - // processing instead of refetching — or reloading the page — exactly once. + // they hard-reloaded. + // + // The first fix polled the photo list blind against a count baseline, which + // cannot tell a slow worker from a photo that failed processing — it just + // stopped after 60s with nothing on screen either way. Poll the upload + // group's real processing status instead (B7): it drives the "processing…" + // notice, refetches the grid as photos land rather than only at the end, and + // reports a failure instead of a silence. const uploadRefreshTimerRef = useRef | null>(null); + const [uploadProcessing, setUploadProcessing] = useState<{ complete: number; total: number } | null>(null); const stopUploadRefresh = () => { if (uploadRefreshTimerRef.current) { clearInterval(uploadRefreshTimerRef.current); @@ -266,25 +274,87 @@ export const GalleryView: React.FC = ({ slug, event, requiresP }; useEffect(() => stopUploadRefresh, []); - const handleUploadComplete = (queuedCount = 1) => { + const handleUploadComplete = (uploadIds: string[] = []) => { setShowUploadModal(false); - const baseline = data?.photos?.length ?? 0; - // Each file is processed independently, so stopping at the FIRST new photo - // leaves the rest of a multi-file upload hidden until a manual refresh — - // the very symptom this polling exists to prevent. Wait for all of them. - const target = baseline + Math.max(1, queuedCount); - const deadline = Date.now() + 60_000; stopUploadRefresh(); + + // Nothing to follow (no id came back, e.g. every file failed on the wire). + // Refetch once rather than polling something unknowable. + if (uploadIds.length === 0) { + void refetch(); + return; + } + + setUploadProcessing({ complete: 0, total: uploadIds.length }); + const deadline = Date.now() + 120_000; + let lastComplete = 0; + let inFlight = false; + + const finish = async (announce?: () => void) => { + stopUploadRefresh(); + setUploadProcessing(null); + await refetch(); + announce?.(); + }; + const poll = async () => { - const result = await refetch(); - if ((result.data?.photos?.length ?? 0) >= target || Date.now() > deadline) { - stopUploadRefresh(); + // The interval keeps firing while a slow request is open; without this + // the requests stack up for the whole deadline. + if (inFlight) return; + inFlight = true; + try { + const status = await galleryService.getUploadStatus(slug, uploadIds); + setUploadProcessing({ + complete: status.complete + status.failed, + total: status.total || uploadIds.length, + }); + + // Refetch as each photo lands, not only once the batch settles, so a + // large upload fills the grid progressively. + if (status.complete > lastComplete) { + lastComplete = status.complete; + void refetch(); + } + + if (status.pending === 0 && status.processing === 0) { + await finish(() => { + if (status.failed > 0) { + toast.error(t('upload.processingFailed', { count: status.failed })); + } + }); + } else if (Date.now() > deadline) { + // Bounded. The worker is genuinely still running, so say that rather + // than leaving the guest with a grid that quietly never updated. + await finish(() => toast.info(t('upload.processingStillRunning'))); + } + } catch { + // The status signal is a convenience — the photos are stored either + // way — so a failing status call degrades to the plain refetch. + await finish(); + } finally { + inFlight = false; } }; + uploadRefreshTimerRef.current = setInterval(poll, 2000); - poll(); + void poll(); }; + // The two layout branches below that render the photo grid have no shared + // wrapper, so the notice is shared as a value rather than as markup. + const uploadProcessingNotice = uploadProcessing ? ( +
+ + + {t('upload.processing')}{' '} + {t('upload.processingProgress', { + complete: uploadProcessing.complete, + total: uploadProcessing.total, + })} + +
+ ) : null; + // Get individual protection settings from event const disableRightClick = data?.event?.disable_right_click === true; const enableDevtoolsProtection = data?.event?.enable_devtools_protection === true; @@ -1393,6 +1463,7 @@ export const GalleryView: React.FC = ({ slug, event, requiresP onClose={() => setShowUploadModal(false)} /> )} + {uploadProcessingNotice} {/* Download size picker (#858) — "download all", or a selection. */} {(showResolutionPicker || resolutionPickerIds) && ( @@ -1800,6 +1871,7 @@ export const GalleryView: React.FC = ({ slug, event, requiresP onClose={() => setShowUploadModal(false)} /> )} + {uploadProcessingNotice} {/* Download size picker (#858) — "download all", or a selection. */} {(showResolutionPicker || resolutionPickerIds) && ( diff --git a/frontend/src/components/gallery/UserPhotoUpload.tsx b/frontend/src/components/gallery/UserPhotoUpload.tsx index 736d4d39..256f4ed1 100644 --- a/frontend/src/components/gallery/UserPhotoUpload.tsx +++ b/frontend/src/components/gallery/UserPhotoUpload.tsx @@ -10,8 +10,9 @@ import { extensionsToMimeTypes, buildUploadAcceptString, extensionsToLabel } fro interface UserPhotoUploadProps { eventId: number; categoryId: number | null | undefined; - /** Receives how many files the server accepted, so the caller can wait for all of them. */ - onUploadComplete: (queuedCount: number) => void; + // Receives the upload-group ids the backend queued the files under, so the + // caller can poll their processing status instead of guessing (B7). + onUploadComplete: (uploadIds: string[]) => void; onClose: () => void; } @@ -143,6 +144,10 @@ export const UserPhotoUpload: React.FC = ({ setUploading(true); let successCount = 0; let failedCount = 0; + // The 202 hands back the id of the upload group the files were queued + // under. One request per file means one id per file; the gallery polls + // them together to know when the background worker is done (B7). + const uploadIds: string[] = []; for (const file of files) { const formData = new FormData(); @@ -152,7 +157,7 @@ export const UserPhotoUpload: React.FC = ({ } try { - await api.post(`/gallery/${eventId}/upload`, formData, { + const response = await api.post<{ upload_id?: string }>(`/gallery/${eventId}/upload`, formData, { headers: { 'Content-Type': 'multipart/form-data', }, @@ -169,7 +174,11 @@ export const UserPhotoUpload: React.FC = ({ } }, }); - // Request resolved → file fully processed by backend. + // Request resolved → the bytes are stored. Processing continues in the + // background worker; `upload_id` is how the gallery follows it. + if (response.data?.upload_id) { + uploadIds.push(response.data.upload_id); + } setProcessingFiles(prev => { const next = { ...prev }; delete next[file.name]; @@ -190,7 +199,7 @@ export const UserPhotoUpload: React.FC = ({ if (successCount > 0) { toast.success(t('toast.uploadSuccess') + ` (${successCount} ${t('common.photos')})`); - onUploadComplete(successCount); + onUploadComplete(uploadIds); } if (failedCount > 0) { diff --git a/frontend/src/components/gallery/__tests__/galleryUploadRefresh.test.ts b/frontend/src/components/gallery/__tests__/galleryUploadRefresh.test.ts index e84631ce..d975c8b6 100644 --- a/frontend/src/components/gallery/__tests__/galleryUploadRefresh.test.ts +++ b/frontend/src/components/gallery/__tests__/galleryUploadRefresh.test.ts @@ -1,5 +1,6 @@ /** - * A guest upload must show up in the grid on its own. + * A guest upload must show up in the grid on its own — and say so while it is + * still being worked on. * * Guest uploads are queued: `POST /gallery/:id/upload` answers 202 and the row * lands as `processing_status: 'pending'`, while `GET /gallery/:slug/photos` @@ -8,6 +9,10 @@ * the payload was still byte-identical, the browser was answered 304, and the * guest's photo silently vanished until they hard-reloaded (QA P4-E.01). * + * The follow-up (B7) replaced the blind count-baseline poll with one driven by + * the real processing status of the guest's own upload group, so the UI can + * show "processing…" and report a failure instead of timing out in silence. + * * GalleryView needs its providers, the router and a dozen child components to * render, so this pins the contract at source level (same approach as * facePreviewRendition.test.ts). @@ -16,42 +21,75 @@ import { describe, it, expect } from 'vitest'; import fs from 'fs'; import path from 'path'; -const source = fs.readFileSync( - path.join(__dirname, '..', 'GalleryView.tsx'), +const read = (...parts: string[]) => + fs.readFileSync(path.join(__dirname, '..', ...parts), 'utf8'); + +const source = read('GalleryView.tsx'); +const uploadSource = read('UserPhotoUpload.tsx'); +const serviceSource = fs.readFileSync( + path.join(__dirname, '..', '..', '..', 'services', 'gallery.service.ts'), 'utf8' ); +const handler = source.slice( + source.indexOf('const handleUploadComplete'), + source.indexOf('const uploadProcessingNotice') +); + describe('post-upload photo refresh', () => { it('never reloads the page to pick up an upload', () => { expect(source).not.toContain('window.location.reload'); }); - it('keeps refetching until the queued photos appear', () => { - const handler = source.slice( - source.indexOf('const handleUploadComplete'), - source.indexOf('// Get individual protection settings') - ); - - expect(handler).toContain('await refetch()'); + it('drives the refresh off the upload group\'s processing status', () => { + expect(handler).toContain('galleryService.getUploadStatus(slug, uploadIds)'); + // Refetch as photos land, not only once the whole batch settles. + expect(handler).toContain('status.complete > lastComplete'); expect(handler).toMatch(/setInterval\(poll/); - // Bounded: stop once the new photos land, and stop regardless after the - // deadline so a failed background job can't leave a poll running forever. - // - // Waits for ALL queued files, not just the first. Each is processed - // independently, so a `> baseline` comparison stops at photo 1 of N and - // leaves the rest hidden until a manual refresh — the exact symptom this - // polling exists to prevent. - expect(handler).toContain('baseline + Math.max(1, queuedCount)'); - expect(handler).toContain('>= target'); + }); + + it('stops on the real terminal condition rather than a count baseline', () => { + expect(handler).toContain('status.pending === 0 && status.processing === 0'); + // Still bounded, so a wedged worker can never leave a poll running forever. expect(handler).toContain('Date.now() > deadline'); }); + it('tells the guest when a photo failed processing or is still queued', () => { + expect(handler).toContain("toast.error(t('upload.processingFailed'"); + expect(handler).toContain("toast.info(t('upload.processingStillRunning')"); + // ...and renders a "processing…" notice while the poll runs. + expect(source).toContain("t('upload.processing')"); + expect(source).toContain("t('upload.processingProgress'"); + }); + + it('degrades to a plain refetch when the status call itself fails', () => { + expect(handler).toContain('} catch {'); + expect(handler).toContain('await finish();'); + }); + it('wires the polling handler into the upload modals that render the grid', () => { const wired = source.match(/onUploadComplete=\{handleUploadComplete\}/g) || []; expect(wired.length).toBeGreaterThanOrEqual(2); + // The notice is rendered next to each of them; the two layout branches + // have no shared wrapper to hang it on. + const shown = source.match(/\{uploadProcessingNotice\}/g) || []; + expect(shown.length).toBe(wired.length); }); it('clears the poll when the gallery unmounts', () => { expect(source).toContain('useEffect(() => stopUploadRefresh, [])'); }); }); + +describe('upload id plumbing', () => { + it('hands the 202 upload ids to the gallery', () => { + expect(uploadSource).toContain('onUploadComplete: (uploadIds: string[]) => void'); + expect(uploadSource).toContain('uploadIds.push(response.data.upload_id)'); + expect(uploadSource).toContain('onUploadComplete(uploadIds)'); + }); + + it('asks the gallery-scoped status route, batching the ids into one request', () => { + expect(serviceSource).toContain('`/gallery/${slug}/uploads/status`'); + expect(serviceSource).toContain("params: { ids: uploadIds.join(',') }"); + }); +}); diff --git a/frontend/src/services/gallery.service.ts b/frontend/src/services/gallery.service.ts index 57f72875..8f9f53ff 100644 --- a/frontend/src/services/gallery.service.ts +++ b/frontend/src/services/gallery.service.ts @@ -44,6 +44,15 @@ function isIOS(): boolean { // the existing server-side zip flow. const MAX_WEB_SHARE_FILES = 25; +// Counts-only snapshot returned by GET /gallery/:slug/uploads/status. +export interface UploadProcessingStatus { + total: number; + pending: number; + processing: number; + complete: number; + failed: number; +} + export const galleryService = { // Verify share token async verifyToken(slug: string, token: string): Promise<{ valid: boolean }> { @@ -89,6 +98,18 @@ export const galleryService = { }; }, + // Processing status for the guest's own uploads (B7). A guest upload is + // queued — the route answers 202 and getGalleryPhotos only returns rows that + // finished processing — so this is what tells the gallery whether a photo + // that has not appeared yet is still in the worker's queue or failed + // outright. Scoped server-side to the gallery this token unlocked. + async getUploadStatus(slug: string, uploadIds: string[]): Promise { + const response = await api.get(`/gallery/${slug}/uploads/status`, { + params: { ids: uploadIds.join(',') }, + }); + return response.data; + }, + // Save single photo. iOS routes through the Web Share API so the // share sheet's "Save Image" action lands the file in Photos. // Everywhere else (Android, desktop) navigates a hidden anchor