Skip to content

[Performance]: The catalogue fires one rating request per card — 13 requests and 12 aggregations to open the home page #86

Description

@MOHITKOURAV01

Summary

Every course card in the catalogue mounts a CourseRatingBadge, and every badge fetches its own rating:

// frontend/src/components/reviews/CourseRatingBadge.jsx
useEffect(() => {
  if (!courseId) return undefined;
  axiosInstance.get(`/api/reviews/${courseId}/summary`).then(...)
}, [courseId]);

A catalogue page holds twelve cards (DEFAULT_LIMIT = 12), so opening the home page issues thirteen requests: one for the courses, twelve for ratings. Each of those twelve runs a separate aggregation:

// backend/controllers/courseReviewController.js
const [summary] = await CourseReview.aggregate([
  { $match: { courseId: objectId } },
  { $group: { _id: "$courseId", averageRating: { $avg: "$rating" }, ... } },
]);

The listing endpoint knows exactly which courses it just returned and could resolve all twelve in one grouped $match/$group, but nothing joins the two.

Expected result

  • Opening the catalogue costs a constant number of requests, not one per card.
  • Paging or searching does not multiply that number.
  • A course with no reviews does not cost a round trip to discover that.

Actual result

  • 13 requests per catalogue page, 12 of them fully independent aggregations over courseReviews.
  • Every keystroke in the search box that changes the result set re-mounts up to twelve badges and fires twelve more. With the 350 ms debounce, a typed word is a few hundred requests over a few seconds on a busy catalogue.
  • All twelve are anonymous GETs with no caching headers, so nothing at any layer coalesces them.
  • On a cold connection the ratings pop in one at a time and the cards reflow as "New" is replaced.
  • The same page is rendered for signed-out visitors on / and for signed-in students through UserHome, so the cost is paid by unauthenticated traffic too — this is the app's most exposed endpoint pattern.
  • Correction to an earlier draft of this issue: SavedCourses.jsx does not render the badge, so the saved-courses page is unaffected today. It fetches its rows in one request. Any fix should still be reusable there, since that page is the obvious next place to want ratings.

Steps to reproduce

  1. Seed more than twelve courses and leave a review on two of them.
  2. Open DevTools → Network, filter on summary, load the home page.
  3. Twelve GET /api/reviews/<id>/summary requests, including for the ten courses that have no reviews at all.
  4. Type react into the search box → another burst of up to twelve per settled query.
  5. Click through to page two → twelve more.

Notes

  • courseReviewSchema already indexes { courseId: 1, createdAt: -1 }, so a single $match: { courseId: { $in: [...] } } followed by $group: { _id: "$courseId", ... } is one indexed pass for the whole page.
  • Two shapes would work: fold a ratings block into the getallcourses response, or add a batch endpoint the badge list can call once. The second keeps the listing controller's response shape stable for existing consumers and is reusable by SavedCourses, which fetches a different set of courses.
  • Whatever the shape, CourseRatingBadge should be able to take a summary it was handed and skip the request entirely, so a parent that already has the data does not pay again.
  • Course ids come from a page the server just produced, but a batch endpoint takes ids from the client, so the id list needs a length cap and per-id ObjectId validation.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

ECSoC26Required label for a PR to be eligible for Sentinel scoring

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions