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<i32>`. 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<BookmarkSummary>.
- api::bookmarks::list_me returns PagedResponse<BookmarkSummary>.
- 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) <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1033,7 +1033,7 @@ checksum = "5e5032e24019045c762d3c0f28f5b6b8bbf38563a65908389bf7978758920897"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.9.0"
|
||||
version = "0.9.1"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.9.1"
|
||||
version = "0.9.2"
|
||||
edition = "2021"
|
||||
|
||||
[lib]
|
||||
|
||||
@@ -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<AppState>,
|
||||
CurrentUser(user): CurrentUser,
|
||||
Query(params): Query<ListParams>,
|
||||
) -> AppResult<Json<PagedResponse<Bookmark>>> {
|
||||
) -> AppResult<Json<PagedResponse<BookmarkSummary>>> {
|
||||
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?;
|
||||
|
||||
@@ -12,3 +12,19 @@ pub struct Bookmark {
|
||||
pub page: Option<i32>,
|
||||
pub created_at: DateTime<Utc>,
|
||||
}
|
||||
|
||||
/// `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<Uuid>,
|
||||
pub chapter_number: Option<i32>,
|
||||
pub page: Option<i32>,
|
||||
pub created_at: DateTime<Utc>,
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<Vec<Bookmark>> {
|
||||
let rows = sqlx::query_as::<_, Bookmark>(
|
||||
) -> AppResult<Vec<BookmarkSummary>> {
|
||||
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
|
||||
"#,
|
||||
)
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "mangalord-frontend",
|
||||
"version": "0.9.1",
|
||||
"version": "0.9.2",
|
||||
"private": true,
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
|
||||
@@ -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;
|
||||
};
|
||||
|
||||
@@ -20,11 +20,16 @@
|
||||
<ul class="bookmark-list" data-testid="bookmark-list">
|
||||
{#each bookmarks as b (b.id)}
|
||||
<li>
|
||||
{#if b.chapter_id}
|
||||
<a href="/manga/{b.manga_id}/chapter/{b.chapter_id}">
|
||||
Chapter bookmark
|
||||
{#if b.chapter_id && b.chapter_number != null}
|
||||
<a href="/manga/{b.manga_id}/chapter/{b.chapter_number}">
|
||||
Chapter {b.chapter_number}
|
||||
{#if b.page}— page {b.page}{/if}
|
||||
</a>
|
||||
{:else if b.chapter_id}
|
||||
<!-- Chapter bookmark whose chapter was deleted; fall
|
||||
back to the manga overview rather than emit a
|
||||
broken link to a number we don't have. -->
|
||||
<a href="/manga/{b.manga_id}">Chapter bookmark (chapter removed)</a>
|
||||
{:else}
|
||||
<a href="/manga/{b.manga_id}">Manga bookmark</a>
|
||||
{/if}
|
||||
|
||||
Reference in New Issue
Block a user