feat(reviews): implement persisted aggregates, verified-purchase gate… - #86
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR centralizes course and book review CRUD, adds verified-purchase authorization and persisted rating aggregates, exposes paginated review routes and rating search filters, adds a backfill migration, and updates integration tests. ChangesReviews and ratings
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Reviewer
participant ReviewRoutes
participant ReviewController
participant ReviewAuth
participant ReviewStats
participant CourseBook
Reviewer->>ReviewRoutes: submit or manage review
ReviewRoutes->>ReviewController: dispatch request
ReviewController->>ReviewAuth: verify purchase or enrollment
ReviewController->>ReviewStats: recompute aggregate statistics
ReviewStats->>CourseBook: save rating metadata
ReviewController-->>Reviewer: return review data and summary
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/controllers/reviewController.js`:
- Around line 81-82: The helpful branch in the review sorting logic incorrectly
orders reviews by rating and date. Remove the unsupported "helpful" sort option,
or update the branch to sort by an existing helpful-vote field; do not label
rating-based ordering as helpful.
- Around line 16-18: Require ratings to be integers on every write path by
updating the validation in the review creation flow and the corresponding update
flow in reviewController.js (anchor lines 16-18; sibling lines 141-145). Reject
fractional, non-numeric, and out-of-range values with the existing 400 APIError
behavior, while preserving valid integer ratings from 1 through 5.
- Around line 52-59: Update the review endpoint responses in
src/controllers/reviewController.js at lines 52-59, 92-106, 151-158, and 196-202
so each returns the standard { success, message, data } envelope: nest creation,
update, and deletion results under data, and add message while nesting listing
results under data. Preserve the existing result fields within each data object.
- Around line 68-90: Update the review retrieval flow around the
Model.findById(...).populate call so pagination, sorting, and reviewer
population occur in the database before materializing the response page. Use an
aggregation pipeline or indexed review collection that filters by the item,
applies the requested sort, skips startIndex, limits to limitNum, and preserves
the existing total count and response behavior.
In `@src/models/Book.js`:
- Around line 22-39: Add a one-time migration for both src/models/Book.js (lines
22-39) and src/models/Course.js (lines 28-45) to backfill rating, numReviews,
and ratingBreakdown from each existing document’s reviews; ensure the migration
updates only records needing these aggregates and does not leave legacy records
at default zero values.
In `@src/routes/books/bookRoutes.js`:
- Around line 98-132: Update the review mutation routes using updateBookReview
and deleteBookReview in src/routes/books/bookRoutes.js lines 98-132 to
invalidate both BOOK and BOOKS cache patterns. Apply the same change to the
corresponding review mutation routes in src/routes/courses/courseRoutes.js lines
84-118, preserving the existing middleware behavior.
In `@src/services/search/searchService.js`:
- Around line 101-104: Validate minRating before constructing search tasks in
the search service: reject non-finite values and any rating outside the
inclusive 0–5 range with a 400 response, and only enable the rating filter for
valid input. Preserve the existing Course and Book search behavior for requests
without minRating.
In `@test/jest.setup.js`:
- Around line 18-20: Remove literal fallbacks from test/jest.setup.js by
validating that MONGO_URI, JWT_SECRET, and PORT are present in process.env and
failing fast when any is missing; do not assign defaults. In
test/reviews.test.js, use the already validated process.env.JWT_SECRET directly
instead of introducing another hardcoded or fallback value.
In `@test/reviews.test.js`:
- Around line 153-176: The success-path review tests around the enrolled,
purchaser, and author requests, plus the other noted review endpoint cases, must
enforce the standard response envelope. Add assertions for res.body.message and
read review payload fields through res.body.data rather than top-level res.body
properties; update the create handler’s response shape to return { success,
message, data } consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2dc9d00f-50a4-4de9-8e89-004c12b6392f
📒 Files selected for processing (12)
src/controllers/books/bookController.jssrc/controllers/courses/courseController.jssrc/controllers/reviewController.jssrc/models/Book.jssrc/models/Course.jssrc/routes/books/bookRoutes.jssrc/routes/courses/courseRoutes.jssrc/services/search/searchService.jssrc/utils/reviewAuth.jssrc/utils/reviewStats.jstest/jest.setup.jstest/reviews.test.js
| rating: { | ||
| type: Number, | ||
| default: 0, | ||
| min: 0, | ||
| max: 5, | ||
| }, | ||
| numReviews: { | ||
| type: Number, | ||
| default: 0, | ||
| min: 0, | ||
| }, | ||
| ratingBreakdown: { | ||
| 1: { type: Number, default: 0 }, | ||
| 2: { type: Number, default: 0 }, | ||
| 3: { type: Number, default: 0 }, | ||
| 4: { type: Number, default: 0 }, | ||
| 5: { type: Number, default: 0 }, | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files 'src/models/Book.js' 'src/models/Course.js' 'src/controllers/reviewController.js' 'src/**/*.js' | sed 's#^`#-` #'
printf '\n== Book.js ==\n'
wc -l src/models/Book.js
cat -n src/models/Book.js
printf '\n== Course.js ==\n'
wc -l src/models/Course.js
cat -n src/models/Course.js
printf '\n== reviewController.js (relevant sections) ==\n'
wc -l src/controllers/reviewController.js
sed -n '1,260p' src/controllers/reviewController.js
printf '\n== Search for ratingBreakdown / reviews usage ==\n'
rg -n "ratingBreakdown|numReviews|reviews" src/models src/controllers src/routes src -g '!**/node_modules/**'Repository: Deen-Bridge/dnb-backend
Length of output: 23847
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== reviewStats.js ==\n'
wc -l src/utils/reviewStats.js
cat -n src/utils/reviewStats.js
printf '\n== Search for any backfill/migration for reviews stats ==\n'
rg -n "backfill|migration|recomputeAndSaveStats|computeReviewStats|numReviews =|ratingBreakdown =" src scripts
printf '\n== Search for aggregate fields in query surfaces ==\n'
rg -n "ratingBreakdown|numReviews|rating" src/controllers src/services src/routes src/models | sed -n '1,220p'Repository: Deen-Bridge/dnb-backend
Length of output: 2941
Backfill the new review aggregates for existing books/courses. src/models/Book.js and src/models/Course.js add rating, numReviews, and ratingBreakdown, but those defaults won’t populate older documents that already have reviews. Add a one-time migration so existing records don’t stay stuck at zero.
📍 Affects 2 files
src/models/Book.js#L22-L39(this comment)src/models/Course.js#L28-L45
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/models/Book.js` around lines 22 - 39, Add a one-time migration for both
src/models/Book.js (lines 22-39) and src/models/Course.js (lines 28-45) to
backfill rating, numReviews, and ratingBreakdown from each existing document’s
reviews; ensure the migration updates only records needing these aggregates and
does not leave legacy records at default zero values.
Source: Path instructions
| it("allows course creator / enrolled user / purchaser to review", async () => { | ||
| const resEnrolled = await request(app) | ||
| .post(`/api/courses/${course._id}/reviews`) | ||
| .set("Authorization", `Bearer ${enrolledToken}`) | ||
| .send({ rating: 5, comment: "Great content!" }); | ||
|
|
||
| expect(resEnrolled.statusCode).toBe(201); | ||
| expect(resEnrolled.body.success).toBe(true); | ||
|
|
||
| const resPurchaser = await request(app) | ||
| .post(`/api/courses/${course._id}/reviews`) | ||
| .set("Authorization", `Bearer ${purchaserToken}`) | ||
| .send({ rating: 4, comment: "Very clear instructions." }); | ||
|
|
||
| expect(resPurchaser.statusCode).toBe(201); | ||
| expect(resPurchaser.body.success).toBe(true); | ||
|
|
||
| const resAuthor = await request(app) | ||
| .post(`/api/courses/${course._id}/reviews`) | ||
| .set("Authorization", `Bearer ${authorToken}`) | ||
| .send({ rating: 5, comment: "I made this course!" }); | ||
|
|
||
| expect(resAuthor.statusCode).toBe(201); | ||
| expect(resAuthor.body.success).toBe(true); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Enforce the standard response envelope in these success-path tests.
Assert message and access endpoint payloads through res.body.data. The supplied create handler currently returns reviews, rating, and aggregates at the top level, so these tests allow the API contract to remain inconsistent.
As per path instructions, changed endpoints require “consistent response shapes ({ success, message, data }).”
Also applies to: 252-263, 293-303, 313-327, 348-383
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/reviews.test.js` around lines 153 - 176, The success-path review tests
around the enrolled, purchaser, and author requests, plus the other noted review
endpoint cases, must enforce the standard response envelope. Add assertions for
res.body.message and read review payload fields through res.body.data rather
than top-level res.body properties; update the create handler’s response shape
to return { success, message, data } consistently.
Source: Path instructions
… parallel test state leakage
…prevent parallel worker thread leaks on CI
|
@Killerjunior pls fix CI |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/controllers/reviewController.js (1)
12-64: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReturn review save conflicts as
409 Conflict.createReviewHandler,updateReviewHandler, anddeleteReviewHandlerall mutate the embeddedreviewsarray and then callrecomputeAndSaveStats(item). Concurrent requests can trip Mongoose’s version check, butVersionErrorcurrently falls through to the generic 500 handler instead of a retryable conflict response. Map that error to409around the shared save path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/controllers/reviewController.js` around lines 12 - 64, Update the shared save path used by createReviewHandler, updateReviewHandler, and deleteReviewHandler around recomputeAndSaveStats(item) to catch Mongoose VersionError instances and pass an APIError with status 409 to the request error flow. Preserve existing handling for all other errors so they continue reaching the generic 500 handler.
🧹 Nitpick comments (1)
src/controllers/reviewController.js (1)
66-141: 🧹 Nitpick | 🔵 TrivialPagination now happens in the pipeline — nice fix. Sort/skip/limit run in the aggregation before
$lookup, so only the current page's reviewers get populated. This resolves the prior concern about materializing the entire review collection per request.One thing worth keeping on the radar: reviews are still embedded in the parent
Book/Coursedocument, so a very popular item accumulating thousands of reviews could eventually approach MongoDB's 16MB document size limit, and this aggregation still has to$unwindthe full array before paging it. Not something to block this PR on, but worth a note for a future migration to a separate, indexedreviewscollection if popular items grow large.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/controllers/reviewController.js` around lines 66 - 141, No code change is required for this review comment; retain the current pagination implementation in getReviewsHandler and record the future consideration of moving embedded reviews to a separate, indexed collection if document size or array growth becomes problematic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/controllers/searchController.js`:
- Around line 19-22: Update the error response in the search controller,
including the APIError branch and query-length validation branch, to use the
standard { success, message, data } envelope: replace error with message and
include the appropriate data field while preserving each branch’s status and
message behavior.
In `@src/migrations/backfillReviewStats.js`:
- Line 16: Update the cursor query in the backfill migration to iterate
documents with empty review arrays as well, removing the "reviews.0" filter or
explicitly including stale/empty-review records. Keep statsDiffer responsible
for skipping documents whose aggregates already match computeReviewStats
results.
In `@src/services/search/searchService.js`:
- Around line 67-72: Make the minRating contract whole-star-only: update the
validation in the search service’s minRating handling to reject non-integer
values while retaining the existing 0–5 finite-number checks. In
test/search.test.js lines 86-91, keep the invalid fixture aligned with this
contract and add Jest coverage confirming a valid minRating filters course/book
results and includes rating and numReviews in the response.
---
Outside diff comments:
In `@src/controllers/reviewController.js`:
- Around line 12-64: Update the shared save path used by createReviewHandler,
updateReviewHandler, and deleteReviewHandler around recomputeAndSaveStats(item)
to catch Mongoose VersionError instances and pass an APIError with status 409 to
the request error flow. Preserve existing handling for all other errors so they
continue reaching the generic 500 handler.
---
Nitpick comments:
In `@src/controllers/reviewController.js`:
- Around line 66-141: No code change is required for this review comment; retain
the current pagination implementation in getReviewsHandler and record the future
consideration of moving embedded reviews to a separate, indexed collection if
document size or array growth becomes problematic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d26ff67d-8c44-4bd1-917e-8dbd4adb8b01
📒 Files selected for processing (14)
package.jsonsrc/controllers/reviewController.jssrc/controllers/searchController.jssrc/migrations/backfillReviewStats.jssrc/models/Book.jssrc/models/Course.jssrc/routes/books/bookRoutes.jssrc/routes/courses/courseRoutes.jssrc/services/search/searchService.jstest/bookUpload.test.jstest/jest.setup.jstest/reviews.test.jstest/search.test.jstest/upload.test.js
|
Changes have been implemented and CI checks are passing. Kindly review. |
Description
Resolves #63.
This PR transforms the half-built embedded reviews feature into a fully functional Reviews & Ratings API for both Course and Book models. It persists aggregate rating statistics, enforces verified-purchase authorization before review creation, exposes paginated & sortable review listings, and enables review updates and deletions (with automatic recalculation of aggregate metrics).
Proposed Changes
1. Schema & Indexes (
src/models/Course.js,src/models/Book.js)rating(Number, default 0, min 0, max 5) andnumReviews(Number, default 0) to both schemas.ratingBreakdownobject tracking star count distributions (1through5).{ rating: -1 }on both schemas to support "top rated" sorting.createBook,addBookReview, andaddCourseReview.2. Review Aggregates Helper (
src/utils/reviewStats.js)computeReviewStats(reviews): Calculates average rating rounded to 1 decimal place, totalnumReviews, and star breakdown. Resets rating and numReviews to0when all reviews are deleted.recomputeAndSaveStats(doc): Recomputes and saves stats on Mongoose document mutations (add/edit/delete).3. Verified-Purchase Gate (
src/utils/reviewAuth.js)verifyItemPurchase({ userId, item, itemType }): Ensures caller owns/has access to the item before submitting a review.createdBy), inenrolledUsers, or has entry inuser.purchasedCourses.author) or has entry inuser.purchasedBooks.403 Forbiddenif user has not purchased/enrolled in the item.400 Bad Request).4. Review CRUD Controller (
src/controllers/reviewController.js)POST /api/courses/:id/reviews&POST /api/books/:id/reviews: Create review (gated by verified purchase).GET /api/courses/:id/reviews&GET /api/books/:id/reviews: Paginated review listing withpage,limit, andsort(recent,rating,helpful). Populates only reviewernameandavatar. Includes aggregate summary (rating,numReviews,ratingBreakdown).PUT/PATCH /api/courses/:id/reviews&PUT/PATCH /api/books/:id/reviews: Update user's existing review (comment/rating) and recompute aggregates.DELETE /api/courses/:id/reviews&DELETE /api/books/:id/reviews: Delete review (caller's own review or admin deletion via review ID) and recompute aggregates.5. Search Service Surface (
src/services/search/searchService.js)ratingandnumReviewsin search projections for Courses and Books, allowing filtering byminRating.Verification Plan
Automated Tests
Run the comprehensive test suite: