Fix enrollment count query to include all courses on page, not just the first#53
Merged
Merged
Conversation
DeFiVC
approved these changes
Jun 24, 2026
DeFiVC
left a comment
Contributor
There was a problem hiding this comment.
PR Review: Fix enrollment count query to include all courses on page
Author: Levi-Ojukwu (Ojukwu_Levi) | Closes: #26 | State: OPEN | Mergeable: MERGEABLE
Review Checklist
| Item | Status |
|---|---|
| Git Identity | ✅ DeFiVC defivc1@email.com |
| Linked Issue | ✅ Closes #26 — matches requirements exactly |
| Code Quality | ✅ Correct and minimal fix |
| CI Status | ✅ Lint & Typecheck: SUCCESS, Test: SUCCESS |
| Merge Conflicts | ✅ None |
Code Analysis
src/modules/courses/course.service.ts (2 additions, 2 deletions):
- Line 1: Added
inArrayto thedrizzle-ormimport — correct - Line 50: Changed
eq(enrollments.courseId, courseIds[0])toinArray(enrollments.courseId, courseIds)— correct
This is a clean, minimal fix that exactly addresses the issue. The original code was filtering enrollments using only the first course ID in the array, causing all other courses to show an enrollment count of 0. The fix uses inArray to query for all course IDs on the current page in a single query.
Decision: APPROVE
The PR is a correct, minimal bug fix that exactly matches the issue requirements. CI is green, no merge conflicts. This is ready to merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix enrolledCount in course listings returning 0 for every course except the first one on the page.
Problem
In CourseService.listCourses, the enrollment count query used eq(enrollments.courseId, courseIds[0]) which only filtered for the first course ID. All other courses on the page received an enrollment count of 0.
Fix
Replaced eq(enrollments.courseId, courseIds[0]) with inArray(enrollments.courseId, courseIds) so the query fetches enrollment counts for all courses on the current page in a single query.
File changed: src/modules/courses/course.service.ts — added inArray import and updated the .where() clause.
Closes #26