close
Skip to content

Migrate patrons_follows_json endpoint to FastAPI - #12869

Merged
RayBB merged 4 commits into
internetarchive:masterfrom
Sanket17052006:feat/migrate-patrons-follows-to-fastapi
Jun 10, 2026
Merged

Migrate patrons_follows_json endpoint to FastAPI#12869
RayBB merged 4 commits into
internetarchive:masterfrom
Sanket17052006:feat/migrate-patrons-follows-to-fastapi

Conversation

@Sanket17052006

@Sanket17052006 Sanket17052006 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Closes #

This PR migrates the patrons_follows_json web.py handler to FastAPI.

Technical

  • Add GET /people/{username}/follows.json FastAPI handler
  • Add POST /people/{username}/follows.json FastAPI handler
  • Add FollowEntry Pydantic response model.
  • Mark legacy web.py handler as @deprecated("migrated to fastapi")
  • Add tests.

Testing

Test file : docker compose run --rm home pytest openlibrary/tests/fastapi/test_follows.py -v

    1. Login
curl -s -c /tmp/cookies.txt -X POST "http://localhost:8080/account/login.json" \
  -H "Content-Type: application/json" \
  -d '{"username":"openlibrary","password":"openlibrary"}'
    1. GET authenticated → 200 + data
curl -s -b /tmp/cookies.txt "http://localhost:18080/people/openlibrary/follows.json"
    1. POST subscribe → 303
- curl -s -b /tmp/cookies.txt -X POST "http://localhost:18080/people/openlibrary/follows.json" \
   -d "publisher=openlibrary&state=0&redir_url=/" -w "%{http_code}"
    1. POST unsubscribe → 303
curl -s -b /tmp/cookies.txt -X POST "http://localhost:18080/people/openlibrary/follows.json" \
-d "publisher=openlibrary&state=1&redir_url=/" -w "%{http_code}"
    1. GET unauthenticated → 303 to login
curl -s "http://localhost:18080/people/openlibrary/follows.json" \
 -w "HTTP %{http_code} -> %{redirect_url}"
    1. POST nonexistent publisher → 404
curl -s -b /tmp/cookies.txt -X POST "http://localhost:18080/people/openlibrary/follows.json" \
 -d "publisher=fakepublisher&state=0&redir_url=/" -w "HTTP %{http_code}"
    1. GET wrong user → 303 to login
curl -s -b /tmp/cookies.txt "http://localhost:18080/people/otheruser/follows.json" \
  -w "HTTP %{http_code} -> %{redirect_url}"

Screenshot

N/A

Stakeholders

@RayBB

@mekarpeles

Copy link
Copy Markdown
Member

Thanks for the PR, @Sanket17052006.

🤖 Copilot has been assigned for an initial review.

There are currently 76 open non-draft PRs ahead of yours in the queue. A reviewer has not yet been assigned to this PR.

Possible improvements for this PR

  • Link a GitHub issue — the PR body has Closes # with no issue number filled in. If this migration was tracked in a GitHub issue, please add the number. If no issue exists, consider opening one so the work can be triaged and assigned.
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.

@mekarpeles mekarpeles added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Jun 6, 2026

Copilot AI left a comment

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.

Pull request overview

This PR migrates the legacy patrons_follows_json web.py handler to FastAPI by introducing GET/POST /people/{username}/follows.json, adding a FollowEntry response model, deprecating the old handler, and adding FastAPI tests to cover the new behavior.

Changes:

  • Added FastAPI GET /people/{username}/follows.json to return a user’s follow list (or redirect to login).
  • Added FastAPI POST /people/{username}/follows.json to subscribe/unsubscribe (or redirect to login / 404 on missing publisher).
  • Added a new FastAPI test suite for the follows endpoints and marked the legacy web.py handler as deprecated.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
openlibrary/fastapi/internal/api.py Adds FollowEntry plus new GET/POST follows endpoints in FastAPI.
openlibrary/plugins/openlibrary/api.py Marks the legacy patrons_follows_json web.py handler as deprecated.
openlibrary/tests/fastapi/test_follows.py Adds FastAPI tests for GET/POST follows behavior, redirects, and 404 handling.

Comment thread openlibrary/fastapi/internal/api.py
Comment thread openlibrary/fastapi/internal/api.py
Comment thread openlibrary/fastapi/internal/api.py
@RayBB

RayBB commented Jun 7, 2026

Copy link
Copy Markdown
Collaborator

@Shikhar-395 could you please test and review this?

@Shikhar-395

Copy link
Copy Markdown
Contributor

Verdict: Needs changes for one routing issue.

Finding

This PR adds the FastAPI handlers for /people/{username}/follows.json, but does not add the legacy route pattern to DEPRECATED_PATHS.

openlibrary/code.py only swaps deprecated web.py routes to DeprecatedJSONEndpointHandler for paths listed in openlibrary/plugins/openlibrary/deprecated_handler.py. Without this entry, /people/<username>/follows.json through legacy routing will continue hitting the old web.py handler instead of proxying to FastAPI.

Please add:

(r"(/people/[^/]+)/follows", "json"),

@github-project-automation github-project-automation Bot moved this to Waiting Review/Merge from Staff in Ray's Project Jun 8, 2026
@github-actions github-actions Bot removed the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Jun 9, 2026
@Sanket17052006

Copy link
Copy Markdown
Contributor Author

Hey @Shikhar-395, I've addressed the review and added the legacy route pattern to DEPRECATED_PATHS. Please take a look!

@github-actions github-actions Bot added the Needs: Response Issues which require feedback from lead label Jun 9, 2026
Comment thread openlibrary/plugins/openlibrary/deprecated_handler.py Outdated
@Sanket17052006
Sanket17052006 force-pushed the feat/migrate-patrons-follows-to-fastapi branch from d549be0 to c6d18a8 Compare June 9, 2026 14:22
@RayBB RayBB added On Testing and removed Needs: Response Issues which require feedback from lead labels Jun 10, 2026

@RayBB RayBB left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Put it on testing and it's working flawlessly.

Great work @Sanket17052006 and @Shikhar-395 !

@RayBB
RayBB merged commit 6afe563 into internetarchive:master Jun 10, 2026
4 checks passed
@github-project-automation github-project-automation Bot moved this from Waiting Review/Merge from Staff to Done in Ray's Project Jun 10, 2026
@Sanket17052006
Sanket17052006 deleted the feat/migrate-patrons-follows-to-fastapi branch July 1, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants