Skip to content

feat: add the /api/reviews routes, review server functions and MCP book_reviews tool - #2843

Merged
roberte777 merged 5 commits into
OMNI-2841/reviews-db-1from
OMNI-2841/reviews-api-2
Oct 21, 2026
Merged

roberte777 merged 5 commits into
OMNI-2841/reviews-db-1from
OMNI-2841/reviews-api-2

Conversation

@roberte777

Copy link
Copy Markdown
Collaborator

Summary

  • GET/PATCH/DELETE /api/reviews/{uuid}; PATCH changes only fields sent
  • /api/rpc/reviews/{get,update,delete} server functions for the Dioxus UI
  • /api/ratings routes marked deprecated in the spec; still served unchanged
  • MCP book_reviews tool; book_read_status stops claiming want-to-read

Test plan

  • cargo nextest run -p omnibus (new backend::reviews tests)
  • cargo nextest run -p omnibus-mcp
  • just openapi-gen leaves openapi/openapi.json unchanged
  • just lint

Version

By default a merge to main cuts a patch release. Pick the version update this PR should trigger; labels override the default:

  • Minor — add the minor version label (vX.Y.Z → vX.(Y+1).0).
  • Patch — the default; add the patch version label or leave unlabeled (vX.Y.Z → vX.Y.(Z+1)).
  • No release — add the no release label to skip cutting a release.

If both minor version and patch version are applied, minor version wins. Unlabeled PRs that only touch docs (*.md, docs/) or CI (.github/) skip the release automatically.

Notes

Copilot AI balanced review requested due to automatic review settings October 20, 2026 19:21
@roberte777 roberte777 self-assigned this Oct 20, 2026
@roberte777
roberte777 added this pull request to stack #2844 October 20, 2026 19:22
@codecov

codecov Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.31959% with 22 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
frontend/src/rpc/reviews.rs 0.00% 22 Missing ⚠️

📢 Thoughts on this report? Let us know!

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

cargo-audit

No RustSec advisories found in Cargo.lock ✅

Posted by the Rust workflow's security job.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Authentication-failure coverage is incomplete, and the published null-clearing contract misstates spoiler handling.

2 open issues
What changed in this PR

Adds the API layer for book reviews, building on the database work while preserving legacy ratings routes.

Changes:

  • Adds review REST endpoints and Dioxus server functions.
  • Adds the read-only MCP book_reviews tool.
  • Updates tests, OpenAPI, and privacy documentation.
File Description
server/​src/​backend/​routes.rs Registers review routes.
server/​src/​backend/​reviews/​tests.rs Tests review endpoints and privacy.
server/​src/​backend/​reviews.rs Implements review REST handlers.
server/​src/​backend/​ratings.rs Deprecates legacy ratings endpoints.
server/​src/​backend/​openapi.rs Adds the Reviews API tag.
server/​src/​backend.rs Registers the reviews module.
openapi/​openapi.json Publishes review schemas and endpoints.
mcp/​src/​tools/​read/​views.rs Adds review response views.
mcp/​src/​tools/​read/​tests.rs Tests review response conversion.
mcp/​src/​tools/​read.rs Adds book_reviews; corrects read-status descriptions.
mcp/​src/​server/​tests/​read.rs Checks review tool registration.
mcp/​src/​server/​tests/​checkin.rs Updates expected tool count.
frontend/​src/​rpc/​reviews.rs Adds review server functions.
frontend/​src/​rpc/​mod.rs Exports review server functions.
docs/​architecture.md Documents private-note isolation.

Review effort: Balanced

Comment thread server/src/backend/reviews.rs Outdated
Comment thread server/src/backend/reviews/tests.rs
…h 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
…leanup

- 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
@roberte777
roberte777 merged commit 22e09f9 into main Oct 21, 2026
48 checks passed
@roberte777
roberte777 deleted the OMNI-2841/reviews-api-2 branch October 21, 2026 01:12
roberte777 added a commit that referenced this pull request Oct 21, 2026
## Summary
- Reviews stop after Stats: your card, household summary, other readers
- Your card: one-tap half stars, a pencil to edit and a trash can to
delete
- Spoiler flag lives in the editor; flagged text stays folded until
Finished
- Title area links to Reviews once two or more readers have rated the
book

## Test plan
- [x] `cargo nextest run -p omnibus-frontend --features server`
- [x] `just e2e-ci tests/flows/reviews.spec.ts`
- [x] `just lint`

## Version
By default a merge to `main` cuts a patch release. Pick the version
update this PR should trigger; labels override the default:
- [x] **Minor** — add the `minor version` label (`vX.Y.Z` →
`vX.(Y+1).0`).
- [ ] **Patch** — the default; add the `patch version` label or leave
unlabeled (`vX.Y.Z` → `vX.Y.(Z+1)`).
- [ ] **No release** — add the `no release` label to skip cutting a
release.

If both `minor version` and `patch version` are applied, `minor version`
wins. Unlabeled PRs that only touch docs (`*.md`, `docs/`) or CI
(`.github/`) skip the release automatically.

## Notes
- Part of #2841: stacked on #2843 (db → API → web → mobile → React →
iOS)
- Book-delete confirmation now counts reviews; the wire field stays
`ratings`
- The journal's Markdown field and highlights inserter are shared with
reviews
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants