migrate import_preview_json to FastAPI#12745
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the import preview JSON handler toward FastAPI by adding a new router, registering it in the ASGI app, marking the legacy JSON handler deprecated, and adding endpoint tests.
Changes:
- Adds FastAPI GET/POST handlers for import preview JSON behavior.
- Registers the import API router in the main ASGI app.
- Adds FastAPI tests for auth, save behavior, and invalid source handling.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
openlibrary/fastapi/importapi.py |
Adds FastAPI import preview endpoints and shared request handling. |
openlibrary/asgi_app.py |
Includes the new import API router. |
openlibrary/plugins/importapi/import_ui.py |
Marks the legacy JSON preview handler as deprecated. |
openlibrary/tests/fastapi/test_importapi.py |
Adds tests for the new FastAPI import preview endpoints. |
Comments suppressed due to low confidence (1)
openlibrary/fastapi/importapi.py:84
- Registering POST on
/import/previewintercepts the existing HTML form posts fromopenlibrary/templates/import_preview.html. Those posts currently render a preview or redirect after saving, but this FastAPI handler always returns JSON, so the import preview UI's submit/import flow will break; use the legacy JSON endpoint URL (such as the.jsonroute) or keep the HTML route handled by the legacy page.
@router.post("/import/preview")
RayBB
left a comment
There was a problem hiding this comment.
In general it looks like it's in the right direction.
Doesn't seem to be working. Please test the endpoints before asking for review.
http://localhost:18080/import/preview.json?provider=ia&identifier=roughcutmystery00gorm
We'll have to use site.get() instead of web.ctx.site
The tests could also be DRY'd up with fixtures for mocking current user, admin user, import request.
So please go head and fix this and ensure it actually works locally for both new endpoints. Give the examples you used to test too.
|
Thanks for the review! I'll fix the issues and test the endpoint thoroughly before updating. |
e97c8ff to
d4c3260
Compare
for more information, see https://pre-commit.ci
|
Hey @RayBB , I've fixed the issues locally - these are the examples I used :
POST with save:
No auth:
|
Closes #
This PR migrates
import_preview_jsonfrom web.py to FastAPI.Technical
openlibrary/fastapi/importapi.pywithGET /import/previewandPOST /import/previewendpointsimport_preview_jsonclass inimport_ui.pywith@deprecated("migrated to fastapi")asgi_app.pyTesting
docker compose run --rm home python -m pytest openlibrary/tests/fastapi/test_importapi.py -v
Screenshot
Stakeholders