From 9d2986cdae09d6c5225af28be322040ed910a9de Mon Sep 17 00:00:00 2001 From: roberte777 Date: Tue, 20 Oct 2026 14:21:51 -0500 Subject: [PATCH 1/3] feat: add the /api/reviews routes, review server functions and MCP book_reviews tool --- docs/architecture.md | 6 +- frontend/src/rpc/mod.rs | 2 + frontend/src/rpc/reviews.rs | 51 ++++ mcp/src/server/tests/checkin.rs | 4 +- mcp/src/server/tests/read.rs | 1 + mcp/src/tools/read.rs | 44 ++- mcp/src/tools/read/tests.rs | 70 ++++- mcp/src/tools/read/views.rs | 80 ++++- openapi/openapi.json | 444 +++++++++++++++++++++++++++- server/src/backend.rs | 1 + server/src/backend/openapi.rs | 3 +- server/src/backend/ratings.rs | 27 +- server/src/backend/reviews.rs | 103 +++++++ server/src/backend/reviews/tests.rs | 259 ++++++++++++++++ server/src/backend/routes.rs | 21 +- 15 files changed, 1073 insertions(+), 43 deletions(-) create mode 100644 frontend/src/rpc/reviews.rs create mode 100644 server/src/backend/reviews.rs create mode 100644 server/src/backend/reviews/tests.rs diff --git a/docs/architecture.md b/docs/architecture.md index e1e53f964..d1a370299 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -154,9 +154,13 @@ No single module owns these, so a change to any one surface has to keep them: taxonomy orphan sweeps, smart-shelf rules — gates on the same override-aware membership (the `effective_*_sql!` macros). A looser rule on one makes them disagree: the palette offers an author the index rejects. -- **Attribution.** Every user-facing name (journal author, rating, shelf owner) +- **Attribution.** Every user-facing name (journal author, reviewer, shelf owner) reads `COALESCE(display_name, username)`; `username` stays the login and admin identity. +- **Private notes.** A review's notes reach only their author: another reader + gets an `AttributedReview`, which has no notes field, and nothing they see — + the review's timestamps, the "Recently Interacted" sort — moves on an edit + that touched only the notes. - **Derived-byte caches.** Thumbnails, the KEPUB cache and the export-EPUB cache all invalidate on `books.last_modified`, so every metadata or cover write must bump it (`touch_book_last_modified`) or all three serve stale bytes. diff --git a/frontend/src/rpc/mod.rs b/frontend/src/rpc/mod.rs index 6cca8d2a8..8fdb53c9d 100644 --- a/frontend/src/rpc/mod.rs +++ b/frontend/src/rpc/mod.rs @@ -31,6 +31,7 @@ mod physical; mod progress; mod ratings; mod read_status; +mod reviews; mod scan; mod series; mod settings; @@ -59,6 +60,7 @@ pub use physical::*; pub use progress::*; pub use ratings::*; pub use read_status::*; +pub use reviews::*; pub use scan::*; pub use series::*; pub use settings::*; diff --git a/frontend/src/rpc/reviews.rs b/frontend/src/rpc/reviews.rs new file mode 100644 index 000000000..66744fc52 --- /dev/null +++ b/frontend/src/rpc/reviews.rs @@ -0,0 +1,51 @@ +//! Book reviews: read one book's reviews, patch your own, delete it. Mobile +//! uses the analogous REST routes in `server::backend::reviews`. + +use dioxus::fullstack::post; +use dioxus::prelude::*; +#[cfg(feature = "server")] +use omnibus_db as db; +use omnibus_shared::{BookReviews, Review, ReviewPatch}; + +#[cfg(feature = "server")] +use super::{internal_rpc_error, AuthUser, PoolExt}; + +/// Your review of a book, the other readers' reviews and the summary. +#[post("/api/rpc/reviews/get", pool: PoolExt, user: AuthUser)] +pub async fn rpc_get_book_reviews(uuid: String) -> Result { + match db::reviews::get_book_reviews(&pool.0, user.id, &uuid).await { + Ok(reviews) => Ok(reviews), + Err(db::reviews::ReviewError::BookNotFound) => { + Err(ServerFnError::new("book not found").into()) + } + Err(db::reviews::ReviewError::Sqlx(e)) => { + Err(internal_rpc_error("get book reviews", e).into()) + } + } +} + +/// Patch your review of a book field by field. `Ok(None)` when the patch left +/// it empty, which deletes it. +#[post("/api/rpc/reviews/update", pool: PoolExt, user: AuthUser)] +pub async fn rpc_update_review(uuid: String, patch: ReviewPatch) -> Result> { + if let Err(msg) = patch.validate() { + return Err(ServerFnError::new(msg).into()); + } + match db::reviews::update_review(&pool.0, user.id, &uuid, &patch).await { + Ok(review) => Ok(review), + Err(db::reviews::ReviewError::BookNotFound) => { + Err(ServerFnError::new("book not found").into()) + } + Err(db::reviews::ReviewError::Sqlx(e)) => { + Err(internal_rpc_error("update review", e).into()) + } + } +} + +/// Delete your whole review of a book, notes included. A no-op when absent. +#[post("/api/rpc/reviews/delete", pool: PoolExt, user: AuthUser)] +pub async fn rpc_delete_review(uuid: String) -> Result<()> { + Ok(db::reviews::delete_review(&pool.0, user.id, &uuid) + .await + .map_err(|e| internal_rpc_error("delete review", e))?) +} diff --git a/mcp/src/server/tests/checkin.rs b/mcp/src/server/tests/checkin.rs index 9ea31d839..ee10bb94c 100644 --- a/mcp/src/server/tests/checkin.rs +++ b/mcp/src/server/tests/checkin.rs @@ -262,8 +262,8 @@ fn combined_router_carries_every_family_without_collisions() { .sum(); let combined = offline_server().tool_router; assert_eq!(combined.list_all().len(), per_family); - // 22 read + 9 checkin + 6 shelf + 6 metadata + 2 merge + 5 content. - assert_eq!(per_family, 50); + // 23 read + 9 checkin + 6 shelf + 6 metadata + 2 merge + 5 content. + assert_eq!(per_family, 51); } #[tokio::test] diff --git a/mcp/src/server/tests/read.rs b/mcp/src/server/tests/read.rs index 2e281800d..05de50d89 100644 --- a/mcp/src/server/tests/read.rs +++ b/mcp/src/server/tests/read.rs @@ -40,6 +40,7 @@ const EXPECTED_TOOLS: &[&str] = &[ "book_read_status", "book_highlights", "book_bookmarks", + "book_reviews", "book_journal_entries", ]; diff --git a/mcp/src/tools/read.rs b/mcp/src/tools/read.rs index 6fc5b29dd..95a6d7eff 100644 --- a/mcp/src/tools/read.rs +++ b/mcp/src/tools/read.rs @@ -9,10 +9,10 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use omnibus_shared::{ - AuthorDetail, AuthorSummary, BookProgress, Bookmark, EbookLibrary, EbookMetadata, GenreWeight, - Highlight, HouseholdReader, JournalEntry, LibraryContents, PhysicalCopy, ProgressFormat, - ReadStatusRecord, ResumePoint, SeriesDetail, SeriesSummary, SessionLogPage, Shelf, - ShelfSummary, SortDir, SortKey, StatsRange, StatsSummary, TagWeight, + AuthorDetail, AuthorSummary, BookProgress, BookReviews, Bookmark, EbookLibrary, EbookMetadata, + GenreWeight, Highlight, HouseholdReader, JournalEntry, LibraryContents, PhysicalCopy, + ProgressFormat, ReadStatusRecord, ResumePoint, ReviewSummary, SeriesDetail, SeriesSummary, + SessionLogPage, Shelf, ShelfSummary, SortDir, SortKey, StatsRange, StatsSummary, TagWeight, }; use crate::client::ClientError; @@ -24,8 +24,9 @@ pub mod views; mod tests; use views::{ - BookProgressView, BookmarkView, HighlightView, JournalEntryView, PhysicalCopyView, - ProgressRecordView, ReadStatusView, ResumePointView, SessionLogEntryView, SessionLogPageView, + BookProgressView, BookmarkView, HighlightView, JournalEntryView, OwnReviewView, + PhysicalCopyView, ProgressRecordView, ReadStatusView, ResumePointView, ReviewView, + SessionLogEntryView, SessionLogPageView, }; /// A single book handle, as returned in `unique_identifier` by the listing @@ -43,7 +44,7 @@ pub struct BookRef { pub enum BookInclude { /// Saved positions, one per format the reader has opened. Progress, - /// want_to_read / reading / finished. + /// unread / reading / finished. ReadStatus, /// Kept lines with their notes. Highlights, @@ -263,6 +264,17 @@ pub struct BookmarkList { pub bookmarks: Vec, } +/// `book_reviews`' answer. +#[derive(Debug, Serialize, JsonSchema)] +pub struct BookReviewsAnswer { + /// The signed-in user's review; null when they haven't reviewed the book. + pub mine: Option, + /// Other readers' reviews: with text first, newest first, then bare ratings. + pub others: Vec, + /// Every reader's stars summed up; null until two readers have rated the book. + pub summary: Option, +} + /// `book_journal_entries`' answer, newest first. #[derive(Debug, Serialize, JsonSchema)] pub struct JournalEntryList { @@ -603,7 +615,7 @@ impl OmnibusMcp { } #[tool( - description = "The signed-in user's read state for one book (want_to_read / reading / finished, with rating context). `read_status` is null when the book has no state yet — treat that as unread. updated_at and finished_at are ISO 8601, with unix seconds alongside under the same name plus _epoch." + description = "The signed-in user's read state for one book: unread, reading or finished. `read_status` is null when the book has no state yet — treat that as unread. Ratings and reviews come from book_reviews. updated_at and finished_at are ISO 8601, with unix seconds alongside under the same name plus _epoch." )] pub async fn book_read_status( &self, @@ -642,6 +654,22 @@ impl OmnibusMcp { Ok(Json(BookmarkList { bookmarks })) } + #[tool( + description = "Reviews of one book: the signed-in user's own (stars from 0.5 to 5 in half steps, Markdown text, private notes only they see, spoiler flag), every other reader's stars and text, and a summary (average, count, half-star buckets) once two readers have rated it. A review flagged as a spoiler gives away the plot in its text, never in its stars. Timestamps are ISO 8601, with unix seconds alongside under the same name plus _epoch." + )] + pub async fn book_reviews( + &self, + Parameters(p): Parameters, + ) -> Result, ErrorData> { + let path = format!("/api/reviews/{}", p.uuid); + let reviews: BookReviews = self.client.get_json(&path, &[]).await?; + Ok(Json(BookReviewsAnswer { + mine: reviews.mine.map(Into::into), + others: reviews.others.into_iter().map(Into::into).collect(), + summary: reviews.summary, + })) + } + #[tool( description = "Journal entries for one book, newest first: every user's published entries plus the signed-in user's own drafts, with rendered HTML bodies. created_at and updated_at are ISO 8601, with unix seconds alongside under the same name plus _epoch." )] diff --git a/mcp/src/tools/read/tests.rs b/mcp/src/tools/read/tests.rs index 2b8005141..b07949e49 100644 --- a/mcp/src/tools/read/tests.rs +++ b/mcp/src/tools/read/tests.rs @@ -14,9 +14,10 @@ use rmcp::handler::server::wrapper::Parameters; use rmcp::model::ErrorCode; use omnibus_shared::{ - BookProgress, Bookmark, Contributor, EbookMetadata, Highlight, HighlightColor, HouseholdReader, - PhysicalCopy, ProgressFormat, ProgressRecord, ReadStatus, ReadStatusRecord, ResumePoint, - SessionFormat, SessionLogEntry, SessionLogPage, StatsSummary, + AttributedReview, BookProgress, BookReviews, Bookmark, Contributor, EbookMetadata, Highlight, + HighlightColor, HouseholdReader, PhysicalCopy, ProgressFormat, ProgressRecord, ReadStatus, + ReadStatusRecord, ResumePoint, Review, SessionFormat, SessionLogEntry, SessionLogPage, + StatsSummary, }; use super::views::iso; @@ -256,6 +257,42 @@ async fn copies(Path(_): Path) -> AxumJson> { }]) } +async fn reviews(Path(_): Path) -> AxumJson { + AxumJson(BookReviews { + mine: Some(Review { + book_uuid: BOOK.into(), + stars: Some(4.5), + body_md: None, + body_html: None, + notes_md: Some("lend to Sam".into()), + notes_html: None, + spoiler: false, + rated_at: Some(WHEN), + created_at: WHEN, + updated_at: WHEN, + rated_at_iso: None, + created_at_iso: None, + updated_at_iso: None, + }), + others: vec![AttributedReview { + user_id: 2, + username: "sam".into(), + has_avatar: false, + stars: None, + body_md: Some("The house is the hero.".into()), + body_html: None, + spoiler: true, + rated_at: None, + created_at: WHEN, + updated_at: WHEN, + rated_at_iso: None, + created_at_iso: None, + updated_at_iso: None, + }], + summary: None, + }) +} + /// Boot a stub instance serving every route these tools reach. async fn stub_service() -> OmnibusMcp { let app = Router::new() @@ -266,6 +303,7 @@ async fn stub_service() -> OmnibusMcp { .route("/api/read-status/{uuid}", get(read_status)) .route("/api/highlights/book/{uuid}", get(highlights)) .route("/api/bookmarks/book/{uuid}", get(bookmarks)) + .route("/api/reviews/{uuid}", get(reviews)) .route("/api/stats", get(stats)) .route("/api/stats/sessions", get(sessions)) .route("/api/physical/{uuid}/copies", get(copies)) @@ -703,3 +741,29 @@ fn the_two_format_vocabularies_are_documented_wherever_they_disagree() { ); } } + +// MARK: - Reviews + +#[tokio::test] +async fn book_reviews_stamps_each_review_and_leaves_absent_fields_out() { + let service = stub_service().await; + + let answer = service + .book_reviews(Parameters(BookRef { uuid: BOOK.into() })) + .await + .unwrap() + .0; + + let mine = json_of(&answer.mine); + assert_eq!(mine["notes_md"], "lend to Sam"); + assert_eq!(mine["rated_at"], "2026-03-04T00:00:00Z"); + assert_eq!(mine["rated_at_epoch"], WHEN); + assert!(mine.get("body_md").is_none(), "absent text is left out"); + let other = json_of(&answer.others[0]); + assert!( + other.get("stars").is_none(), + "a text-only review has no stars" + ); + assert_eq!(other["spoiler"], true); + assert_eq!(other["created_at"], "2026-03-04T00:00:00Z"); +} diff --git a/mcp/src/tools/read/views.rs b/mcp/src/tools/read/views.rs index 2a60e9ebe..b27fa48f4 100644 --- a/mcp/src/tools/read/views.rs +++ b/mcp/src/tools/read/views.rs @@ -14,9 +14,10 @@ use serde::Serialize; use omnibus_shared::cross_format::CrossFormatCandidate; use omnibus_shared::{ - BookProgress, Bookmark, Contributor, EbookMetadata, Highlight, HighlightColor, JournalEntry, - JournalStatus, PhysicalCopy, ProgressFormat, ProgressRecord, ReadStatus, ReadStatusRecord, - ResolvedPosition, ResumePoint, SessionFormat, SessionLogEntry, SessionLogPage, + AttributedReview, BookProgress, Bookmark, Contributor, EbookMetadata, Highlight, + HighlightColor, JournalEntry, JournalStatus, PhysicalCopy, ProgressFormat, ProgressRecord, + ReadStatus, ReadStatusRecord, ResolvedPosition, ResumePoint, Review, SessionFormat, + SessionLogEntry, SessionLogPage, }; /// Unix seconds as an ISO 8601 UTC stamp (`2026-09-04T03:25:36Z`), total over `i64`. @@ -287,6 +288,79 @@ impl From for JournalEntryView { } } +/// The signed-in user's own review of a book, private notes included. +#[derive(Debug, Clone, Serialize, JsonSchema)] +pub struct OwnReviewView { + /// Half-star steps from 0.5 to 5; absent when they gave no stars. + #[serde(skip_serializing_if = "Option::is_none")] + pub stars: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub body_md: Option, + /// Only the signed-in user ever sees these. + #[serde(skip_serializing_if = "Option::is_none")] + pub notes_md: Option, + pub spoiler: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub rated_at: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub rated_at_epoch: Option, + pub created_at: String, + pub created_at_epoch: i64, + pub updated_at: String, + pub updated_at_epoch: i64, +} + +impl From for OwnReviewView { + fn from(r: Review) -> Self { + OwnReviewView { + stars: r.stars, + body_md: r.body_md, + notes_md: r.notes_md, + spoiler: r.spoiler, + rated_at: iso_opt(r.rated_at), + rated_at_epoch: r.rated_at, + created_at: iso(r.created_at), + created_at_epoch: r.created_at, + updated_at: iso(r.updated_at), + updated_at_epoch: r.updated_at, + } + } +} + +/// Another reader's review of a book. Their private notes never reach here. +#[derive(Debug, Clone, Serialize, JsonSchema)] +pub struct ReviewView { + pub user_id: i64, + pub username: String, + /// Half-star steps from 0.5 to 5; absent when they gave no stars. + #[serde(skip_serializing_if = "Option::is_none")] + pub stars: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub body_md: Option, + /// The reviewer flagged the text as containing spoilers. + pub spoiler: bool, + pub created_at: String, + pub created_at_epoch: i64, + pub updated_at: String, + pub updated_at_epoch: i64, +} + +impl From for ReviewView { + fn from(r: AttributedReview) -> Self { + ReviewView { + user_id: r.user_id, + username: r.username, + stars: r.stars, + body_md: r.body_md, + spoiler: r.spoiler, + created_at: iso(r.created_at), + created_at_epoch: r.created_at, + updated_at: iso(r.updated_at), + updated_at_epoch: r.updated_at, + } + } +} + /// One recorded sitting. #[derive(Debug, Clone, Serialize, JsonSchema)] pub struct SessionLogEntryView { diff --git a/openapi/openapi.json b/openapi/openapi.json index 3b8793f2f..bc61df520 100644 --- a/openapi/openapi.json +++ b/openapi/openapi.json @@ -5652,7 +5652,7 @@ "Ratings" ], "summary": "Rate a book", - "description": "Sets your star rating for a book, from 0.5 to 5 in half-star steps, replacing any rating\nyou gave it before.", + "description": "Sets your star rating for a book, from 0.5 to 5 in half-star steps, replacing any rating\nyou gave it before. Any review text and notes stay. Use `PATCH /api/reviews/{uuid}`.", "operationId": "post_rating", "requestBody": { "content": { @@ -5681,7 +5681,8 @@ "404": { "description": "No book has that `book_uuid`." } - } + }, + "deprecated": true } }, "/api/ratings/others/{uuid}": { @@ -5690,7 +5691,7 @@ "Ratings" ], "summary": "List other users' ratings of a book", - "description": "Every other user's rating of the book, with their name, newest first. Returns up to\n1,000, and an empty list for a uuid no book has.", + "description": "Every other user's rating of the book, with their name, most recently rated first.\nReturns up to 1,000, and an empty list for a uuid no book has. Use\n`GET /api/reviews/{uuid}`.", "operationId": "get_other_ratings", "parameters": [ { @@ -5717,7 +5718,8 @@ } } } - } + }, + "deprecated": true } }, "/api/ratings/{uuid}": { @@ -5726,7 +5728,7 @@ "Ratings" ], "summary": "Get your rating for a book", - "description": "The body is `null` when you haven't rated the book, or no book has the uuid.", + "description": "The body is `null` when you haven't rated the book, or no book has the uuid. Use\n`GET /api/reviews/{uuid}`.", "operationId": "get_rating", "parameters": [ { @@ -5757,14 +5759,15 @@ } } } - } + }, + "deprecated": true }, "delete": { "tags": [ "Ratings" ], "summary": "Remove your rating of a book", - "description": "Succeeds whether or not you had rated the book.", + "description": "Clears only the stars: any review text and notes stay. Succeeds whether or not you had\nrated the book. Use `PATCH /api/reviews/{uuid}` with `\"stars\": null`.", "operationId": "delete_rating", "parameters": [ { @@ -5781,7 +5784,8 @@ "204": { "description": "You have no rating for the book." } - } + }, + "deprecated": true } }, "/api/read-status": { @@ -5883,6 +5887,120 @@ } } }, + "/api/reviews/{uuid}": { + "get": { + "tags": [ + "Reviews" + ], + "summary": "Get a book's reviews", + "description": "Your own review, private notes included; every other reader's review with stars or\ntext, without their notes; and a summary of every reader's stars once at least two\nhave rated the book.", + "operationId": "get_book_reviews", + "parameters": [ + { + "name": "uuid", + "in": "path", + "description": "The book's uuid.", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "The book's reviews.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/BookReviews" + } + } + } + }, + "404": { + "description": "No book has that uuid." + } + } + }, + "delete": { + "tags": [ + "Reviews" + ], + "summary": "Delete your review of a book", + "description": "Removes your stars, text and private notes together. Succeeds whether or not you had\nreviewed the book.", + "operationId": "delete_review", + "parameters": [ + { + "name": "uuid", + "in": "path", + "description": "The book's uuid.", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "204": { + "description": "You have no review of the book." + } + } + }, + "patch": { + "tags": [ + "Reviews" + ], + "summary": "Change your review of a book", + "description": "Creates the review on first write. Only the fields you send change: leave a field out\nto keep it, send it as null to clear it. Stars, text and notes are independent, so\nclearing the stars keeps your text. The body is `null` when the change left the review\nwith no stars, text or notes, which deletes it.", + "operationId": "patch_review", + "parameters": [ + { + "name": "uuid", + "in": "path", + "description": "The book's uuid.", + "required": true, + "schema": { + "type": "string" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ReviewPatch" + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "Your review as stored, or `null` if it is now empty and deleted.", + "content": { + "application/json": { + "schema": { + "oneOf": [ + { + "$ref": "#/components/schemas/Review" + }, + { + "type": "null" + } + ] + } + } + } + }, + "400": { + "description": "`stars` is outside 0.5 to 5 or not a whole number of half stars, or `body_md` or `notes_md` is over 65,536 bytes." + }, + "404": { + "description": "No book has that uuid." + } + } + } + }, "/api/scan-library": { "post": { "tags": [ @@ -8601,6 +8719,96 @@ } } }, + "AttributedReview": { + "type": "object", + "description": "Another reader's review of a book, with who wrote it. Never carries their private notes.", + "required": [ + "user_id", + "username", + "spoiler", + "created_at", + "updated_at" + ], + "properties": { + "body_html": { + "type": [ + "string", + "null" + ], + "description": "`body_md` rendered to HTML, with unsafe markup removed." + }, + "body_md": { + "type": [ + "string", + "null" + ], + "description": "The public review text, as Markdown. Null for a rating with no text." + }, + "created_at": { + "type": "integer", + "format": "int64", + "description": "When the review first had stars or text, in Unix seconds." + }, + "created_at_iso": { + "type": [ + "string", + "null" + ], + "description": "The same moment as `created_at`, as an ISO 8601 string in UTC." + }, + "has_avatar": { + "type": "boolean", + "description": "Whether the reviewer has an avatar. `GET /api/users/{user_id}/avatar` serves it." + }, + "rated_at": { + "type": [ + "integer", + "null" + ], + "format": "int64", + "description": "When they last changed the stars, in Unix seconds. Null when there are none." + }, + "rated_at_iso": { + "type": [ + "string", + "null" + ], + "description": "The same moment as `rated_at`, as an ISO 8601 string in UTC." + }, + "spoiler": { + "type": "boolean", + "description": "Whether the reviewer marked the review as containing spoilers. The stars are never\na spoiler; hide only the text." + }, + "stars": { + "type": [ + "number", + "null" + ], + "format": "float", + "description": "The star rating, from 0.5 to 5 in half-star steps. Null when they gave none." + }, + "updated_at": { + "type": "integer", + "format": "int64", + "description": "When they last changed the stars, text or spoiler flag, in Unix seconds." + }, + "updated_at_iso": { + "type": [ + "string", + "null" + ], + "description": "The same moment as `updated_at`, as an ISO 8601 string in UTC." + }, + "user_id": { + "type": "integer", + "format": "int64" + }, + "username": { + "type": "string", + "description": "The reviewer's display name, or their username when they have none." + } + } + }, "AudiobookInspection": { "type": "object", "description": "The metadata `POST /api/uploads/audiobooks/inspect` reads from an uploaded\naudiobook's tags.", @@ -9405,6 +9613,44 @@ } } }, + "BookReviews": { + "type": "object", + "description": "Everything readers have said about one book.", + "required": [ + "others" + ], + "properties": { + "mine": { + "oneOf": [ + { + "$ref": "#/components/schemas/Review", + "description": "Your own review. Null when you haven't reviewed the book." + }, + { + "type": "null" + } + ] + }, + "others": { + "type": "array", + "items": { + "$ref": "#/components/schemas/AttributedReview" + }, + "description": "Other readers' reviews: those with text first, newest first, then ratings with no\ntext, most recently rated first. A review holding only private notes isn't listed." + }, + "summary": { + "oneOf": [ + { + "$ref": "#/components/schemas/ReviewSummary", + "description": "The book's ratings summed up. Null until at least two readers have rated it." + }, + { + "type": "null" + } + ] + } + } + }, "BookSuggestion": { "type": "object", "description": "A book Hardcover suggests for readers of a library book. It is not in the library;\n`hardcover_url` links to it on Hardcover.", @@ -14132,6 +14378,182 @@ } } }, + "Review": { + "type": "object", + "description": "Your review of a book, private notes included.", + "required": [ + "book_uuid", + "spoiler", + "created_at", + "updated_at" + ], + "properties": { + "body_html": { + "type": [ + "string", + "null" + ], + "description": "`body_md` rendered to HTML, with unsafe markup removed." + }, + "body_md": { + "type": [ + "string", + "null" + ], + "description": "Your public review text, as Markdown. Null when you wrote none." + }, + "book_uuid": { + "type": "string" + }, + "created_at": { + "type": "integer", + "format": "int64", + "description": "When the review first had stars or text, in Unix seconds. For a review holding only\nnotes, when you wrote them." + }, + "created_at_iso": { + "type": [ + "string", + "null" + ], + "description": "The same moment as `created_at`, as an ISO 8601 string in UTC." + }, + "notes_html": { + "type": [ + "string", + "null" + ], + "description": "`notes_md` rendered to HTML, with unsafe markup removed." + }, + "notes_md": { + "type": [ + "string", + "null" + ], + "description": "Your private notes, as Markdown. Only you ever see them. Null when you wrote none." + }, + "rated_at": { + "type": [ + "integer", + "null" + ], + "format": "int64", + "description": "When you last changed the stars, in Unix seconds. Null when there are none." + }, + "rated_at_iso": { + "type": [ + "string", + "null" + ], + "description": "The same moment as `rated_at`, as an ISO 8601 string in UTC." + }, + "spoiler": { + "type": "boolean", + "description": "Whether you marked the review as containing spoilers." + }, + "stars": { + "type": [ + "number", + "null" + ], + "format": "float", + "description": "Your star rating, from 0.5 to 5 in half-star steps. Null when you gave none." + }, + "updated_at": { + "type": "integer", + "format": "int64", + "description": "When you last changed the stars, text or spoiler flag, in Unix seconds. Editing\nyour notes doesn't move it, because other readers see it too." + }, + "updated_at_iso": { + "type": [ + "string", + "null" + ], + "description": "The same moment as `updated_at`, as an ISO 8601 string in UTC." + } + } + }, + "ReviewBucket": { + "type": "object", + "description": "How many readers gave a book one star value.", + "required": [ + "half_stars", + "readers" + ], + "properties": { + "half_stars": { + "type": "integer", + "format": "int64", + "description": "The rating in half stars, from 1 to 10. Divide by 2 for stars, so 10 is 5 stars." + }, + "readers": { + "type": "integer", + "format": "int64" + } + } + }, + "ReviewPatch": { + "type": "object", + "description": "The changes `PATCH /api/reviews/{uuid}` makes to your review of a book. A field left\nout stays as it is; a field sent as null is cleared. Blank text counts as null. A\nreview left with no stars, text or notes is deleted.", + "properties": { + "body_md": { + "type": [ + "string", + "null" + ], + "description": "The public review text, as Markdown, up to 65,536 bytes." + }, + "notes_md": { + "type": [ + "string", + "null" + ], + "description": "Private notes only you see, as Markdown, up to 65,536 bytes." + }, + "spoiler": { + "type": [ + "boolean", + "null" + ], + "description": "Whether the review contains spoilers." + }, + "stars": { + "type": [ + "number", + "null" + ], + "format": "float", + "description": "The star rating, from 0.5 to 5 in half-star steps. Null clears the stars and keeps\nany text and notes." + } + } + }, + "ReviewSummary": { + "type": "object", + "description": "Every reader's rating of a book, yours included, summed up.", + "required": [ + "average_stars", + "ratings", + "buckets" + ], + "properties": { + "average_stars": { + "type": "number", + "format": "float", + "description": "The mean rating, in stars." + }, + "buckets": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ReviewBucket" + }, + "description": "All ten half-star values, lowest first, zeros included." + }, + "ratings": { + "type": "integer", + "format": "int64", + "description": "How many readers rated the book." + } + } + }, "RuleField": { "type": "string", "description": "A field a smart shelf condition can match on.", @@ -16507,9 +16929,13 @@ "name": "Journals", "description": "Journal entries about a book, and the images in them." }, + { + "name": "Reviews", + "description": "Book reviews: your stars, text and private notes, and what other readers wrote." + }, { "name": "Ratings", - "description": "Star ratings." + "description": "Star ratings alone. Superseded by Reviews, which carry the same stars." }, { "name": "Read status", diff --git a/server/src/backend.rs b/server/src/backend.rs index 6da11de3a..618c1fad4 100644 --- a/server/src/backend.rs +++ b/server/src/backend.rs @@ -54,6 +54,7 @@ mod progress; mod ratings; mod react_ui; mod read_status; +mod reviews; mod routes; mod scan; mod search; diff --git a/server/src/backend/openapi.rs b/server/src/backend/openapi.rs index 66e419c9b..7fcdaee20 100644 --- a/server/src/backend/openapi.rs +++ b/server/src/backend/openapi.rs @@ -43,7 +43,8 @@ use utoipa::{ (name = "Progress", description = "Reading and listening positions, reading sessions, and moving between a book's formats."), (name = "Annotations", description = "Highlights and bookmarks."), (name = "Journals", description = "Journal entries about a book, and the images in them."), - (name = "Ratings", description = "Star ratings."), + (name = "Reviews", description = "Book reviews: your stars, text and private notes, and what other readers wrote."), + (name = "Ratings", description = "Star ratings alone. Superseded by Reviews, which carry the same stars."), (name = "Read status", description = "Whether you have read a book, and when."), (name = "Stats", description = "Reading statistics and goals: yours, and those other readers share."), (name = "Shelves", description = "Shelves you fill by hand, and smart shelves that fill themselves from a rule."), diff --git a/server/src/backend/ratings.rs b/server/src/backend/ratings.rs index 5637cb1f7..f97efba4e 100644 --- a/server/src/backend/ratings.rs +++ b/server/src/backend/ratings.rs @@ -1,8 +1,8 @@ -//! Star-rating REST handlers for the mobile client (`/api/ratings*`): set, get, -//! and clear a half-star rating (`0.5..=5.0`) per `(user, book)`. Writes are -//! last-write-wins, with the range and half-step validated at the boundary so -//! an invalid value returns 400. Web clients use the `/api/rpc/ratings*` server -//! functions in `omnibus_frontend::rpc`. +//! Star-rating REST handlers (`/api/ratings*`), the stars-only view of a +//! review that installed iOS builds still call: set, get, and clear a +//! half-star rating (`0.5..=5.0`) per `(user, book)`, with the range and +//! half-step validated at the boundary so an invalid value returns 400. Each is +//! `#[deprecated]`, which marks it so in the spec; `/api/reviews` replaces them. use axum::{ extract::{Path, State}, @@ -18,7 +18,7 @@ use crate::auth::AuthUser; /// Rate a book /// /// Sets your star rating for a book, from 0.5 to 5 in half-star steps, replacing any rating -/// you gave it before. +/// you gave it before. Any review text and notes stay. Use `PATCH /api/reviews/{uuid}`. #[utoipa::path( post, path = "/api/ratings", @@ -30,6 +30,7 @@ use crate::auth::AuthUser; (status = 404, description = "No book has that `book_uuid`."), ), )] +#[deprecated = "use the /api/reviews routes"] pub(super) async fn post_rating( user: AuthUser, State(state): State, @@ -49,7 +50,8 @@ pub(super) async fn post_rating( /// Get your rating for a book /// -/// The body is `null` when you haven't rated the book, or no book has the uuid. +/// The body is `null` when you haven't rated the book, or no book has the uuid. Use +/// `GET /api/reviews/{uuid}`. #[utoipa::path( get, path = "/api/ratings/{uuid}", @@ -57,6 +59,7 @@ pub(super) async fn post_rating( params(("uuid" = String, Path, description = "The book's uuid.")), responses((status = 200, description = "Your rating, or `null`.", body = Option)), )] +#[deprecated = "use the /api/reviews routes"] pub(super) async fn get_rating( user: AuthUser, State(state): State, @@ -70,8 +73,9 @@ pub(super) async fn get_rating( /// List other users' ratings of a book /// -/// Every other user's rating of the book, with their name, newest first. Returns up to -/// 1,000, and an empty list for a uuid no book has. +/// Every other user's rating of the book, with their name, most recently rated first. +/// Returns up to 1,000, and an empty list for a uuid no book has. Use +/// `GET /api/reviews/{uuid}`. #[utoipa::path( get, path = "/api/ratings/others/{uuid}", @@ -79,6 +83,7 @@ pub(super) async fn get_rating( params(("uuid" = String, Path, description = "The book's uuid.")), responses((status = 200, description = "Other users' ratings, newest first.", body = Vec)), )] +#[deprecated = "use the /api/reviews routes"] pub(super) async fn get_other_ratings( user: AuthUser, State(state): State, @@ -92,7 +97,8 @@ pub(super) async fn get_other_ratings( /// Remove your rating of a book /// -/// Succeeds whether or not you had rated the book. +/// Clears only the stars: any review text and notes stay. Succeeds whether or not you had +/// rated the book. Use `PATCH /api/reviews/{uuid}` with `"stars": null`. #[utoipa::path( delete, path = "/api/ratings/{uuid}", @@ -100,6 +106,7 @@ pub(super) async fn get_other_ratings( params(("uuid" = String, Path, description = "The book's uuid.")), responses((status = 204, description = "You have no rating for the book.")), )] +#[deprecated = "use the /api/reviews routes"] pub(super) async fn delete_rating( user: AuthUser, State(state): State, diff --git a/server/src/backend/reviews.rs b/server/src/backend/reviews.rs new file mode 100644 index 000000000..d3dc2af7f --- /dev/null +++ b/server/src/backend/reviews.rs @@ -0,0 +1,103 @@ +//! Book review REST handlers (`/api/reviews/{uuid}`): read a book's reviews, +//! patch your own field by field, delete it. A patch is validated at the +//! boundary so bad stars or oversized text return 400. Private notes reach +//! only their author; `db::reviews` enforces that by type. Web clients use +//! the `/api/rpc/reviews/*` server functions in `omnibus_frontend::rpc`. + +use axum::{ + extract::{Path, State}, + http::StatusCode, + response::{IntoResponse, Response}, + Json, +}; +use omnibus_db::{self as db, reviews::ReviewError}; +use omnibus_shared::{BookReviews, Review, ReviewPatch}; + +use super::{internal, AppState}; +use crate::auth::AuthUser; + +/// Get a book's reviews +/// +/// Your own review, private notes included; every other reader's review with stars or +/// text, without their notes; and a summary of every reader's stars once at least two +/// have rated the book. +#[utoipa::path( + get, + path = "/api/reviews/{uuid}", + tag = "Reviews", + params(("uuid" = String, Path, description = "The book's uuid.")), + responses( + (status = 200, description = "The book's reviews.", body = BookReviews), + (status = 404, description = "No book has that uuid."), + ), +)] +pub(super) async fn get_book_reviews( + user: AuthUser, + State(state): State, + Path(uuid): Path, +) -> Response { + match db::reviews::get_book_reviews(&state.pool, user.id, &uuid).await { + Ok(reviews) => Json(reviews).into_response(), + Err(ReviewError::BookNotFound) => (StatusCode::NOT_FOUND, "book not found").into_response(), + Err(ReviewError::Sqlx(e)) => internal("get_book_reviews", e), + } +} + +/// Change your review of a book +/// +/// Creates the review on first write. Only the fields you send change: leave a field out +/// to keep it, send it as null to clear it. Stars, text and notes are independent, so +/// clearing the stars keeps your text. The body is `null` when the change left the review +/// with no stars, text or notes, which deletes it. +#[utoipa::path( + patch, + path = "/api/reviews/{uuid}", + tag = "Reviews", + params(("uuid" = String, Path, description = "The book's uuid.")), + request_body = ReviewPatch, + responses( + (status = 200, description = "Your review as stored, or `null` if it is now empty and deleted.", body = Option), + (status = 400, description = "`stars` is outside 0.5 to 5 or not a whole number of half stars, or `body_md` or `notes_md` is over 65,536 bytes."), + (status = 404, description = "No book has that uuid."), + ), +)] +pub(super) async fn patch_review( + user: AuthUser, + State(state): State, + Path(uuid): Path, + Json(patch): Json, +) -> Response { + if let Err(msg) = patch.validate() { + return (StatusCode::BAD_REQUEST, msg).into_response(); + } + match db::reviews::update_review(&state.pool, user.id, &uuid, &patch).await { + Ok(review) => Json(review).into_response(), + Err(ReviewError::BookNotFound) => (StatusCode::NOT_FOUND, "book not found").into_response(), + Err(ReviewError::Sqlx(e)) => internal("patch_review", e), + } +} + +/// Delete your review of a book +/// +/// Removes your stars, text and private notes together. Succeeds whether or not you had +/// reviewed the book. +#[utoipa::path( + delete, + path = "/api/reviews/{uuid}", + tag = "Reviews", + params(("uuid" = String, Path, description = "The book's uuid.")), + responses((status = 204, description = "You have no review of the book.")), +)] +pub(super) async fn delete_review( + user: AuthUser, + State(state): State, + Path(uuid): Path, +) -> Response { + match db::reviews::delete_review(&state.pool, user.id, &uuid).await { + Ok(()) => StatusCode::NO_CONTENT.into_response(), + Err(e) => internal("delete_review", e), + } +} + +#[cfg(test)] +mod tests; diff --git a/server/src/backend/reviews/tests.rs b/server/src/backend/reviews/tests.rs new file mode 100644 index 000000000..e676393e2 --- /dev/null +++ b/server/src/backend/reviews/tests.rs @@ -0,0 +1,259 @@ +//! Tests for the book review REST handlers. +use axum::http::StatusCode; +use omnibus_shared::{BookReviews, Review}; +use tower::ServiceExt; + +use crate::auth::test_support as auth_test_support; +use crate::backend::test_support::*; + +async fn reader(pool: &sqlx::SqlitePool, name: &str) -> (i64, String) { + let user = auth_test_support::create_user(pool, name).await; + let token = auth_test_support::bearer_token(pool, user.id).await; + (user.id, token) +} + +fn patch( + uuid: &str, + token: &str, + body: serde_json::Value, +) -> axum::http::Request { + request( + "PATCH", + &format!("/api/reviews/{uuid}"), + Some(token), + Some(body), + ) +} + +#[tokio::test] +async fn api_patch_review_requires_auth() { + let (app, _state, _pool) = fixture().await; + let res = app + .oneshot(request( + "PATCH", + "/api/reviews/x", + None, + Some(serde_json::json!({ "stars": 3 })), + )) + .await + .unwrap(); + assert_eq!(res.status(), StatusCode::UNAUTHORIZED); +} + +#[tokio::test] +async fn api_patch_review_creates_then_changes_only_the_fields_sent() { + let (app, _state, pool) = fixture().await; + let (_, uuid) = seed_book_with_uuid(&pool, "/lib", "Book A").await; + let (_, token) = reader(&pool, "alice").await; + + let res = app + .clone() + .oneshot(patch(&uuid, &token, serde_json::json!({ "stars": 4.5 }))) + .await + .unwrap(); + assert_eq!(res.status(), StatusCode::OK); + + let res = app + .oneshot(patch( + &uuid, + &token, + serde_json::json!({ "body_md": "*Great*", "notes_md": "mine", "spoiler": true }), + )) + .await + .unwrap(); + assert_eq!(res.status(), StatusCode::OK); + let review: Option = body_json(res).await; + let review = review.unwrap(); + assert_eq!(review.stars, Some(4.5)); + assert_eq!(review.notes_md.as_deref(), Some("mine")); + assert!(review.spoiler); + assert!(review.body_html.unwrap().contains("Great")); +} + +#[tokio::test] +async fn api_patch_review_returns_null_when_the_patch_empties_the_review() { + let (app, _state, pool) = fixture().await; + let (_, uuid) = seed_book_with_uuid(&pool, "/lib", "Book A").await; + let (_, token) = reader(&pool, "alice").await; + app.clone() + .oneshot(patch(&uuid, &token, serde_json::json!({ "stars": 2 }))) + .await + .unwrap(); + + let res = app + .oneshot(patch(&uuid, &token, serde_json::json!({ "stars": null }))) + .await + .unwrap(); + + assert_eq!(res.status(), StatusCode::OK); + let review: Option = body_json(res).await; + assert_eq!(review, None); +} + +#[tokio::test] +async fn api_patch_review_rejects_stars_off_the_half_star_scale() { + let (app, _state, pool) = fixture().await; + let (_, uuid) = seed_book_with_uuid(&pool, "/lib", "Book A").await; + let (_, token) = reader(&pool, "alice").await; + + let res = app + .oneshot(patch(&uuid, &token, serde_json::json!({ "stars": 4.3 }))) + .await + .unwrap(); + + assert_eq!(res.status(), StatusCode::BAD_REQUEST); +} + +#[tokio::test] +async fn api_patch_review_returns_404_for_an_unknown_book() { + let (app, _state, pool) = fixture().await; + let (_, token) = reader(&pool, "alice").await; + + let res = app + .oneshot(patch( + "no-such-book", + &token, + serde_json::json!({ "stars": 3 }), + )) + .await + .unwrap(); + + assert_eq!(res.status(), StatusCode::NOT_FOUND); +} + +#[tokio::test] +async fn api_patch_review_returns_500_on_db_failure() { + let (app, _state, pool) = fixture().await; + let (_, uuid) = seed_book_with_uuid(&pool, "/lib", "Book A").await; + let (_, token) = reader(&pool, "alice").await; + sqlx::query("DROP TABLE reviews") + .execute(&pool) + .await + .unwrap(); + + let res = app + .oneshot(patch(&uuid, &token, serde_json::json!({ "stars": 3 }))) + .await + .unwrap(); + + assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); +} + +#[tokio::test] +async fn api_get_book_reviews_hides_other_readers_notes() { + let (app, _state, pool) = fixture().await; + let (_, uuid) = seed_book_with_uuid(&pool, "/lib", "Book A").await; + let (_, alice) = reader(&pool, "alice").await; + let (bob_id, bob) = reader(&pool, "bob").await; + app.clone() + .oneshot(patch(&uuid, &alice, serde_json::json!({ "stars": 3 }))) + .await + .unwrap(); + app.clone() + .oneshot(patch( + &uuid, + &bob, + serde_json::json!({ "stars": 5, "body_md": "Loved it", "notes_md": "bob only" }), + )) + .await + .unwrap(); + + let res = app + .oneshot(get_with_bearer(&format!("/api/reviews/{uuid}"), &alice)) + .await + .unwrap(); + + assert_eq!(res.status(), StatusCode::OK); + let text = body_text(res).await; + assert!(!text.contains("bob only"), "notes leaked: {text}"); + let reviews: BookReviews = serde_json::from_str(&text).unwrap(); + assert_eq!(reviews.mine.unwrap().stars, Some(3.0)); + assert_eq!(reviews.others.len(), 1); + assert_eq!(reviews.others[0].user_id, bob_id); + assert_eq!(reviews.summary.unwrap().average_stars, 4.0); +} + +#[tokio::test] +async fn api_get_book_reviews_returns_404_for_an_unknown_book() { + let (app, _state, pool) = fixture().await; + let (_, token) = reader(&pool, "alice").await; + + let res = app + .oneshot(get_with_bearer("/api/reviews/no-such-book", &token)) + .await + .unwrap(); + + assert_eq!(res.status(), StatusCode::NOT_FOUND); +} + +#[tokio::test] +async fn api_get_book_reviews_returns_500_on_db_failure() { + let (app, _state, pool) = fixture().await; + let (_, uuid) = seed_book_with_uuid(&pool, "/lib", "Book A").await; + let (_, token) = reader(&pool, "alice").await; + sqlx::query("DROP TABLE reviews") + .execute(&pool) + .await + .unwrap(); + + let res = app + .oneshot(get_with_bearer(&format!("/api/reviews/{uuid}"), &token)) + .await + .unwrap(); + + assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); +} + +#[tokio::test] +async fn api_delete_review_removes_it_and_succeeds_when_absent() { + let (app, _state, pool) = fixture().await; + let (_, uuid) = seed_book_with_uuid(&pool, "/lib", "Book A").await; + let (_, token) = reader(&pool, "alice").await; + app.clone() + .oneshot(patch(&uuid, &token, serde_json::json!({ "body_md": "x" }))) + .await + .unwrap(); + let delete = || { + request( + "DELETE", + &format!("/api/reviews/{uuid}"), + Some(&token), + None, + ) + }; + + let first = app.clone().oneshot(delete()).await.unwrap(); + let second = app.clone().oneshot(delete()).await.unwrap(); + + assert_eq!(first.status(), StatusCode::NO_CONTENT); + assert_eq!(second.status(), StatusCode::NO_CONTENT); + let res = app + .oneshot(get_with_bearer(&format!("/api/reviews/{uuid}"), &token)) + .await + .unwrap(); + let reviews: BookReviews = body_json(res).await; + assert_eq!(reviews.mine, None); +} + +#[tokio::test] +async fn api_delete_review_returns_500_on_db_failure() { + let (app, _state, pool) = fixture().await; + let (_, uuid) = seed_book_with_uuid(&pool, "/lib", "Book A").await; + let (_, token) = reader(&pool, "alice").await; + sqlx::query("DROP TABLE reviews") + .execute(&pool) + .await + .unwrap(); + + let res = app + .oneshot(request( + "DELETE", + &format!("/api/reviews/{uuid}"), + Some(&token), + None, + )) + .await + .unwrap(); + + assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); +} diff --git a/server/src/backend/routes.rs b/server/src/backend/routes.rs index f5b9b1c88..2d28ea3e8 100644 --- a/server/src/backend/routes.rs +++ b/server/src/backend/routes.rs @@ -13,9 +13,9 @@ use crate::rate_limit::{rate_limit_by_ip, RateLimiter, RateLimits}; use super::{ account, admin_health, admin_sessions, audiobooks, author_photos, authors, bookmarks, cleanup, covers, cross_format, deletion, ebooks, genres, highlights, journals, kindle, kobo_devices, - logs, merge, metadata, overrides, physical, profile, progress, ratings, read_status, scan, - search, series, settings, shelves, stats, suggestions, summary, tags, tasks, uploads, users, - AppState, + logs, merge, metadata, overrides, physical, profile, progress, ratings, read_status, reviews, + scan, search, series, settings, shelves, stats, suggestions, summary, tags, tasks, uploads, + users, AppState, }; /// Health check, settings, ebooks, and audiobook playback routes. @@ -190,9 +190,13 @@ fn bookmark_routes() -> OpenApiRouter { .routes(routes!(bookmarks::put_bookmark, bookmarks::delete_bookmark)) } -/// Reading stats, star ratings, and read/unread state — -/// mobile-facing REST. Web hits the analogous `/api/rpc/stats`, -/// `/api/rpc/ratings/*`, and `/api/rpc/read-status/*` server functions. +/// Reading stats, reviews (and the deprecated stars-only ratings view), and +/// read/unread state — mobile-facing REST. Web hits the analogous +/// `/api/rpc/stats`, `/api/rpc/reviews/*`, `/api/rpc/ratings/*`, and +/// `/api/rpc/read-status/*` server functions. +// The ratings handlers are `#[deprecated]` so the spec marks them; serving them +// until installed clients move to `/api/reviews` is deliberate. +#[allow(deprecated)] fn engagement_routes() -> OpenApiRouter { OpenApiRouter::new() .routes(routes!(stats::get_household_readers)) @@ -207,6 +211,11 @@ fn engagement_routes() -> OpenApiRouter { .routes(routes!(ratings::post_rating)) .routes(routes!(ratings::get_other_ratings)) .routes(routes!(ratings::get_rating, ratings::delete_rating)) + .routes(routes!( + reviews::get_book_reviews, + reviews::patch_review, + reviews::delete_review + )) .routes(routes!(read_status::put_read_status)) .routes(routes!(read_status::get_read_status)) } From a4441edb415e46c87332f122e0335baf8633327d Mon Sep 17 00:00:00 2001 From: roberte777 Date: Tue, 20 Oct 2026 17:18:47 -0500 Subject: [PATCH 2/3] fix: tighten the review API's spoiler contract, MCP not-found and auth coverage - ReviewPatch.spoiler documents as a non-nullable boolean: leave it out to keep it - book_reviews reports an unknown uuid as book not found, not an internal error - cover anonymous GET and DELETE on /api/reviews; a refused DELETE keeps the review - document review image embeds and cleanup, and the text-review sort order - ratings routes name their Android and iOS callers; rule 13 covers deprecation - share the reader test helper through backend::test_support --- .claude/rules/13-openapi.md | 4 ++ .claude/skills/add-backend-route/SKILL.md | 2 +- mcp/src/tools/read.rs | 6 ++- mcp/src/tools/read/tests.rs | 44 ++++++++++++++++---- openapi/openapi.json | 21 ++++------ server/src/backend/ebooks/tests/stacks.rs | 8 ---- server/src/backend/journals.rs | 8 ++-- server/src/backend/ratings.rs | 11 ++--- server/src/backend/reviews.rs | 12 ++++-- server/src/backend/reviews/tests.rs | 50 +++++++++++++++++++---- server/src/backend/test_support.rs | 7 ++++ shared/src/reviews.rs | 12 +++--- 12 files changed, 131 insertions(+), 54 deletions(-) diff --git a/.claude/rules/13-openapi.md b/.claude/rules/13-openapi.md index 19f239388..d174fe2ff 100644 --- a/.claude/rules/13-openapi.md +++ b/.claude/rules/13-openapi.md @@ -65,6 +65,10 @@ pub(super) async fn get_ebook_by_uuid(/* … */) -> Response { on duplicates. - **A type only a query parameter references** isn't collected by utoipa. Add it to `components(schemas(...))` in `openapi.rs`; a test fails on a dangling `$ref`. +- **Deprecating a route:** put `#[deprecated = "use …"]` on the handler, which sets the + operation's `deprecated` flag in the spec, and say what replaces it in the description. + Keep serving it while installed clients call it: `#[allow(deprecated)]` on the + `routes.rs` builder that registers it (`backend/ratings.rs` is the example). ## Writing the docs diff --git a/.claude/skills/add-backend-route/SKILL.md b/.claude/skills/add-backend-route/SKILL.md index c78bdc905..1a99158fc 100644 --- a/.claude/skills/add-backend-route/SKILL.md +++ b/.claude/skills/add-backend-route/SKILL.md @@ -54,7 +54,7 @@ Write the handler function in the matching per-domain module under [server/src/backend/](../../../server/src/backend/) (e.g. `backend/ebooks.rs`, `backend/progress.rs`), give it a `#[utoipa::path(...)]` attribute with the method, path, tag, params, body and every response with a description (see -[13-openapi.md](../../rules/13-openapi.md), and any handler in `backend/ratings.rs` +[13-openapi.md](../../rules/13-openapi.md), and any handler in `backend/reviews.rs` for the shape), then register it with `.routes(routes!(handler))` inside the matching builder function in [server/src/backend/routes.rs](../../../server/src/backend/routes.rs) — diff --git a/mcp/src/tools/read.rs b/mcp/src/tools/read.rs index 95a6d7eff..9d494e657 100644 --- a/mcp/src/tools/read.rs +++ b/mcp/src/tools/read.rs @@ -269,7 +269,8 @@ pub struct BookmarkList { pub struct BookReviewsAnswer { /// The signed-in user's review; null when they haven't reviewed the book. pub mine: Option, - /// Other readers' reviews: with text first, newest first, then bare ratings. + /// Other readers' reviews: with text first, newest first by when the text was first + /// written, then bare ratings, most recently rated first. pub others: Vec, /// Every reader's stars summed up; null until two readers have rated the book. pub summary: Option, @@ -662,7 +663,8 @@ impl OmnibusMcp { Parameters(p): Parameters, ) -> Result, ErrorData> { let path = format!("/api/reviews/{}", p.uuid); - let reviews: BookReviews = self.client.get_json(&path, &[]).await?; + let reviews: Option = self.client.get_json_opt(&path, &[]).await?; + let reviews = reviews.ok_or_else(|| not_found("book"))?; Ok(Json(BookReviewsAnswer { mine: reviews.mine.map(Into::into), others: reviews.others.into_iter().map(Into::into).collect(), diff --git a/mcp/src/tools/read/tests.rs b/mcp/src/tools/read/tests.rs index b07949e49..cd13b4d6a 100644 --- a/mcp/src/tools/read/tests.rs +++ b/mcp/src/tools/read/tests.rs @@ -16,8 +16,8 @@ use rmcp::model::ErrorCode; use omnibus_shared::{ AttributedReview, BookProgress, BookReviews, Bookmark, Contributor, EbookMetadata, Highlight, HighlightColor, HouseholdReader, PhysicalCopy, ProgressFormat, ProgressRecord, ReadStatus, - ReadStatusRecord, ResumePoint, Review, SessionFormat, SessionLogEntry, SessionLogPage, - StatsSummary, + ReadStatusRecord, ResumePoint, Review, ReviewBucket, ReviewSummary, SessionFormat, + SessionLogEntry, SessionLogPage, StatsSummary, }; use super::views::iso; @@ -257,8 +257,11 @@ async fn copies(Path(_): Path) -> AxumJson> { }]) } -async fn reviews(Path(_): Path) -> AxumJson { - AxumJson(BookReviews { +async fn reviews(Path(uuid): Path) -> Result, StatusCode> { + if uuid != BOOK { + return Err(StatusCode::NOT_FOUND); + } + Ok(AxumJson(BookReviews { mine: Some(Review { book_uuid: BOOK.into(), stars: Some(4.5), @@ -289,8 +292,20 @@ async fn reviews(Path(_): Path) -> AxumJson { created_at_iso: None, updated_at_iso: None, }], - summary: None, - }) + summary: Some(review_summary()), + })) +} + +fn review_summary() -> ReviewSummary { + let bucket = |half_stars| ReviewBucket { + half_stars, + readers: 1, + }; + ReviewSummary { + average_stars: 4.25, + ratings: 2, + buckets: vec![bucket(8), bucket(9)], + } } /// Boot a stub instance serving every route these tools reach. @@ -745,7 +760,7 @@ fn the_two_format_vocabularies_are_documented_wherever_they_disagree() { // MARK: - Reviews #[tokio::test] -async fn book_reviews_stamps_each_review_and_leaves_absent_fields_out() { +async fn book_reviews_stamps_each_review_and_passes_the_summary_through() { let service = stub_service().await; let answer = service @@ -766,4 +781,19 @@ async fn book_reviews_stamps_each_review_and_leaves_absent_fields_out() { ); assert_eq!(other["spoiler"], true); assert_eq!(other["created_at"], "2026-03-04T00:00:00Z"); + assert_eq!(answer.summary, Some(review_summary())); +} + +#[tokio::test] +async fn book_reviews_reports_not_found_for_an_unknown_uuid() { + let service = stub_service().await; + let err = expect_err( + service + .book_reviews(Parameters(BookRef { + uuid: "uuid-missing".into(), + })) + .await, + ); + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("book not found")); } diff --git a/openapi/openapi.json b/openapi/openapi.json index cca728424..cb9c7be61 100644 --- a/openapi/openapi.json +++ b/openapi/openapi.json @@ -4722,7 +4722,7 @@ "Journals" ], "summary": "Upload a journal image", - "description": "Stores an image to embed in a journal entry and returns its URL. Put the URL in the\nentry's markdown as `![caption](url)`.", + "description": "Stores an image to embed in a journal entry or review and returns its URL. Put the URL\nin the markdown as `![caption](url)`.", "operationId": "post_journal_image", "requestBody": { "content": { @@ -4771,7 +4771,7 @@ "Journals" ], "summary": "Get a journal image", - "description": "An image uploaded for a journal entry, by the name at the end of its URL. A name never\npoints at different bytes, so the response may be cached for a year.", + "description": "An image uploaded for a journal entry or review, by the name at the end of its URL. A\nname never points at different bytes, so the response may be cached for a year.", "operationId": "get_journal_image", "parameters": [ { @@ -5706,7 +5706,7 @@ ], "responses": { "200": { - "description": "Other users' ratings, newest first.", + "description": "Other users' ratings, most recently rated first.", "content": { "application/json": { "schema": { @@ -5927,7 +5927,7 @@ "Reviews" ], "summary": "Delete your review of a book", - "description": "Removes your stars, text and private notes together. Succeeds whether or not you had\nreviewed the book.", + "description": "Removes your stars, text and private notes together. An uploaded image they showed,\nand no journal entry or review still shows, is deleted. Succeeds whether or not you had\nreviewed the book.", "operationId": "delete_review", "parameters": [ { @@ -5951,7 +5951,7 @@ "Reviews" ], "summary": "Change your review of a book", - "description": "Creates the review on first write. Only the fields you send change: leave a field out\nto keep it, send it as null to clear it. Stars, text and notes are independent, so\nclearing the stars keeps your text. The body is `null` when the change left the review\nwith no stars, text or notes, which deletes it.", + "description": "Creates the review on first write. Only the fields you send change: leave a field out\nto keep it. Send `stars`, `body_md` or `notes_md` as null to clear it; `spoiler` is\ntrue or false, so leave it out to keep it. Stars, text and notes are independent, so\nclearing the stars keeps your text. The body is `null` when the change left the review\nwith no stars, text or notes, which deletes it.\n\nText and notes can embed images uploaded with `POST /api/journals/images`. An uploaded\nimage the change drops, and no journal entry or review still shows, is deleted.", "operationId": "patch_review", "parameters": [ { @@ -9636,7 +9636,7 @@ "items": { "$ref": "#/components/schemas/AttributedReview" }, - "description": "Other readers' reviews: those with text first, newest first, then ratings with no\ntext, most recently rated first. A review holding only private notes isn't listed." + "description": "Other readers' reviews: those with text first, newest first by when the text was\nfirst written, then ratings with no text, most recently rated first. A review\nholding only private notes isn't listed." }, "summary": { "oneOf": [ @@ -14493,7 +14493,7 @@ }, "ReviewPatch": { "type": "object", - "description": "The changes `PATCH /api/reviews/{uuid}` makes to your review of a book. A field left\nout stays as it is; a field sent as null is cleared. Blank text counts as null. A\nreview left with no stars, text or notes is deleted.", + "description": "The changes `PATCH /api/reviews/{uuid}` makes to your review of a book. A field left\nout stays as it is; `stars`, `body_md` or `notes_md` sent as null is cleared. Blank\ntext counts as null. A review left with no stars, text or notes is deleted.", "properties": { "body_md": { "type": [ @@ -14510,11 +14510,8 @@ "description": "Private notes only you see, as Markdown, up to 65,536 bytes." }, "spoiler": { - "type": [ - "boolean", - "null" - ], - "description": "Whether the review contains spoilers." + "type": "boolean", + "description": "Whether the review contains spoilers: true or false; leave it out to keep it." }, "stars": { "type": [ diff --git a/server/src/backend/ebooks/tests/stacks.rs b/server/src/backend/ebooks/tests/stacks.rs index ba5617235..a1bdb8ce2 100644 --- a/server/src/backend/ebooks/tests/stacks.rs +++ b/server/src/backend/ebooks/tests/stacks.rs @@ -7,7 +7,6 @@ use omnibus_db::test_support::indexed; use omnibus_shared::{SeriesStack, Settings, StackMemberState}; use tower::ServiceExt; -use crate::auth::test_support as auth_test_support; use crate::backend::test_support::*; use super::super::*; @@ -41,13 +40,6 @@ fn saga_and_lone() -> Vec { ] } -/// A new reader's id and bearer token. -async fn reader(pool: &sqlx::SqlitePool, name: &str) -> (i64, String) { - let user = auth_test_support::create_user(pool, name).await; - let token = auth_test_support::bearer_token(pool, user.id).await; - (user.id, token) -} - /// One `GET /api/ebooks` response, unpacked. struct Page { total: Option, diff --git a/server/src/backend/journals.rs b/server/src/backend/journals.rs index 3020f864b..ec122d372 100644 --- a/server/src/backend/journals.rs +++ b/server/src/backend/journals.rs @@ -169,8 +169,8 @@ pub(super) async fn post_journal_preview(_user: AuthUser, Json(body_md): Json)), + responses((status = 200, description = "Other users' ratings, most recently rated first.", body = Vec)), )] #[deprecated = "use the /api/reviews routes"] pub(super) async fn get_other_ratings( diff --git a/server/src/backend/reviews.rs b/server/src/backend/reviews.rs index d3dc2af7f..aac6df3e9 100644 --- a/server/src/backend/reviews.rs +++ b/server/src/backend/reviews.rs @@ -13,9 +13,10 @@ use axum::{ use omnibus_db::{self as db, reviews::ReviewError}; use omnibus_shared::{BookReviews, Review, ReviewPatch}; -use super::{internal, AppState}; use crate::auth::AuthUser; +use super::{internal, AppState}; + /// Get a book's reviews /// /// Your own review, private notes included; every other reader's review with stars or @@ -46,9 +47,13 @@ pub(super) async fn get_book_reviews( /// Change your review of a book /// /// Creates the review on first write. Only the fields you send change: leave a field out -/// to keep it, send it as null to clear it. Stars, text and notes are independent, so +/// to keep it. Send `stars`, `body_md` or `notes_md` as null to clear it; `spoiler` is +/// true or false, so leave it out to keep it. Stars, text and notes are independent, so /// clearing the stars keeps your text. The body is `null` when the change left the review /// with no stars, text or notes, which deletes it. +/// +/// Text and notes can embed images uploaded with `POST /api/journals/images`. An uploaded +/// image the change drops, and no journal entry or review still shows, is deleted. #[utoipa::path( patch, path = "/api/reviews/{uuid}", @@ -79,7 +84,8 @@ pub(super) async fn patch_review( /// Delete your review of a book /// -/// Removes your stars, text and private notes together. Succeeds whether or not you had +/// Removes your stars, text and private notes together. An uploaded image they showed, +/// and no journal entry or review still shows, is deleted. Succeeds whether or not you had /// reviewed the book. #[utoipa::path( delete, diff --git a/server/src/backend/reviews/tests.rs b/server/src/backend/reviews/tests.rs index e676393e2..f5d30b560 100644 --- a/server/src/backend/reviews/tests.rs +++ b/server/src/backend/reviews/tests.rs @@ -3,15 +3,8 @@ use axum::http::StatusCode; use omnibus_shared::{BookReviews, Review}; use tower::ServiceExt; -use crate::auth::test_support as auth_test_support; use crate::backend::test_support::*; -async fn reader(pool: &sqlx::SqlitePool, name: &str) -> (i64, String) { - let user = auth_test_support::create_user(pool, name).await; - let token = auth_test_support::bearer_token(pool, user.id).await; - (user.id, token) -} - fn patch( uuid: &str, token: &str, @@ -40,6 +33,49 @@ async fn api_patch_review_requires_auth() { assert_eq!(res.status(), StatusCode::UNAUTHORIZED); } +#[tokio::test] +async fn api_get_book_reviews_requires_auth() { + let (app, _state, pool) = fixture().await; + let (_, uuid) = seed_book_with_uuid(&pool, "/lib", "Book A").await; + + let res = app + .oneshot(request("GET", &format!("/api/reviews/{uuid}"), None, None)) + .await + .unwrap(); + + assert_eq!(res.status(), StatusCode::UNAUTHORIZED); +} + +#[tokio::test] +async fn api_delete_review_requires_auth_and_leaves_the_review_intact() { + let (app, _state, pool) = fixture().await; + let (_, uuid) = seed_book_with_uuid(&pool, "/lib", "Book A").await; + let (_, token) = reader(&pool, "alice").await; + app.clone() + .oneshot(patch(&uuid, &token, serde_json::json!({ "stars": 4 }))) + .await + .unwrap(); + + let res = app + .clone() + .oneshot(request( + "DELETE", + &format!("/api/reviews/{uuid}"), + None, + None, + )) + .await + .unwrap(); + + assert_eq!(res.status(), StatusCode::UNAUTHORIZED); + let res = app + .oneshot(get_with_bearer(&format!("/api/reviews/{uuid}"), &token)) + .await + .unwrap(); + let reviews: BookReviews = body_json(res).await; + assert_eq!(reviews.mine.unwrap().stars, Some(4.0)); +} + #[tokio::test] async fn api_patch_review_creates_then_changes_only_the_fields_sent() { let (app, _state, pool) = fixture().await; diff --git a/server/src/backend/test_support.rs b/server/src/backend/test_support.rs index 0eebb309d..a4cf0b8af 100644 --- a/server/src/backend/test_support.rs +++ b/server/src/backend/test_support.rs @@ -41,6 +41,13 @@ pub(crate) async fn reader_token(pool: &sqlx::SqlitePool) -> String { auth_test_support::bearer_token(pool, reader.id).await } +/// Create a non-admin user named `name` and return its id and a bearer token for it. +pub(crate) async fn reader(pool: &sqlx::SqlitePool, name: &str) -> (i64, String) { + let user = auth_test_support::create_user(pool, name).await; + let token = auth_test_support::bearer_token(pool, user.id).await; + (user.id, token) +} + /// Whether the worker holds a task `wanted` accepts, queued, running or just finished. pub(crate) fn worker_holds_task( state: &AppState, diff --git a/shared/src/reviews.rs b/shared/src/reviews.rs index d66fa4513..c9d762ad0 100644 --- a/shared/src/reviews.rs +++ b/shared/src/reviews.rs @@ -150,16 +150,17 @@ pub struct ReviewSummary { pub struct BookReviews { /// Your own review. Null when you haven't reviewed the book. pub mine: Option, - /// Other readers' reviews: those with text first, newest first, then ratings with no - /// text, most recently rated first. A review holding only private notes isn't listed. + /// Other readers' reviews: those with text first, newest first by when the text was + /// first written, then ratings with no text, most recently rated first. A review + /// holding only private notes isn't listed. pub others: Vec, /// The book's ratings summed up. Null until at least two readers have rated it. pub summary: Option, } /// The changes `PATCH /api/reviews/{uuid}` makes to your review of a book. A field left -/// out stays as it is; a field sent as null is cleared. Blank text counts as null. A -/// review left with no stars, text or notes is deleted. +/// out stays as it is; `stars`, `body_md` or `notes_md` sent as null is cleared. Blank +/// text counts as null. A review left with no stars, text or notes is deleted. #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] pub struct ReviewPatch { @@ -188,8 +189,9 @@ pub struct ReviewPatch { )] #[cfg_attr(feature = "openapi", schema(value_type = Option))] pub notes_md: Option>, - /// Whether the review contains spoilers. + /// Whether the review contains spoilers: true or false; leave it out to keep it. #[serde(default, skip_serializing_if = "Option::is_none")] + #[cfg_attr(feature = "openapi", schema(value_type = bool))] pub spoiler: Option, } From 055ffd4255f6f31a0fe16d676e03b5f3e17a373c Mon Sep 17 00:00:00 2001 From: roberte777 Date: Tue, 20 Oct 2026 17:38:21 -0500 Subject: [PATCH 3/3] fix: document the null spoiler reading and the shared journal image cleanup - a null spoiler counts as leaving it out and keeps the flag; pinned in shared and REST tests - journal edit and delete docs keep an image any journal entry or review still shows --- openapi/openapi.json | 8 ++++---- server/src/backend/journals.rs | 6 +++--- server/src/backend/reviews.rs | 9 +++++---- server/src/backend/reviews/tests.rs | 24 ++++++++++++++++++++++++ shared/src/reviews.rs | 3 ++- shared/src/reviews/tests.rs | 7 +++++++ 6 files changed, 45 insertions(+), 12 deletions(-) diff --git a/openapi/openapi.json b/openapi/openapi.json index cb9c7be61..7826ec6ea 100644 --- a/openapi/openapi.json +++ b/openapi/openapi.json @@ -4854,7 +4854,7 @@ "Journals" ], "summary": "Delete a journal entry", - "description": "Deletes one of your entries, and any uploaded image it showed that no other entry\nshows.", + "description": "Deletes one of your entries. An uploaded image it showed, and no other journal entry or\nreview still shows, is deleted.", "operationId": "delete_journal", "parameters": [ { @@ -4881,7 +4881,7 @@ "Journals" ], "summary": "Edit a journal entry", - "description": "Replaces the body and progress of one of your entries. Send `status` to publish a draft\nor turn an entry back into one; leave it out to keep the status. An uploaded image the\nold body showed and no entry shows any more is deleted.", + "description": "Replaces the body and progress of one of your entries. Send `status` to publish a draft\nor turn an entry back into one; leave it out to keep the status. An uploaded image the\nchange drops, and no journal entry or review still shows, is deleted.", "operationId": "patch_journal", "parameters": [ { @@ -5951,7 +5951,7 @@ "Reviews" ], "summary": "Change your review of a book", - "description": "Creates the review on first write. Only the fields you send change: leave a field out\nto keep it. Send `stars`, `body_md` or `notes_md` as null to clear it; `spoiler` is\ntrue or false, so leave it out to keep it. Stars, text and notes are independent, so\nclearing the stars keeps your text. The body is `null` when the change left the review\nwith no stars, text or notes, which deletes it.\n\nText and notes can embed images uploaded with `POST /api/journals/images`. An uploaded\nimage the change drops, and no journal entry or review still shows, is deleted.", + "description": "Creates the review on first write. Only the fields you send change: leave a field out\nto keep it. Send `stars`, `body_md` or `notes_md` as null to clear it. `spoiler` takes\ntrue or false; a null counts as leaving it out, so it keeps the flag rather than\nclearing it. Stars, text and notes are independent, so clearing the stars keeps your\ntext. The body is `null` when the change left the review with no stars, text or notes,\nwhich deletes it.\n\nText and notes can embed images uploaded with `POST /api/journals/images`. An uploaded\nimage the change drops, and no journal entry or review still shows, is deleted.", "operationId": "patch_review", "parameters": [ { @@ -14511,7 +14511,7 @@ }, "spoiler": { "type": "boolean", - "description": "Whether the review contains spoilers: true or false; leave it out to keep it." + "description": "Whether the review contains spoilers: true or false. Leave it out to keep it; a null\ncounts as leaving it out and doesn't clear the flag." }, "stars": { "type": [ diff --git a/server/src/backend/journals.rs b/server/src/backend/journals.rs index ec122d372..f114baf88 100644 --- a/server/src/backend/journals.rs +++ b/server/src/backend/journals.rs @@ -101,7 +101,7 @@ async fn resolve_id(state: &AppState, user_id: i64, raw: &str) -> ResultGreat")); } +#[tokio::test] +async fn api_patch_review_keeps_the_spoiler_flag_when_it_is_sent_as_null() { + let (app, _state, pool) = fixture().await; + let (_, uuid) = seed_book_with_uuid(&pool, "/lib", "Book A").await; + let (_, token) = reader(&pool, "alice").await; + app.clone() + .oneshot(patch( + &uuid, + &token, + serde_json::json!({ "body_md": "Twist", "spoiler": true }), + )) + .await + .unwrap(); + + let res = app + .oneshot(patch(&uuid, &token, serde_json::json!({ "spoiler": null }))) + .await + .unwrap(); + + assert_eq!(res.status(), StatusCode::OK); + let review: Option = body_json(res).await; + assert!(review.unwrap().spoiler); +} + #[tokio::test] async fn api_patch_review_returns_null_when_the_patch_empties_the_review() { let (app, _state, pool) = fixture().await; diff --git a/shared/src/reviews.rs b/shared/src/reviews.rs index c9d762ad0..ac3b77275 100644 --- a/shared/src/reviews.rs +++ b/shared/src/reviews.rs @@ -189,7 +189,8 @@ pub struct ReviewPatch { )] #[cfg_attr(feature = "openapi", schema(value_type = Option))] pub notes_md: Option>, - /// Whether the review contains spoilers: true or false; leave it out to keep it. + /// Whether the review contains spoilers: true or false. Leave it out to keep it; a null + /// counts as leaving it out and doesn't clear the flag. #[serde(default, skip_serializing_if = "Option::is_none")] #[cfg_attr(feature = "openapi", schema(value_type = bool))] pub spoiler: Option, diff --git a/shared/src/reviews/tests.rs b/shared/src/reviews/tests.rs index 970949454..f2d0ffdc7 100644 --- a/shared/src/reviews/tests.rs +++ b/shared/src/reviews/tests.rs @@ -12,6 +12,13 @@ fn review_patch_reads_an_absent_field_as_unchanged_and_null_as_cleared() { assert_eq!(patch.spoiler, None); } +#[test] +fn review_patch_reads_a_null_spoiler_as_left_out() { + let patch: ReviewPatch = serde_json::from_str(r#"{"spoiler": null}"#).unwrap(); + assert_eq!(patch.spoiler, None); + assert_eq!(patch, ReviewPatch::default()); +} + #[test] fn review_patch_serializes_back_to_the_same_absent_and_null_fields() { let patch = ReviewPatch {