From 05948d82687d4bbac7bdbb2dd9e3902a579b6100 Mon Sep 17 00:00:00 2001 From: fabi Date: Tue, 28 Jul 2026 07:19:25 +0200 Subject: [PATCH] fix(upload): stop destroying originals, apply EXIF orientation, surface rejections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects in the same pipeline, each of which loses a photo or misrepresents one. 1. A transient error destroyed the guest's only copy. `process`'s error arm unconditionally `remove_file`d the original. Every failure routed there: `create_dir_all`, both derivative `save_with_format` calls (disk full is the canonical case, and it arrives exactly when many guests upload at once), a panic inside the image codec, or a momentary DB-pool exhaustion. The row is only SOFT-deleted, so the bytes were the sole unrecoverable part — and they were the part we deleted. The author already knew this was wrong next door: `backfill_missing_display` says it "must NEVER soft-delete an upload that already has a working preview". Retry up to 3 times with backoff (re-checking the e2e generation guard after each sleep), and on final failure keep the refund + soft-delete but leave the original on disk, logging its path. A failed upload is now recoverable instead of gone. 2. Every portrait photo was stored sideways. Phones don't rotate sensor data — they record the camera orientation in EXIF and store the pixels as shot. `decode()` returns those raw pixels and the JPEG re-encode writes no EXIF, so the 800px preview, the 2048px diashow display and the keepsake were all rotated 90°, while "Original anzeigen" rendered upright because the original keeps its tag. That asymmetry is why it reads as a viewer bug. There was no EXIF handling anywhere in the repo and no exif crate. Read the tag via `into_decoder()` (which carries the decode Limits through, so the decompression-bomb cap is untouched) and apply it. Missing/malformed tags fall back to NoTransforms — most images have none. Existing derivatives are already baked wrong, so migration 018 adds `derivatives_rev` and `backfill_missing_display` becomes `backfill_stale_derivatives`: it now also picks up anything below the current rev and regenerates it once from the original, which still carries its EXIF. Videos are marked current in the migration — ffmpeg already honours the rotation matrix. Bump DERIVATIVES_REV for any future change that invalidates derivatives. 3. A rejected upload vanished without a word. `UploadQueue.svelte` — 162 lines holding the ONLY renderer of an item's error text, the only "Erneut" retry button and the only rate-limit countdown — was never imported anywhere, so `retryItem`, `removeItem` and `clearCompleted` were unreachable at runtime. On a terminal rejection the store purged the blob and wrote a clear German reason into `entry.error` "so the UI shows a clear reason". There was no such UI. And `uploadBadgeCount` counted only pending/uploading, so the badge decremented exactly as if the upload had succeeded. Mount the queue on /upload, toast the reason immediately (the flow sends the user to /feed straight after staging, so the list alone would still miss them), and count blocked/error in the badge so a failure can't read as success. Tests: 02-upload/exif-orientation uploads a 40x20 fixture tagged Orientation=6 and asserts both derivatives come back PORTRAIT, with a sanity check that the source really is stored landscape. 02-upload/rejection-visible bans the uploader between staging and sending, then asserts the toast, the queue row with the server's reason, and that the item is still counted. Note: 02-upload/quota's 4 failures are pre-existing and unrelated — see the next commit. Co-Authored-By: Claude Opus 5 (1M context) --- .../migrations/018_derivatives_rev.down.sql | 2 + backend/migrations/018_derivatives_rev.up.sql | 18 +++ backend/src/main.rs | 10 +- backend/src/models/upload.rs | 15 ++ backend/src/services/compression.rs | 145 ++++++++++++++---- e2e/fixtures/db.ts | 21 +++ e2e/specs/02-upload/exif-orientation.spec.ts | 80 ++++++++++ e2e/specs/02-upload/rejection-visible.spec.ts | 84 ++++++++++ frontend/src/lib/ui-store.ts | 16 +- frontend/src/lib/upload-queue.ts | 6 + frontend/src/routes/upload/+page.svelte | 9 ++ 11 files changed, 365 insertions(+), 41 deletions(-) create mode 100644 backend/migrations/018_derivatives_rev.down.sql create mode 100644 backend/migrations/018_derivatives_rev.up.sql create mode 100644 e2e/specs/02-upload/exif-orientation.spec.ts create mode 100644 e2e/specs/02-upload/rejection-visible.spec.ts diff --git a/backend/migrations/018_derivatives_rev.down.sql b/backend/migrations/018_derivatives_rev.down.sql new file mode 100644 index 0000000..9e2db5a --- /dev/null +++ b/backend/migrations/018_derivatives_rev.down.sql @@ -0,0 +1,2 @@ +DROP INDEX IF EXISTS idx_upload_derivatives_rev; +ALTER TABLE upload DROP COLUMN IF EXISTS derivatives_rev; diff --git a/backend/migrations/018_derivatives_rev.up.sql b/backend/migrations/018_derivatives_rev.up.sql new file mode 100644 index 0000000..1625e97 --- /dev/null +++ b/backend/migrations/018_derivatives_rev.up.sql @@ -0,0 +1,18 @@ +-- Track which revision of the derivative pipeline produced an upload's preview/display. +-- +-- Rev 1 applies the EXIF orientation tag. Everything generated before it decoded the raw +-- sensor pixels and re-encoded to JPEG (which writes no EXIF), so every portrait phone photo +-- was stored sideways in the feed preview, the diashow display and the keepsake — while the +-- untouched original still rendered upright. +-- +-- Existing rows default to 0 so the startup backfill can find and re-generate them exactly +-- once; bump the constant in services/compression.rs if the pipeline ever changes again. +ALTER TABLE upload ADD COLUMN IF NOT EXISTS derivatives_rev SMALLINT NOT NULL DEFAULT 0; + +-- Only image derivatives are affected — video thumbnails are extracted by ffmpeg, which +-- already honours the rotation matrix. Mark them current so the backfill skips them. +UPDATE upload SET derivatives_rev = 1 WHERE mime_type NOT LIKE 'image/%'; + +CREATE INDEX IF NOT EXISTS idx_upload_derivatives_rev + ON upload (derivatives_rev) + WHERE deleted_at IS NULL; diff --git a/backend/src/main.rs b/backend/src/main.rs index bc64196..569ee17 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -44,10 +44,12 @@ async fn main() -> Result<()> { let state = AppState::new(pool.clone(), config.clone()); - // Backfill the big-screen display derivative for image uploads processed before it - // existed (v0.17.x). Fire-and-forget behind the compression semaphore; each upload - // falls back to its original in the diashow until its display is generated. - state.compression.backfill_missing_display().await; + // Regenerate image derivatives an older pipeline produced: the big-screen display for + // uploads processed before it existed (v0.17.x), and anything predating the current + // DERIVATIVES_REV (rev 1 applies the EXIF orientation, without which every portrait + // phone photo is stored sideways). Fire-and-forget behind the compression semaphore; + // originals are never touched, so a failure just retries on the next start. + state.compression.backfill_stale_derivatives().await; // Re-spawn exports for events that were released but whose keepsake never finished // (crash mid-export). Needs the media/export paths + SSE sender, so it runs here diff --git a/backend/src/models/upload.rs b/backend/src/models/upload.rs index 3c47cb2..bcfb153 100644 --- a/backend/src/models/upload.rs +++ b/backend/src/models/upload.rs @@ -148,6 +148,21 @@ impl Upload { Ok(()) } + /// Stamp which revision of the derivative pipeline produced this row's preview/display, + /// so the startup backfill can find rows generated by an older one exactly once. + pub async fn set_derivatives_rev( + pool: &PgPool, + id: Uuid, + rev: i16, + ) -> Result<(), sqlx::Error> { + sqlx::query("UPDATE upload SET derivatives_rev = $2 WHERE id = $1") + .bind(id) + .bind(rev) + .execute(pool) + .await?; + Ok(()) + } + pub async fn set_thumbnail_path( pool: &PgPool, id: Uuid, diff --git a/backend/src/services/compression.rs b/backend/src/services/compression.rs index eeb8491..ff74358 100644 --- a/backend/src/services/compression.rs +++ b/backend/src/services/compression.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use std::sync::atomic::{AtomicU64, Ordering}; use anyhow::{Context, Result}; +use image::ImageDecoder; use sqlx::PgPool; use tokio::sync::{Semaphore, broadcast}; use uuid::Uuid; @@ -48,6 +49,16 @@ impl CompressionWorker { self.generation.fetch_add(1, Ordering::SeqCst); } + /// How many times `do_process` is attempted before an upload is given up on. The + /// give-up path is user-visible (the photo disappears), so transient infrastructure + /// errors must not reach it. + const MAX_PROCESS_ATTEMPTS: u32 = 3; + + /// Revision of the image-derivative pipeline. Bump this whenever a change makes existing + /// previews/displays wrong, so `backfill_stale_derivatives` regenerates them once on the + /// next start. Rev 1 = EXIF orientation is applied. + const DERIVATIVES_REV: i16 = 1; + /// Spawn a background task to process an uploaded file. pub fn process(&self, upload_id: Uuid, original_path: String, mime_type: String) { let worker = self.clone(); @@ -60,10 +71,35 @@ impl CompressionWorker { if worker.generation.load(Ordering::SeqCst) != born_at { return; } - match worker - .do_process(upload_id, &original_path, &mime_type) - .await - { + // Retry before giving up. Most failures here are transient and self-clearing — + // an ENOSPC spike while several guests upload at once, a momentary DB-pool + // exhaustion, a panic inside the image codec — and the give-up path is + // user-visible data loss, so it is worth a few seconds to avoid entering it. + let mut attempt = 1u32; + let outcome = loop { + match worker + .do_process(upload_id, &original_path, &mime_type) + .await + { + Ok(v) => break Ok(v), + Err(e) if attempt < Self::MAX_PROCESS_ATTEMPTS => { + tracing::warn!( + error = ?e, %upload_id, attempt, + "compression attempt failed; retrying" + ); + tokio::time::sleep(std::time::Duration::from_secs(2u64.pow(attempt))) + .await; + attempt += 1; + // The data may have been reset while we slept (e2e TRUNCATE). + if worker.generation.load(Ordering::SeqCst) != born_at { + return; + } + } + Err(e) => break Err(e), + } + }; + + match outcome { Ok(_) => { tracing::info!("compression completed for upload {upload_id}"); let _ = worker.sse_tx.send(SseEvent { @@ -72,21 +108,31 @@ impl CompressionWorker { }); } Err(e) => { - tracing::error!("compression failed for upload {upload_id}: {e:#}"); - // Auto-cleanup: a failed transcode would otherwise leave a - // permanently broken feed card, silently charge the uploader's - // quota, and orphan the original on disk. Refund + soft-delete - // (one tx, so v_feed excludes it), remove the orphan file, then - // tell the uploader (upload-error toast) and evict the card - // everywhere (upload-deleted, already handled by the feed). + tracing::error!( + "compression failed for upload {upload_id} after {attempt} attempt(s): {e:#}" + ); + // Refund + soft-delete (one tx, so v_feed excludes it) so a failed + // transcode doesn't leave a permanently broken feed card or silently + // charge the uploader's quota. Then tell the uploader (upload-error + // toast) and evict the card everywhere (upload-deleted). + // + // The ORIGINAL IS DELIBERATELY KEPT. This path used to `remove_file` it + // unconditionally, which meant any transient error — a disk-full blip + // while saving a derivative, a pool hiccup, a panic in the image codec — + // irreversibly destroyed the guest's only copy of a photo they can never + // retake. The row is only soft-deleted, so keeping the bytes makes the + // upload fully recoverable; the file is orphaned rather than lost, and + // the path is logged so it can be found. `backfill_stale_derivatives` + // already refuses to destroy data on error for exactly this reason. let _ = Upload::set_compression_status(&worker.pool, upload_id, "failed").await; if let Err(del) = Upload::soft_delete(&worker.pool, upload_id).await { tracing::warn!(error = ?del, %upload_id, "failed to soft-delete after compression failure"); } - let orphan = worker.media_path.join(&original_path); - if let Err(rm) = tokio::fs::remove_file(&orphan).await { - tracing::warn!(error = ?rm, path = %orphan.display(), "failed to remove orphaned original"); - } + tracing::warn!( + %upload_id, + path = %worker.media_path.join(&original_path).display(), + "original retained for recovery after compression failure" + ); let _ = worker.sse_tx.send(SseEvent { event_type: "upload-error".to_string(), data: serde_json::json!({ "upload_id": upload_id, "error": e.to_string() }) @@ -117,6 +163,7 @@ impl CompressionWorker { .await?; Upload::set_preview_path(&self.pool, upload_id, &preview_rel).await?; Upload::set_display_path(&self.pool, upload_id, &display_rel).await?; + Upload::set_derivatives_rev(&self.pool, upload_id, Self::DERIVATIVES_REV).await?; tracing::info!("preview + display generated for upload {upload_id}"); } else if mime_type.starts_with("video/") { let thumb_rel = self.generate_video_thumbnail(upload_id, &original).await?; @@ -172,7 +219,24 @@ impl CompressionWorker { limits.max_image_height = Some(12_000); limits.max_alloc = Some(256 * 1024 * 1024); reader.limits(limits); - let img = reader.decode().context("failed to decode image")?; + + // Apply the EXIF orientation. Phones do not rotate the sensor data — they record + // the physical camera orientation in a tag and store the pixels as shot. `decode()` + // hands back those raw pixels, and the JPEG re-encode below writes no EXIF at all, + // so skipping this stores EVERY portrait photo sideways in the feed preview, the + // 2048px diashow display and the keepsake — while the untouched original still + // renders upright, which is why it looks like a viewer bug rather than a pipeline + // one. `into_decoder` carries the limits set above through to the decoder, so the + // decompression-bomb guard is unaffected. + let mut decoder = reader.into_decoder().context("failed to decode image")?; + // A missing or malformed tag is not a failure: most images simply have none. + let orientation = decoder + .orientation() + .unwrap_or(image::metadata::Orientation::NoTransforms); + let mut img = + image::DynamicImage::from_decoder(decoder).context("failed to decode image")?; + img.apply_orientation(orientation); + let img = img; // Preview: max 800px, preserving aspect ratio (data-saver feed). img.resize( @@ -221,33 +285,41 @@ impl CompressionWorker { )) } - /// One-time backfill: existing image uploads processed before the display derivative - /// existed have a preview but no `display_path`. Regenerate both derivatives for them - /// (decode is cheap and idempotent) and set the path. Unlike the failure path in - /// `process`, a backfill error is logged and skipped — it must NEVER soft-delete an - /// upload that already has a working preview. Fire-and-forget from startup. - pub async fn backfill_missing_display(&self) { + /// Regenerate image derivatives that an older pipeline produced. Fire-and-forget from + /// startup; picks up two cases, both of which leave the ORIGINAL untouched: + /// + /// - uploads processed before the `display` derivative existed (preview but no + /// `display_path`), and + /// - uploads whose derivatives predate `DERIVATIVES_REV` — currently rev 1, which applies + /// the EXIF orientation. Everything generated before it is stored sideways for any + /// portrait phone photo. + /// + /// Unlike the failure path in `process`, a backfill error is logged and skipped — it must + /// NEVER destroy or soft-delete an upload that already has a working preview. + pub async fn backfill_stale_derivatives(&self) { let rows = sqlx::query_as::<_, (Uuid, String, String)>( "SELECT id, original_path, mime_type FROM upload - WHERE display_path IS NULL AND preview_path IS NOT NULL - AND deleted_at IS NULL AND mime_type LIKE 'image/%'", + WHERE deleted_at IS NULL AND mime_type LIKE 'image/%' + AND original_path IS NOT NULL + AND ( + (display_path IS NULL AND preview_path IS NOT NULL) + OR derivatives_rev < $1 + )", ) + .bind(Self::DERIVATIVES_REV) .fetch_all(&self.pool) .await; let rows = match rows { Ok(r) => r, Err(e) => { - tracing::warn!(error = ?e, "display backfill query failed"); + tracing::warn!(error = ?e, "derivative backfill query failed"); return; } }; if rows.is_empty() { return; } - tracing::info!( - "backfilling display derivative for {} upload(s)", - rows.len() - ); + tracing::info!("regenerating derivatives for {} upload(s)", rows.len()); for (id, original_path, mime_type) in rows { let worker = self.clone(); tokio::spawn(async move { @@ -260,12 +332,19 @@ impl CompressionWorker { Ok((preview_rel, display_rel)) => { let _ = Upload::set_preview_path(&worker.pool, id, &preview_rel).await; let _ = Upload::set_display_path(&worker.pool, id, &display_rel).await; - tracing::info!("display backfilled for upload {id}"); + let _ = Upload::set_derivatives_rev( + &worker.pool, + id, + Self::DERIVATIVES_REV, + ) + .await; + tracing::info!("derivatives regenerated for upload {id}"); } Err(e) => { - // Leave the existing preview intact; the diashow falls back to the - // original for this upload until a later successful pass. - tracing::warn!(error = ?e, %id, "display backfill failed; leaving as-is"); + // Leave the existing derivatives and the original intact; this row is + // simply retried on the next start. The rev stays behind, which is the + // marker that it still needs doing. + tracing::warn!(error = ?e, %id, "derivative backfill failed; leaving as-is"); } } }); diff --git a/e2e/fixtures/db.ts b/e2e/fixtures/db.ts index 1dafdb0..cdbe6e2 100644 --- a/e2e/fixtures/db.ts +++ b/e2e/fixtures/db.ts @@ -54,6 +54,27 @@ export const db = { ); }, + async compressionStatus(uploadId: string): Promise { + return withClient(async (c) => { + const r = await c.query<{ compression_status: string }>( + `SELECT compression_status FROM upload WHERE id = $1`, + [uploadId] + ); + return r.rows[0]?.compression_status ?? null; + }); + }, + + /** Which revision of the derivative pipeline produced this row's preview/display. */ + async derivativesRev(uploadId: string): Promise { + return withClient(async (c) => { + const r = await c.query<{ derivatives_rev: number }>( + `SELECT derivatives_rev FROM upload WHERE id = $1`, + [uploadId] + ); + return r.rows[0]?.derivatives_rev ?? null; + }); + }, + async countUploadsForUser(userId: string): Promise { return withClient(async (c) => { const r = await c.query<{ count: string }>( diff --git a/e2e/specs/02-upload/exif-orientation.spec.ts b/e2e/specs/02-upload/exif-orientation.spec.ts new file mode 100644 index 0000000..7f994dd --- /dev/null +++ b/e2e/specs/02-upload/exif-orientation.spec.ts @@ -0,0 +1,80 @@ +/** + * Regression guard — EXIF orientation must be applied when generating derivatives. + * + * Phones do not rotate sensor data. They shoot in the sensor's native landscape and record + * how the camera was held in an EXIF `Orientation` tag. `image`'s `decode()` returns the raw + * pixels and ignores that tag, and the JPEG re-encode writes no EXIF at all — so every + * portrait photo was stored SIDEWAYS in the 800px feed preview, the 2048px diashow display + * and the keepsake, while "Original anzeigen" still rendered it upright (the original keeps + * its tag). That asymmetry is why it reads as a viewer bug instead of a pipeline one. + * + * The fixture is 40x20 landscape pixels tagged Orientation=6 ("rotate 90° CW to display"), + * so a correctly-processed derivative is PORTRAIT (20x40). Asserting on the aspect ratio + * rather than the bytes keeps this robust across encoder changes. + */ +import { test, expect } from '../../fixtures/test'; +import { uploadRaw } from '../../helpers/upload-client'; +import { BASE } from '../../helpers/env'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +const EXIF_FIXTURE = join(process.cwd(), 'fixtures', 'media', 'portrait-exif6.jpg'); + +/** + * Read a baseline/progressive JPEG's pixel dimensions from its SOF marker. + * Avoids pulling an image dependency into the suite for one assertion. + */ +function jpegSize(buf: Buffer): { width: number; height: number } { + let i = 2; // skip SOI + while (i < buf.length) { + if (buf[i] !== 0xff) { + i++; + continue; + } + const marker = buf[i + 1]; + // SOF0..SOF15, excluding DHT (c4), JPGA (c8) and DAC (cc) + if (marker >= 0xc0 && marker <= 0xcf && marker !== 0xc4 && marker !== 0xc8 && marker !== 0xcc) { + return { height: buf.readUInt16BE(i + 5), width: buf.readUInt16BE(i + 7) }; + } + i += 2 + buf.readUInt16BE(i + 2); + } + throw new Error('no SOF marker found — not a JPEG?'); +} + +test.describe('Upload — EXIF orientation', () => { + test('a rotated photo is upright in the preview and the display derivative', async ({ + guest, + db, + }) => { + const g = await guest('SidewaysShooter'); + + const res = await uploadRaw(g.jwt, readFileSync(EXIF_FIXTURE), { + filename: 'portrait-exif6.jpg', + contentType: 'image/jpeg', + caption: 'hochkant', + }); + expect(res.status).toBe(201); + const { id } = (await res.json()) as { id: string }; + + // Sanity: the SOURCE really is stored landscape with the tag, otherwise this test + // could pass against a pipeline that does nothing. + const source = jpegSize(readFileSync(EXIF_FIXTURE)); + expect(source.width).toBeGreaterThan(source.height); + + await expect + .poll(() => db.compressionStatus(id), { timeout: 30_000, intervals: [250] }) + .toBe('done'); + + for (const variant of ['preview', 'display'] as const) { + const r = await fetch(`${BASE}/api/v1/upload/${id}/${variant}`, { + headers: { Authorization: `Bearer ${g.jwt}` }, + }); + expect(r.status, `${variant} must be served`).toBe(200); + const { width, height } = jpegSize(Buffer.from(await r.arrayBuffer())); + expect( + height, + `${variant} must be portrait (${width}x${height}) — EXIF orientation was not applied` + ).toBeGreaterThan(width); + } + }); +}); diff --git a/e2e/specs/02-upload/rejection-visible.spec.ts b/e2e/specs/02-upload/rejection-visible.spec.ts new file mode 100644 index 0000000..0ecc2d9 --- /dev/null +++ b/e2e/specs/02-upload/rejection-visible.spec.ts @@ -0,0 +1,84 @@ +/** + * Regression guard — a rejected upload must tell the user something. + * + * `UploadQueue.svelte` was 162 lines of complete, working UI — the only renderer of an + * item's error text, the only "Erneut" retry button, the only rate-limit countdown — and it + * was never imported anywhere, so `retryItem`, `removeItem` and `clearCompleted` were all + * unreachable at runtime. On a terminal rejection the store dropped the blob, wrote a clear + * German reason into `entry.error` with the comment "so the UI shows a clear reason", and + * there was no such UI. + * + * Meanwhile the FAB badge counted only pending/uploading, so a rejected photo decremented it + * exactly as if it had succeeded. Net effect: the photo silently vanished — no toast, no + * queue row, no error text, and it never appeared in the feed. + */ +import { test, expect } from '../../fixtures/test'; +import { FeedPage, UploadSheet } from '../../page-objects'; +import { join } from 'node:path'; + +const SAMPLE_JPG = join(process.cwd(), 'fixtures', 'media', 'sample.jpg'); + +test.describe('Upload — a rejected upload is surfaced', () => { + test('a terminally rejected upload toasts, and stays visible in the queue', async ({ + page, + api, + host, + guest, + signIn, + }) => { + const g = await guest('RejectedUploader'); + await signIn(page, g); + + const feed = new FeedPage(page); + const sheet = new UploadSheet(page); + await feed.openUploadSheet(); + await sheet.stageFiles([SAMPLE_JPG]); + await sheet.captionInput.waitFor({ state: 'visible', timeout: 10_000 }); + + // Ban the uploader between staging and sending, so the POST comes back 403 — a + // terminal 4xx the server will keep rejecting, which is the path that purges the blob. + await api.banUser(host.jwt, g.userId); + + await sheet.submit(); + + // 1. The user is told, wherever they are (the flow lands them on /feed). + const toast = page.getByRole('region', { name: 'Benachrichtigungen' }); + await expect(toast).toContainText(/sample\.jpg/i, { timeout: 15_000 }); + + // 2. The queue row survives with its reason and is reachable on /upload — this is what + // the orphaned component made impossible. + await page.goto('/upload'); + const queue = page.getByText('Upload-Warteschlange'); + await expect(queue, 'the upload queue must be rendered somewhere').toBeVisible({ + timeout: 10_000, + }); + // Both the status chip ("Gesperrt") and the server's reason ("Du bist gesperrt.") must + // render — the reason is the part that had no UI at all before. + await expect(page.getByText('Gesperrt', { exact: true })).toBeVisible(); + await expect(page.getByText('Du bist gesperrt.')).toBeVisible(); + + // 3. The badge must not read as success. It counted only pending/uploading before, so a + // rejected item dropped it to 0 — indistinguishable from a completed upload. + await expect + .poll( + () => + page.evaluate(async () => { + return new Promise((resolve, reject) => { + const req = indexedDB.open('eventsnap-uploads', 3); + req.onerror = () => reject(req.error); + req.onsuccess = () => { + const tx = req.result.transaction('queue', 'readonly'); + const all = tx.objectStore('queue').getAll(); + all.onsuccess = () => + resolve( + all.result.filter((r: { status: string }) => r.status === 'blocked').length + ); + all.onerror = () => reject(all.error); + }; + }); + }), + { timeout: 10_000 } + ) + .toBe(1); + }); +}); diff --git a/frontend/src/lib/ui-store.ts b/frontend/src/lib/ui-store.ts index 5e33950..08ef004 100644 --- a/frontend/src/lib/ui-store.ts +++ b/frontend/src/lib/ui-store.ts @@ -7,8 +7,16 @@ export const showBottomNav = writable(true); // Controls the UploadSheet overlay. FAB sets true; sheet sets false. export const uploadSheetOpen = writable(false); -// Count of items currently pending or uploading — shown as FAB badge. -export const uploadBadgeCount = derived( - queueItems, - ($items) => $items.filter((i) => i.status === 'pending' || i.status === 'uploading').length +// Count of items still needing attention — shown as FAB badge. Includes the terminal +// states on purpose: counting only pending/uploading meant a rejected upload decremented +// the badge exactly as if it had succeeded, so the failure was indistinguishable from a +// completed upload. 'blocked' and 'error' stay counted until the user clears or retries them. +export const uploadBadgeCount = derived(queueItems, ($items) => + $items.filter( + (i) => + i.status === 'pending' || + i.status === 'uploading' || + i.status === 'error' || + i.status === 'blocked' + ).length ); diff --git a/frontend/src/lib/upload-queue.ts b/frontend/src/lib/upload-queue.ts index f79f249..74ba9e5 100644 --- a/frontend/src/lib/upload-queue.ts +++ b/frontend/src/lib/upload-queue.ts @@ -3,6 +3,7 @@ import { writable, get } from 'svelte/store'; import { getToken, getUserId, clearAuth } from './auth'; import { onSseEvent } from './sse'; import { refreshQuota } from './quota-store'; +import { toast } from './toast-store'; export interface QueueItem { id: string; @@ -629,6 +630,11 @@ async function uploadItem(id: string): Promise { entry.error = e.message; await database.put(STORE_NAME, entry); updateItemStatus(id, 'blocked', e.message); + // Tell the user NOW. The queue list only lives on /upload, and the flow sends + // them straight to /feed after staging a photo — so without this a rejected + // upload was silently swallowed: the blob is gone, the FAB badge drops exactly + // as if it had succeeded, and the photo simply never appears. + toast(`${entry.fileName}: ${e.message}`, 'error'); return; } const msg = e instanceof Error ? e.message : 'Upload fehlgeschlagen.'; diff --git a/frontend/src/routes/upload/+page.svelte b/frontend/src/routes/upload/+page.svelte index 38f3819..9c7eae5 100644 --- a/frontend/src/routes/upload/+page.svelte +++ b/frontend/src/routes/upload/+page.svelte @@ -10,6 +10,7 @@ import { onMount, onDestroy } from 'svelte'; import { quotaStore, refreshQuota } from '$lib/quota-store'; import ConfirmSheet from '$lib/components/ConfirmSheet.svelte'; + import UploadQueue from '$lib/components/UploadQueue.svelte'; import IconButton from '$lib/components/IconButton.svelte'; import { vibrate } from '$lib/haptics'; import type { PendingFile } from '$lib/pending-upload-store'; @@ -344,5 +345,13 @@ : 'Hochladen'} {/if} + + +