From 563524d51e227ad147692fca5850769f49e95f2b Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 16 May 2026 23:20:45 +0200 Subject: [PATCH] bugfix: bookmark chapter links use chapter number, not UUID The reader route is keyed on chapter number (URL `/manga/{id}/chapter/{n}`, loaded via `Number(params.n)`), but the bookmarks list was building hrefs from `chapter_id` (a UUID). Following any chapter bookmark produced a NaN load on the reader page. Fix at the API layer so every consumer of /me/bookmarks gets the information without a follow-up round-trip per bookmark. - domain::BookmarkSummary: new type, `Bookmark` plus `chapter_number: Option`. Populated by a LEFT JOIN on chapters so manga-level bookmarks come back with `chapter_number = null` and chapter-level ones get the value. `Bookmark` itself stays minimal for POST / DELETE responses. - repo::bookmark::list_for_user returns Vec. - api::bookmarks::list_me returns PagedResponse. - Frontend `Bookmark` type carries an optional `chapter_number`. - /bookmarks page builds `/manga/{manga_id}/chapter/{chapter_number}` for chapter bookmarks, falling back to the manga overview if the chapter has been deleted out from under the bookmark (chapter_id is ON DELETE SET NULL, so this is a real edge case). New test asserts both branches of the JOIN: a chapter-level bookmark comes back with the right chapter_number and page, a manga-level one has a null chapter_number. Lockstep version bump to 0.9.2. Co-Authored-By: Claude Opus 4.7 (1M context) --- backend/Cargo.lock | 2 +- backend/Cargo.toml | 2 +- backend/src/api/bookmarks.rs | 4 +- backend/src/domain/bookmark.rs | 16 +++++ backend/src/domain/mod.rs | 2 +- backend/src/repo/bookmark.rs | 27 ++++++-- backend/tests/api_bookmarks.rs | 75 ++++++++++++++++++++++ frontend/package.json | 2 +- frontend/src/lib/api/bookmarks.ts | 6 ++ frontend/src/routes/bookmarks/+page.svelte | 11 +++- 10 files changed, 131 insertions(+), 16 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 7fa981f..4039604 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1033,7 +1033,7 @@ checksum = "5e5032e24019045c762d3c0f28f5b6b8bbf38563a65908389bf7978758920897" [[package]] name = "mangalord" -version = "0.9.0" +version = "0.9.1" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 514fdcf..fd8b86e 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.9.1" +version = "0.9.2" edition = "2021" [lib] diff --git a/backend/src/api/bookmarks.rs b/backend/src/api/bookmarks.rs index 04220d4..2ea991d 100644 --- a/backend/src/api/bookmarks.rs +++ b/backend/src/api/bookmarks.rs @@ -12,7 +12,7 @@ use uuid::Uuid; use crate::api::pagination::PagedResponse; use crate::app::AppState; use crate::auth::extractor::CurrentUser; -use crate::domain::Bookmark; +use crate::domain::{Bookmark, BookmarkSummary}; use crate::error::{AppError, AppResult}; use crate::repo; @@ -95,7 +95,7 @@ async fn list_me( State(state): State, CurrentUser(user): CurrentUser, Query(params): Query, -) -> AppResult>> { +) -> AppResult>> { let limit = params.limit.clamp(1, 200); let offset = params.offset.max(0); let items = repo::bookmark::list_for_user(&state.db, user.id, limit, offset).await?; diff --git a/backend/src/domain/bookmark.rs b/backend/src/domain/bookmark.rs index 62b8dd8..57ac6ed 100644 --- a/backend/src/domain/bookmark.rs +++ b/backend/src/domain/bookmark.rs @@ -12,3 +12,19 @@ pub struct Bookmark { pub page: Option, pub created_at: DateTime, } + +/// `Bookmark` enriched with the chapter's reader-facing number, so the +/// frontend can build `/manga/{id}/chapter/{number}` links without an +/// extra round-trip per bookmark. Populated by a LEFT JOIN; `null` for +/// manga-level bookmarks or for chapter bookmarks whose chapter has +/// since been deleted (`chapter_id` is `ON DELETE SET NULL`). +#[derive(Debug, Clone, Serialize, FromRow)] +pub struct BookmarkSummary { + pub id: Uuid, + pub user_id: Uuid, + pub manga_id: Uuid, + pub chapter_id: Option, + pub chapter_number: Option, + pub page: Option, + pub created_at: DateTime, +} diff --git a/backend/src/domain/mod.rs b/backend/src/domain/mod.rs index 771380a..1b501f3 100644 --- a/backend/src/domain/mod.rs +++ b/backend/src/domain/mod.rs @@ -7,7 +7,7 @@ pub mod session; pub mod user; pub use api_token::ApiToken; -pub use bookmark::Bookmark; +pub use bookmark::{Bookmark, BookmarkSummary}; pub use chapter::Chapter; pub use manga::Manga; pub use page::Page; diff --git a/backend/src/repo/bookmark.rs b/backend/src/repo/bookmark.rs index 4efd4f6..ab79ea9 100644 --- a/backend/src/repo/bookmark.rs +++ b/backend/src/repo/bookmark.rs @@ -3,7 +3,7 @@ use sqlx::PgPool; use uuid::Uuid; -use crate::domain::Bookmark; +use crate::domain::{Bookmark, BookmarkSummary}; use crate::error::{AppError, AppResult}; pub async fn create( @@ -36,18 +36,31 @@ pub async fn create( } } +/// Returns the user's bookmarks enriched with each chapter's number +/// (LEFT JOINed; `chapter_number` is `null` for manga-level bookmarks +/// or for chapter bookmarks whose chapter has since been deleted — +/// `bookmarks.chapter_id` is `ON DELETE SET NULL`). The frontend uses +/// the number to build reader URLs, which are keyed on number, not id. pub async fn list_for_user( pool: &PgPool, user_id: Uuid, limit: i64, offset: i64, -) -> AppResult> { - let rows = sqlx::query_as::<_, Bookmark>( +) -> AppResult> { + let rows = sqlx::query_as::<_, BookmarkSummary>( r#" - SELECT id, user_id, manga_id, chapter_id, page, created_at - FROM bookmarks - WHERE user_id = $1 - ORDER BY created_at DESC + SELECT + b.id, + b.user_id, + b.manga_id, + b.chapter_id, + c.number AS chapter_number, + b.page, + b.created_at + FROM bookmarks b + LEFT JOIN chapters c ON c.id = b.chapter_id + WHERE b.user_id = $1 + ORDER BY b.created_at DESC LIMIT $2 OFFSET $3 "#, ) diff --git a/backend/tests/api_bookmarks.rs b/backend/tests/api_bookmarks.rs index bc1fb3f..f11b25c 100644 --- a/backend/tests/api_bookmarks.rs +++ b/backend/tests/api_bookmarks.rs @@ -221,6 +221,81 @@ async fn list_me_requires_authentication(pool: PgPool) { assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); } +#[sqlx::test(migrations = "./migrations")] +async fn list_me_enriches_chapter_bookmarks_with_chapter_number(pool: PgPool) { + let h = common::harness(pool.clone()); + let (_, cookie) = common::register_user(&h.app).await; + let manga_id = common::seed_manga_via_api(&h.app, &cookie, "Berserk").await; + // Seed a chapter directly so we know its number without uploading pages. + mangalord::repo::chapter::create(&pool, manga_id, 7, Some("The Brand")) + .await + .unwrap(); + // Look up its id so we can bookmark it. + let chapter_id: Uuid = sqlx::query_scalar( + "SELECT id FROM chapters WHERE manga_id = $1 AND number = $2", + ) + .bind(manga_id) + .bind(7_i32) + .fetch_one(&pool) + .await + .unwrap(); + + // Bookmark the chapter at page 4. + let resp = h + .app + .clone() + .oneshot(common::post_json_with_cookie( + "/api/v1/bookmarks", + json!({ + "manga_id": manga_id.to_string(), + "chapter_id": chapter_id.to_string(), + "page": 4 + }), + &cookie, + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::CREATED); + + // Also bookmark the manga at the manga level — chapter_number must + // come back null for it, populated for the chapter one. + let _ = h + .app + .clone() + .oneshot(common::post_json_with_cookie( + "/api/v1/bookmarks", + json!({ "manga_id": manga_id.to_string() }), + &cookie, + )) + .await + .unwrap(); + + let resp = h + .app + .oneshot(common::get_with_cookie("/api/v1/me/bookmarks", &cookie)) + .await + .unwrap(); + let body = common::body_json(resp).await; + let items = body["items"].as_array().unwrap(); + assert_eq!(items.len(), 2); + + let chapter_bookmark = items + .iter() + .find(|b| !b["chapter_id"].is_null()) + .expect("chapter-level bookmark present"); + assert_eq!(chapter_bookmark["chapter_number"], 7); + assert_eq!(chapter_bookmark["page"], 4); + + let manga_bookmark = items + .iter() + .find(|b| b["chapter_id"].is_null()) + .expect("manga-level bookmark present"); + assert!( + manga_bookmark["chapter_number"].is_null(), + "manga-level bookmark must have a null chapter_number" + ); +} + #[sqlx::test(migrations = "./migrations")] async fn list_me_returns_paged_envelope(pool: PgPool) { let h = common::harness(pool); diff --git a/frontend/package.json b/frontend/package.json index a965d55..bf4ef25 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.9.1", + "version": "0.9.2", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/lib/api/bookmarks.ts b/frontend/src/lib/api/bookmarks.ts index 247d311..dd869f5 100644 --- a/frontend/src/lib/api/bookmarks.ts +++ b/frontend/src/lib/api/bookmarks.ts @@ -5,6 +5,12 @@ export type Bookmark = { user_id: string; manga_id: string; chapter_id: string | null; + /** + * Reader-facing chapter number, populated by the backend's LEFT + * JOIN when listing. `null` for manga-level bookmarks and for + * chapter bookmarks whose chapter has been deleted. + */ + chapter_number?: number | null; page: number | null; created_at: string; }; diff --git a/frontend/src/routes/bookmarks/+page.svelte b/frontend/src/routes/bookmarks/+page.svelte index fdb419b..ea3a410 100644 --- a/frontend/src/routes/bookmarks/+page.svelte +++ b/frontend/src/routes/bookmarks/+page.svelte @@ -20,11 +20,16 @@