fix: make bookmark add idempotent instead of 409

Re-adding an already-bookmarked manga returned 409, which the UI surfaced as
a false "Could not add bookmark" toast — collections are idempotent, bookmarks
weren't. On a unique violation, fetch and return the existing bookmark, and
answer 200 (vs 201 for a fresh insert). The frontend toggle already treats a
successful POST as done, so the false toast disappears with no client change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

backend/Cargo.lock

@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
 
 [[package]]
 name = "mangalord"
-version = "0.124.20"
+version = "0.124.21"
 dependencies = [
  "anyhow",
  "argon2",

backend/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mangalord"
-version = "0.124.20"
+version = "0.124.21"
 edition = "2021"
 default-run = "mangalord"
 

backend/src/api/bookmarks.rs

@@ -72,7 +72,7 @@ async fn create(
         }
     }
 
-    let bookmark = repo::bookmark::create(
+    let (bookmark, created) = repo::bookmark::create(
         &state.db,
         user.id,
         input.manga_id,
@@ -103,7 +103,13 @@ async fn create(
         }
     });
 
-    Ok((StatusCode::CREATED, Json(bookmark)))
+    // 201 for a fresh bookmark, 200 when it already existed (idempotent add).
+    let status = if created {
+        StatusCode::CREATED
+    } else {
+        StatusCode::OK
+    };
+    Ok((status, Json(bookmark)))
 }
 
 async fn delete_one(

backend/src/repo/bookmark.rs

@@ -6,13 +6,23 @@ use uuid::Uuid;
 use crate::domain::{Bookmark, BookmarkSummary};
 use crate::error::{AppError, AppResult};
 
+/// Add a bookmark, idempotently. Returns the bookmark plus whether it was
+/// newly created (`true`) or already existed (`false`), so the handler can
+/// answer 201 vs 200. Re-adding an existing bookmark is a no-op success rather
+/// than a 409 — matching the idempotent collection semantics, so the UI doesn't
+/// show a false "Could not add bookmark" toast when the manga is already saved.
+///
+/// Uniqueness is per `(user_id, manga_id, chapter_id)` — enforced by the 0001
+/// constraint for chapter-level rows and the 0004 partial index for manga-level
+/// (NULL chapter) rows. `page` is not part of the key, so the existing row is
+/// returned unchanged (its page is not overwritten).
 pub async fn create(
     pool: &PgPool,
     user_id: Uuid,
     manga_id: Uuid,
     chapter_id: Option<Uuid>,
     page: Option<i32>,
-) -> AppResult<Bookmark> {
+) -> AppResult<(Bookmark, bool)> {
     let result = sqlx::query_as::<_, Bookmark>(
         r#"
         INSERT INTO bookmarks (user_id, manga_id, chapter_id, page)
@@ -28,10 +38,27 @@ pub async fn create(
     .await;
 
     match result {
-        Ok(b) => Ok(b),
-        Err(sqlx::Error::Database(ref db_err)) if db_err.is_unique_violation() => Err(
-            AppError::Conflict("bookmark already exists for this manga/chapter".into()),
-        ),
+        Ok(b) => Ok((b, true)),
+        Err(sqlx::Error::Database(ref db_err)) if db_err.is_unique_violation() => {
+            // A bookmark for this (user, manga, chapter) already exists — fetch
+            // and return it. `IS NOT DISTINCT FROM` matches a NULL chapter_id
+            // (manga-level bookmark) as well as a concrete one, covering both
+            // uniqueness paths with a single lookup.
+            let existing = sqlx::query_as::<_, Bookmark>(
+                r#"
+                SELECT id, user_id, manga_id, chapter_id, page, created_at
+                FROM bookmarks
+                WHERE user_id = $1 AND manga_id = $2
+                  AND chapter_id IS NOT DISTINCT FROM $3
+                "#,
+            )
+            .bind(user_id)
+            .bind(manga_id)
+            .bind(chapter_id)
+            .fetch_one(pool)
+            .await?;
+            Ok((existing, false))
+        }
         Err(e) => Err(AppError::Database(e)),
     }
 }

backend/tests/api_bookmarks.rs

@@ -52,7 +52,7 @@ async fn create_then_list_returns_only_own(pool: PgPool) {
 }
 
 #[sqlx::test(migrations = "./migrations")]
-async fn create_returns_409_on_duplicate_manga_level(pool: PgPool) {
+async fn create_is_idempotent_on_duplicate_manga_level(pool: PgPool) {
     let h = common::harness(pool);
     let (_, cookie) = common::register_user(&h.app).await;
     let manga_id = common::seed_manga_via_api(&h.app, &cookie, "Berserk").await;
@@ -64,12 +64,26 @@ async fn create_returns_409_on_duplicate_manga_level(pool: PgPool) {
             &cookie,
         )
     };
+    // First add creates (201); re-adding is an idempotent no-op (200) that
+    // returns the SAME bookmark rather than a 409 — collections behave this way
+    // too, so the UI shouldn't surface a false "Could not add bookmark" error.
     let first = h.app.clone().oneshot(make()).await.unwrap();
     assert_eq!(first.status(), StatusCode::CREATED);
-    let second = h.app.oneshot(make()).await.unwrap();
-    assert_eq!(second.status(), StatusCode::CONFLICT);
-    let body = common::body_json(second).await;
-    assert_eq!(body["error"]["code"], "conflict");
+    let first_body = common::body_json(first).await;
+
+    let second = h.app.clone().oneshot(make()).await.unwrap();
+    assert_eq!(second.status(), StatusCode::OK);
+    let second_body = common::body_json(second).await;
+    assert_eq!(second_body["id"], first_body["id"], "same bookmark returned");
+
+    // Exactly one row exists.
+    let list = h
+        .app
+        .oneshot(common::get_with_cookie("/api/v1/me/bookmarks", &cookie))
+        .await
+        .unwrap();
+    let body = common::body_json(list).await;
+    assert_eq!(body["items"].as_array().unwrap().len(), 1);
 }
 
 #[sqlx::test(migrations = "./migrations")]
@@ -225,13 +239,16 @@ async fn concurrent_manga_bookmarks_serialised_by_unique_index(pool: PgPool) {
 
     let (s1, s2) = tokio::join!(f1, f2);
     let statuses = [s1.unwrap(), s2.unwrap()];
+    // The unique index serialises the two inserts: one wins with 201, the other
+    // hits the violation and — now idempotent — returns the existing bookmark
+    // with 200 rather than a 409.
     assert!(
         statuses.contains(&StatusCode::CREATED),
         "expected one winner with 201, got {statuses:?}"
     );
     assert!(
-        statuses.contains(&StatusCode::CONFLICT),
-        "expected one loser with 409 (the partial unique index), got {statuses:?}"
+        statuses.contains(&StatusCode::OK),
+        "expected the loser to return 200 (idempotent), got {statuses:?}"
     );
 }