Skip to content

refactor: use inArray for enrollment count query#14

Open
okeolaolatun23-glitch wants to merge 2 commits into
ChainLearnOfficial:mainfrom
okeolaolatun23-glitch:list-Coures-only-counts-enrollments-for-first-course-on-page
Open

refactor: use inArray for enrollment count query#14
okeolaolatun23-glitch wants to merge 2 commits into
ChainLearnOfficial:mainfrom
okeolaolatun23-glitch:list-Coures-only-counts-enrollments-for-first-course-on-page

Conversation

@okeolaolatun23-glitch

Copy link
Copy Markdown

Closes #1

PR Documentation

Title

Fix course list enrollment counts across paginated results

Summary

listCourses in src/modules/courses/course.service.ts was only counting enrollments for the first course on the page. As a result, the course catalog showed the correct enrolledCount for the first item and 0 for every other course in the same response.

Change Made

  • Added inArray from drizzle-orm.
  • Replaced the enrollment count filter from eq(enrollments.courseId, courseIds[0]) to inArray(enrollments.courseId, courseIds).
  • Kept the existing grouping logic so counts are still aggregated per course.

Impact

  • Each course on the page now receives its own enrollment count.
  • The course catalog enrolledCount value is now accurate for all listed courses, not just the first one.

Verification

Manual verification steps:

  1. Create 3 courses in the database.
  2. Enroll 5 users in course 1, 3 users in course 2, and 0 users in course 3.
  3. Call GET /api/courses?page=1&limit=10.
  4. Confirm the response contains:
    • course 1: enrolledCount: 5
    • course 2: enrolledCount: 3
    • course 3: enrolledCount: 0

@DeFiVC DeFiVC 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.

Thanks for the contribution! The code change itself is correct and addresses issue #1 perfectly — using inArray instead of eq with only the first course ID is exactly what's needed.

However, this PR currently cannot be merged due to two issues:

1. Merge Conflicts

The branch has conflicts with src/modules/courses/course.service.ts. Main has been updated with PR #17 which added withLock imports and other changes. You'll need to rebase on the latest main:

git fetch origin
git rebase origin/main
# resolve conflicts in src/modules/courses/course.service.ts
git add .
git rebase --continue

2. CI Failure

The "Lint & Typecheck" job failed because package-lock.json is missing from your branch. It was added to main after your branch was created. The rebase above will fix this.

Please rebase on latest main, resolve any conflicts, and push — I'll re-review once CI passes. 👍

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.

Bug: listCourses only counts enrollments for first course on page

2 participants