Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 30 additions & 1 deletion backend/python/app/routers/route_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from app.dependencies.auth import require_driver
from app.models import get_session
from app.models.route import RouteWithDateRead
from app.models.route import Route, RouteWithDateRead
from app.services.implementations.route_service import RouteService

# Initialize service
Expand Down Expand Up @@ -38,6 +38,35 @@ async def get_routes(
return routes


@router.get("/{route_id}", response_model=Route, status_code=status.HTTP_200_OK)
async def get_route(
route_id: UUID,
session: AsyncSession = Depends(get_session),
) -> Route:
"""
Get a route by its unique identifier.

Parameters:
route_id (UUID): The unique identifier of the route to delete.
session (AsyncSession): The database session dependency.

Authentication:
Requires the user to be authenticated as a driver.

Returns:
None. Responds with HTTP 200 OK on successful get.

Raises:
HTTPException:
- 404 Not Found: If the route with the specified ID does not exist.
- 500 Server Error
"""
Comment on lines +46 to +63
Copy link
Collaborator

@ludavidca ludavidca Feb 11, 2026

Choose a reason for hiding this comment

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

It's good that you are leaving docstring, but I think that this might be a little too verbose. Just a description, parameter, and returns are fine. Auth should be evident by reading code and raises can be a quick reference to 404 (500 comes default)!


# TODO: the auth here does not work, I think this is an auth issue
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove To Do, good catch, and I will make sure to address this soon.

route = await route_service.get_route(session, route_id)
return route


@router.delete("/{route_id}", status_code=status.HTTP_204_NO_CONTENT)
async def delete_route(
route_id: UUID,
Expand Down
21 changes: 21 additions & 0 deletions backend/python/app/services/implementations/route_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from datetime import datetime
from uuid import UUID

from fastapi import HTTPException
from sqlalchemy import and_, exists
from sqlalchemy import select as sql_select
from sqlalchemy.ext.asyncio import AsyncSession
Expand Down Expand Up @@ -92,6 +93,25 @@ async def get_routes(
for row in rows
]

async def get_route(self, session: AsyncSession, route_id: UUID) -> Route:
try:
"""Get route by ID"""
statement = select(Route).where(Route.route_id == route_id)
result = await session.execute(statement)
route = result.scalars().first()

if not route:
self.logger.error(f"Route with id {route_id} not found")
raise ValueError(f"Route with id {route_id} not found")

Comment on lines +102 to +106
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this error handling would raise a 500 Internal Server Error rather than a 404. It would be better to do something like:

raise HTTPException(
status_code=404,
detail=f"Route with id {route_id} not found"
)

return route
except Exception as error:
self.logger.exception("Failed to get route %s", route_id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does %s mean?

await session.rollback()
raise HTTPException(
status_code=500, detail="Failed to retrieve route."
) from error

async def delete_route(self, session: AsyncSession, route_id: UUID) -> bool:
"""Delete route by ID"""
try:
Expand All @@ -110,4 +130,5 @@ async def delete_route(self, session: AsyncSession, route_id: UUID) -> bool:
except Exception as error:
self.logger.error(f"Failed to delete route {route_id}: {error!s}")
await session.rollback()
# TODO: do we really want to return the raw error
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be good for now! There might be logging that is done at a level above; this just raises an error.

raise error