Skip to content

fix: add .json suffix to check-ins route#12392

Closed
Jah-yee wants to merge 1 commit intointernetarchive:masterfrom
Jah-yee:fix/checkins-json-suffix
Closed

fix: add .json suffix to check-ins route#12392
Jah-yee wants to merge 1 commit intointernetarchive:masterfrom
Jah-yee:fix/checkins-json-suffix

Conversation

@Jah-yee
Copy link
Copy Markdown

@Jah-yee Jah-yee commented Apr 16, 2026

Summary

Frontend POSTs to /works/OL{work_id}W/check-ins.json but FastAPI route only defined for /works/OL{work_id}W/check-ins causing 404.

Fix

Added .json suffix to FastAPI router decorator.

Testing

  • Check-in submission on book pages returns 200 instead of 404

Closes #12391

Frontend POSTs to /works/OL{work_id}W/check-ins.json but FastAPI route only defined for /works/OL{work_id}W/check-ins causing 404.
@github-actions github-actions bot added the Priority: 0 Fix now: Issue prevents users from using the site or active data corruption. [managed] label Apr 16, 2026
@github-project-automation github-project-automation bot moved this to Waiting Review/Merge from Staff in Ray's Project Apr 16, 2026
@mekarpeles mekarpeles requested a review from Copilot April 16, 2026 08:43
@mekarpeles
Copy link
Copy Markdown
Member

Thanks for the PR, @Jah-yee! Welcome to Open Library — this is your first contribution.

🤖 Copilot has been assigned for an initial review.

@RayBB is assigned to this PR and currently has:

  • 0 open PR(s) of equal or higher priority to review first
PR triage checklist (maintainers / Pam)
  • PR description — not empty; explains what the change does and how to verify it
  • References an issue — PR body contains a #NNN reference
    • Linked issue is triaged — has a Priority: * label (not just Needs: Triage)
    • Linked issue is assigned — has at least one assignee
  • Commit history clean — no WIP/fixup/conflict noise; commit messages are meaningful
  • CI passing — no failing check-runs
  • Test cases present — if the change touches substantive logic, test coverage exists or is explained
  • Proof of testing — PR body includes a description of what was tested, a screenshot, or a video

Note

This comment was automatically generated by Pam, Open Library's Project AI Manager, on behalf of @mekarpeles. Pam is designed to provide status visibility, perform basic project management functions and relevant codebase research, and provide actionable feedback so contributors aren't left waiting.

Copy link
Copy Markdown
Contributor

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

Fixes a 404 when submitting reading-log check-ins by aligning the FastAPI POST route with the frontend’s .json URL.

Changes:

  • Updated the FastAPI check-ins POST route to use /works/OL{work_id}W/check-ins.json.

@@ -51,7 +51,7 @@ class CheckInResponse(BaseModel):
id: int


Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

Changing the route from /works/OL{work_id}W/check-ins to only /works/OL{work_id}W/check-ins.json will break existing callers that still POST to the non-suffixed path (e.g. openlibrary/plugins/openlibrary/js/goodreads_import.js posts to ${work.key}/check-ins). To avoid a regression, keep the original route as an alias (e.g. add a second @router.post("/works/OL{work_id}W/check-ins", include_in_schema=False) decorator above this handler, similar to openlibrary/fastapi/books.py:42-44).

Suggested change
@router.post("/works/OL{work_id}W/check-ins", include_in_schema=False)

Copilot uses AI. Check for mistakes.
Comment thread openlibrary/fastapi/checkins.py
@RayBB
Copy link
Copy Markdown
Collaborator

RayBB commented Apr 16, 2026

Please follow our contributing guidelines and wait to be assigned before working on any issues. Thanks

https://docs.openlibrary.org/developers/CONTRIBUTING.html

@RayBB RayBB closed this Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: 0 Fix now: Issue prevents users from using the site or active data corruption. [managed]

Projects

Status: Waiting Review/Merge from Staff

Development

Successfully merging this pull request may close these issues.

Bug: Check-in fails with 404 due to .json suffix mismatch in FastAPI route

4 participants