fix(gallery): no-store private JSON, and give guest uploads a real status
B6 -- seven gallery routes returned private, per-guest data with no
Cache-Control at all, relying on heuristic freshness. noStoreCache is mounted
per route rather than on the router, because the media routes set their own
private, max-age=1800/3600 and must keep it. Covered: /photos (own
likes/favourites/ratings, hidden photos for a client token), /people, /stats,
/verify-token/:token (an authorization decision -- a cached {valid:true}
outlives a rotated token), /show/:token/session (the response IS a credential;
it mints a gallery JWT), /show/:token/state and /download-jobs/:token (live
polls, where a cached "preparing" strands the caller). Deliberately untouched:
the photo/thumbnail/hero/preview and css-template routes, which set their own
caching, the binary downloads, and /info + /resolve, which are unauthenticated
public metadata rather than per-guest private.
ETag/304 revalidation is intact and pinned by a test: no-store stops the
browser retaining the body, not express agreeing an unchanged payload is
unchanged. That matters because the post-upload poll depends on it.
B7 -- the guest upload flow had no progress signal, so the UI polled the photo
list blind and gave up after 60s with no explanation. Adds
GET /:slug/uploads/status?ids=... rather than pending counts in the photos
payload: counts there are event-wide, so another guest's or the admin's stuck
upload would spin the notice forever and it could never say "your photo
failed".
Authorization: verifyGalleryAccess already resolves req.event from the
caller's token, and the query is scoped `.where('event_id', req.event.id)`, so
an id from another gallery matches no row -- neither a cross-event read nor an
existence oracle, since it returns all-zero counts rather than a 403/404 that
would confirm the id exists elsewhere. Slideshow tokens are denied (a kiosk
never uploads). Ids are pattern-validated, max 50. The response is counts
only: no filenames and specifically no processing_error strings, which can
carry internal paths. Not gated on allow_user_uploads, so an admin flipping
the toggle mid-flight does not strand an in-progress guest.
The frontend now finishes on the real terminal condition, refetches as each
photo lands rather than only at the end, shows a processing pill, and reports
real failures instead of silently timing out.
Refs testplan REPORT.md B6, B7.
This commit is contained in:
@@ -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');
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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=<upload_id>[,<upload_id>…]
|
||||
*
|
||||
* 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)
|
||||
|
||||
@@ -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<GalleryViewProps> = ({ 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<ReturnType<typeof setInterval> | 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<GalleryViewProps> = ({ 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 ? (
|
||||
<div className="fixed bottom-4 left-1/2 -translate-x-1/2 z-50 flex items-center gap-2 rounded-full bg-neutral-900/90 px-4 py-2 text-sm text-white shadow-lg">
|
||||
<Loader2 className="w-4 h-4 animate-spin shrink-0" />
|
||||
<span>
|
||||
{t('upload.processing')}{' '}
|
||||
{t('upload.processingProgress', {
|
||||
complete: uploadProcessing.complete,
|
||||
total: uploadProcessing.total,
|
||||
})}
|
||||
</span>
|
||||
</div>
|
||||
) : 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<GalleryViewProps> = ({ 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<GalleryViewProps> = ({ slug, event, requiresP
|
||||
onClose={() => setShowUploadModal(false)}
|
||||
/>
|
||||
)}
|
||||
{uploadProcessingNotice}
|
||||
|
||||
{/* Download size picker (#858) — "download all", or a selection. */}
|
||||
{(showResolutionPicker || resolutionPickerIds) && (
|
||||
|
||||
@@ -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<UserPhotoUploadProps> = ({
|
||||
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<UserPhotoUploadProps> = ({
|
||||
}
|
||||
|
||||
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<UserPhotoUploadProps> = ({
|
||||
}
|
||||
},
|
||||
});
|
||||
// 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<UserPhotoUploadProps> = ({
|
||||
|
||||
if (successCount > 0) {
|
||||
toast.success(t('toast.uploadSuccess') + ` (${successCount} ${t('common.photos')})`);
|
||||
onUploadComplete(successCount);
|
||||
onUploadComplete(uploadIds);
|
||||
}
|
||||
|
||||
if (failedCount > 0) {
|
||||
|
||||
@@ -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(',') }");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<UploadProcessingStatus> {
|
||||
const response = await api.get<UploadProcessingStatus>(`/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
|
||||
|
||||
Reference in New Issue
Block a user