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/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..9d494e657 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,18 @@ 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 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, +} + /// `book_journal_entries`' answer, newest first. #[derive(Debug, Serialize, JsonSchema)] pub struct JournalEntryList { @@ -603,7 +616,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 +655,23 @@ 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: 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(), + 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..cd13b4d6a 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, ReviewBucket, ReviewSummary, SessionFormat, + SessionLogEntry, SessionLogPage, StatsSummary, }; use super::views::iso; @@ -256,6 +257,57 @@ async fn copies(Path(_): Path) -> AxumJson> { }]) } +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), + 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: 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. async fn stub_service() -> OmnibusMcp { let app = Router::new() @@ -266,6 +318,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 +756,44 @@ fn the_two_format_vocabularies_are_documented_wherever_they_disagree() { ); } } + +// MARK: - Reviews + +#[tokio::test] +async fn book_reviews_stamps_each_review_and_passes_the_summary_through() { + 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"); + 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/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..7826ec6ea 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": [ { @@ -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": [ { @@ -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": [ { @@ -5705,7 +5706,7 @@ ], "responses": { "200": { - "description": "Other users' ratings, newest first.", + "description": "Other users' ratings, most recently rated first.", "content": { "application/json": { "schema": { @@ -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. 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": [ + { + "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 `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": [ + { + "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. Adding text to a review that\nhad none restarts it, so a text review dates from its text." + }, + "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 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": [ + { + "$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,179 @@ } } }, + "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. Adding text to a review that\nhad none restarts it, so a text review dates from its text. 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; `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": [ + "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", + "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": [ + "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 +16926,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/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..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) -> Result, @@ -49,7 +51,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 +60,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,15 +74,17 @@ 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}", tag = "Ratings", params(("uuid" = String, Path, description = "The book's uuid.")), - responses((status = 200, description = "Other users' ratings, newest first.", body = Vec)), + 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( user: AuthUser, State(state): State, @@ -92,7 +98,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 +107,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..1872fef70 --- /dev/null +++ b/server/src/backend/reviews.rs @@ -0,0 +1,110 @@ +//! 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 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 +/// 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 `stars`, `body_md` or `notes_md` as null to clear it. `spoiler` takes +/// true or false; a null counts as leaving it out, so it keeps the flag rather than +/// clearing 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}", + 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. 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, + 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..cd67ec9b3 --- /dev/null +++ b/server/src/backend/reviews/tests.rs @@ -0,0 +1,319 @@ +//! Tests for the book review REST handlers. +use axum::http::StatusCode; +use omnibus_shared::{BookReviews, Review}; +use tower::ServiceExt; + +use crate::backend::test_support::*; + +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_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; + 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_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; + 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)) } 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..ac3b77275 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,10 @@ 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; 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 {