Migrate account loans to FastAPI#12889
Conversation
| class TestAccountLoansPages: | ||
| def test_loans_page_renders_legacy_template(self, fastapi_client, mock_optional_authenticated_user): | ||
| legacy_user = FakeLegacyUser() | ||
| template = web.storage(rawtext="<main>Loans page</main>") |
There was a problem hiding this comment.
Best Practice: Page has multiple main landmarks.
Page should have exactly one main landmark.
Details
The main landmark contains the primary content of the page. Screen readers allow users to jump directly to main content. Use a single <main> element (or role='main') to wrap the central content, excluding headers, footers, and navigation.
Best Practice: Page has multiple main landmarks.
Page should not have more than one main landmark.
Details
Only one main landmark should exist per page. The main landmark identifies the primary content area. If you have multiple content sections, use <section> with appropriate headings instead of multiple main elements.
RayBB
left a comment
There was a problem hiding this comment.
Good start. We only want to migrate the json endpoints for this so where encoding = "json" in the get method.
Once you fix that up tag @Sanket17052006 to review/test!
|
@Sanket17052006 PTAL. |
Sanket17052006
left a comment
There was a problem hiding this comment.
All the tests and endpoints are working as intended. LGTM!
RayBB
left a comment
There was a problem hiding this comment.
I put this up on testing and it kind of worked except failing intermittently because of the shim. We never should shim threaded dicts for web.ctx for fastapi endpoints because they can hit race conditions.
As such, I removed the shim and replaced the web.ctx.site with calls to site.get().
This is working very well on testing and it's ready to merge.
Thanks for your work on this one! It was a deceptively hard one with all the code paths that this goes down.
A great next step would be to try to asyncify these two endpoints but might also be a challenging one since the IA code goes through many layers.
Summary
Migrates the account loan and loan-history routes from legacy web.py routing to FastAPI while preserving existing My Books rendering and lending helper behavior.
Migrated endpoints:
Testing
/account/loans.jsonmatches legacy behavior.user_borrow_history401 behavior on both legacy and FastAPI.Stakeholders
@RayBB