Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
9 changes: 5 additions & 4 deletions chatbot-core/api/routes/chatbot.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from fastapi import (
APIRouter,
HTTPException,
Request,
Response,
WebSocket,
WebSocketDisconnect,
Expand Down Expand Up @@ -155,17 +156,17 @@ async def chatbot_stream(websocket: WebSocket, session_id: str):
response_model=SessionResponse,
status_code=status.HTTP_201_CREATED,
)
def start_chat(response: Response):
def start_chat(request: Request, response: Response):
"""
Create a new chat session.

Returns a unique session ID that can be used for subsequent
chatbot interactions.
"""
session_id = init_session()
response.headers["Location"] = (
f"/sessions/{session_id}/message"
)
response.headers["Location"] = request.url_for(
Copy link
Contributor

Choose a reason for hiding this comment

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

In my mind, I thought Location would be used by UI to navigate the page.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Location

Content-Location may be more appropriate

https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Location

"chatbot_reply", session_id=session_id
).path
return SessionResponse(session_id=session_id)

@router.delete(
Expand Down
20 changes: 20 additions & 0 deletions chatbot-core/tests/unit/routes/test_chatbot.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
"""Unit Tests for FastAPI routes."""

from fastapi import FastAPI
from fastapi.testclient import TestClient

from api.routes.chatbot import router


def test_start_chat(client, mock_init_session):
Copy link
Contributor

Choose a reason for hiding this comment

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

"""Testing that creating a session returns session ID and location."""
mock_init_session.return_value = "test-session-id"
Expand All @@ -11,6 +17,20 @@ def test_start_chat(client, mock_init_session):
assert response.headers["location"] == "/sessions/test-session-id/message"
Copy link
Contributor

Choose a reason for hiding this comment

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

You should change this line, instead of creating a new test



def test_start_chat_location_includes_router_prefix(mock_init_session):
"""Location header should include API prefix when router is mounted with one."""
mock_init_session.return_value = "test-session-id"

app = FastAPI()
app.include_router(router, prefix="/api/chatbot")
prefixed_client = TestClient(app)

response = prefixed_client.post("/api/chatbot/sessions")

assert response.status_code == 201
assert response.headers["location"] == "/api/chatbot/sessions/test-session-id/message"


def test_chatbot_reply_success(client, mock_session_exists, mock_get_chatbot_reply):
"""Testing that sending a valid message in a valid session returns a response."""
mock_session_exists.return_value = True
Expand Down
Loading