Skip to content

Adjust /course/filter/values/{filter} endpoint to return names on top of just codes #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jzgom067 merged 2 commits into from
Nov 18, 2025
Merged

Adjust /course/filter/values/{filter} endpoint to return names on top of just codes #17

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
972f900
Update filter values endpoint to return mappings
ramonechen Nov 18, 2025
0298751
Return empty dict instead of None in filters endpoint
ramonechen Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions app/routers/course.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,14 +220,20 @@ def search_course(


@router.get("/filter/values/{filter}")
def get_filter_values(session: SessionDep, filter: CourseFilter) -> list[str]:
column = None
def get_filter_values(session: SessionDep, filter: CourseFilter) -> dict[str, str]:
code_col, title_col = None, None
if filter is CourseFilter.subjects:
column = Subject.subj_code
code_col = Subject.subj_code
title_col = Subject.title
elif filter is CourseFilter.attributes:
column = Attribute.attr_code
code_col = Attribute.attr_code
title_col = Attribute.title
elif filter is CourseFilter.semesters:
column = Course_Offering.semester
result_scalars = (
session.execute(select(Course_Offering.semester).distinct()).scalars().all()
)
return {sem: sem.capitalize() for sem in result_scalars}

Copilot AI Nov 18, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using sem.capitalize() on semester codes may not produce meaningful names. For example, if semester codes are formatted like "202401" or "spring2024", capitalize() would return "202401" or "Spring2024" respectively. Consider implementing proper semester name mapping logic or documenting the expected semester code format. Example:

# If semesters are like "spring2024", "fall2024"
return {sem: sem.replace(sem[:sem.index(next(filter(str.isdigit, sem)))], sem[:sem.index(next(filter(str.isdigit, sem)))].capitalize()) + " " + sem[sem.index(next(filter(str.isdigit, sem))):] for sem in result_scalars}

# Or simpler, if you want proper formatting:
def format_semester(sem: str) -> str:
    # Implement based on actual semester format
    return sem.capitalize()  # placeholder

return {sem: format_semester(sem) for sem in result_scalars}

Copilot uses AI. Check for mistakes.
else:
return None
return session.execute(select(column).distinct()).scalars().all()
return {}
result_mappings = session.execute(select(code_col, title_col)).mappings().all()

Copilot AI Nov 18, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The query is missing .distinct() which was present in the original implementation. Without it, if there are duplicate rows in the Subject or Attribute tables (or due to joins), the resulting dictionary could have duplicate keys with overwritten values. Add .distinct() to the select statement:

result_mappings = session.execute(select(code_col, title_col).distinct()).mappings().all()
Suggested change
result_mappings = session.execute(select(code_col, title_col)).mappings().all()
result_mappings = session.execute(select(code_col, title_col).distinct()).mappings().all()

Copilot uses AI. Check for mistakes.
return {row[code_col]: row[title_col] for row in result_mappings}

Copilot AI Nov 18, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using SQLAlchemy column objects (code_col, title_col) as dictionary keys in row[code_col] may not work as expected with .mappings(). The mapping keys are typically the column names as strings, not the column objects themselves. Consider using the column names as strings or accessing the row values by index:

# Option 1: Access by column name strings
result_mappings = session.execute(select(code_col, title_col)).mappings().all()
code_key = code_col.key
title_key = title_col.key
return {row[code_key]: row[title_key] for row in result_mappings}

# Option 2: Use tuples and access by index
result_tuples = session.execute(select(code_col, title_col)).all()
return {row[0]: row[1] for row in result_tuples}
Suggested change
return {row[code_col]: row[title_col] for row in result_mappings}
code_key = code_col.key
title_key = title_col.key
return {row[code_key]: row[title_key] for row in result_mappings}

Copilot uses AI. Check for mistakes.
Loading