Adjust /course/filter/values/{filter} endpoint to return names on top of just codes - #17
Conversation
The /filter/values/{filter} endpoint now returns a dictionary mapping codes to titles for subjects and attributes, and a capitalized dictionary for semesters. This improves the API response structure for frontend consumption.
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the /course/filter/values/{filter} endpoint to return dictionaries mapping codes to names instead of simple lists of codes. This enables the frontend to dynamically fetch both codes and their associated names for subjects, attributes, and semesters.
Key changes:
- Changed return type from
list[str]todict[str, str] - Modified queries to select both code and title columns for subjects and attributes
- Added special handling for semesters to map codes to capitalized values
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return None | ||
| return session.execute(select(column).distinct()).scalars().all() | ||
| return {} | ||
| result_mappings = session.execute(select(code_col, title_col)).mappings().all() |
There was a problem hiding this comment.
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()| result_mappings = session.execute(select(code_col, title_col)).mappings().all() | |
| result_mappings = session.execute(select(code_col, title_col).distinct()).mappings().all() |
| result_scalars = ( | ||
| session.execute(select(Course_Offering.semester).distinct()).scalars().all() | ||
| ) | ||
| return {sem: sem.capitalize() for sem in result_scalars} |
There was a problem hiding this comment.
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}| return session.execute(select(column).distinct()).scalars().all() | ||
| return {} | ||
| result_mappings = session.execute(select(code_col, title_col)).mappings().all() | ||
| return {row[code_col]: row[title_col] for row in result_mappings} |
There was a problem hiding this comment.
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}| 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} |
What?
Adjusts the
/course/filter/values/{filter}endpoint to return mappings of codes to names instead of a list of just codes.Closes #12.
Why?
There is a part of the course planner website that hardcodes subject codes and names. This data can very easily change on a semester-to-semester basis and can easily grow outdated if it isn't dynamically fetched from an external source. In addition, including names to attribute and semester codes allows more flexibility on the frontend side should they ever want to be able to map those codes to names in the future.
How?
Jack and I modified the select statements
/course/filter/values/{filter}endpoint function in thecourse.pyrouter.Testing?
It works with Postman.
Anything Else?
Have a nice day.