fix: duplicate courses shown to moderators in created courses list - #2544
Sandeepambala wants to merge 1 commit into
Conversation
The moderator fallback in get_created_courses() runs the base query without the instructor filter. Since the query joins Course Instructor (a child table) with LMS Course, a course with N instructors returns N identical rows, and with limit(3) a single course can fill every slot on the Home page's Created Courses section, hiding other courses. Add .distinct() to the base query so each course appears once. Fixes: frappe#2476 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Confidence Score: 4/5The changed created-courses query can fail on SQL backends that require ordered columns to be selected with
lms/lms/api.py Reviews (1): Last reviewed commit: "fix: duplicate courses shown to moderato..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| .join(Course) | ||
| .on(CourseInstructor.parent == Course.name) | ||
| .select(Course.name) | ||
| .distinct() |
There was a problem hiding this comment.
When get_created_courses() runs on PostgreSQL or a strict SQL backend, this can generate SELECT DISTINCT on only Course.name while ordering by Course.published_on. Those backends can reject that query, so the created-courses section can fail for instructors and for the moderator fallback instead of returning courses.
Context Used: Guidelines for reviewing Frappe Framework applicat... (source)
|
Could you please resolve greptile's comments? (comment under the issue if you feel like you disagree) |
|
Closing due to inactivity, please reopen with requested changes |
2 participants
The moderator fallback in
get_created_courses()runs the base query without the instructor filter. Since the query joinsCourse InstructorwithLMS Course, a course with multiple instructors returns duplicate rows. Withlimit(3), a single course can occupy all slots in the "Created Courses" section.Add
.distinct()to ensure each course is returned only once.Fixes #2476