Skip to content

Add redirect for / to avoid 404 responses#10

Open
mrauter1 wants to merge 2 commits intomainfrom
codex/fix-404-error-on-root-route
Open

Add redirect for / to avoid 404 responses#10
mrauter1 wants to merge 2 commits intomainfrom
codex/fix-404-error-on-root-route

Conversation

@mrauter1
Copy link
Copy Markdown
Owner

Motivation

  • The application returned 404 for requests to the site root (/), which should instead take users to the main UI at /app.

Description

  • Add an explicit FastAPI root endpoint @app.get("/", include_in_schema=False) that returns a RedirectResponse to /app with HTTP 303.
  • Import RedirectResponse from fastapi.responses to support the redirect.
  • Add a regression test test_root_route_redirects_to_app asserting that GET / responds with status 303 and Location: /app.

Testing

  • Ran pytest -q tests/test_auth_requester.py -k root_route_redirects_to_app -rs, which was skipped in this environment because fastapi is not installed.
  • Compiled the modified files with python -m compileall app/main.py tests/test_auth_requester.py, which succeeded.

Codex Task

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements a redirect from the root path ('/') to '/app' using a 303 See Other status code and adds a corresponding test case to verify the functionality. The reviewer suggests improving the maintainability of the code by replacing hardcoded strings and status codes with constants in both the application logic and the test suite.

app/main.py Outdated

@app.get("/", include_in_schema=False)
def root_redirect() -> RedirectResponse:
return RedirectResponse("/app", status_code=303)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This line uses hardcoded values for the path ("/app") and status code (303). For better maintainability and readability, it's recommended to:

  • Define "/app" as a constant and reuse it here and in the tests.
  • Use the symbolic constant status.HTTP_303_SEE_OTHER from fastapi instead of the number 303. You'll need to import status from fastapi.

Comment on lines +544 to +545
assert response.status_code == 303
assert response.headers["location"] == "/app"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

These assertions use hardcoded values (303 and "/app"). To make the test more robust and maintainable, consider using constants instead:

  • Assert against status.HTTP_303_SEE_OTHER from fastapi for the status code.
  • Assert against the same path constant suggested for the application code for the location header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant