Skip to content

Conversation

@ramonechen
Copy link
Contributor

@ramonechen ramonechen commented Nov 18, 2025

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 the course.py router.

Testing?

It works with Postman.

Anything Else?

Have a nice day.

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.
@jzgom067 jzgom067 merged commit b29712c into main-preview Nov 18, 2025
6 checks passed
@jzgom067 jzgom067 deleted the return-course-code-name-mappings branch November 18, 2025 22:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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] to dict[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()
Copy link

Copilot AI Nov 18, 2025

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.
result_scalars = (
session.execute(select(Course_Offering.semester).distinct()).scalars().all()
)
return {sem: sem.capitalize() for sem in result_scalars}
Copy link

Copilot AI Nov 18, 2025

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.
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}
Copy link

Copilot AI Nov 18, 2025

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API Related to the API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants