Conversation
There was a problem hiding this comment.
Pull request overview
Adds an optional minify query parameter to the /api/topics endpoint to support backward-compatible “full” topic responses (including images) while keeping the default behavior minified for Sefaria.org.
Changes:
- Add
minifyquery param parsing totopics_list_api(defaulting to1). - Pass
minifythrough toTopic.contents()to toggle minified vs full topic payloads. - Update OpenAPI documentation to describe the new
minifyparameter and clarify response behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| reader/views.py | Adds minify query parameter support for /api/topics, wiring it into Topic.contents(). |
| docs/openAPI.json | Documents minify (0/1) and updates endpoint descriptions to reflect minified-by-default behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "parameters": [ | ||
| { | ||
| "examples": { | ||
| "Return all topics": { | ||
| "value": 0 | ||
| }, | ||
| "Return 20 topics": { | ||
| "value": 20 | ||
| } | ||
| }, | ||
| "name": "limit", |
There was a problem hiding this comment.
The newly added parameter blocks under /api/topics use inconsistent indentation compared to the surrounding OpenAPI JSON (many lines are over-indented). Since this file is documentation consumers may diff/parse, please reformat this section to match the existing style (or run the repo’s formatter/generator if applicable) to keep the spec maintainable.
| minify = bool(int(request.GET.get("minify", 1))) | ||
| all_topics = get_all_topics(limit, active_module=request.active_module) | ||
| all_topics_json = [] | ||
| for topic in all_topics: | ||
| topic_json = topic.contents(minify=True, with_html=True) | ||
| topic_json = topic.contents(minify=minify, with_html=True) | ||
| topic_json["titles"] = topic.titles |
There was a problem hiding this comment.
This change introduces new externally-visible behavior (minify toggles between minified vs full topic payloads) but there are no Django API tests covering /api/topics at all. Adding tests for default behavior (minify omitted/1) vs minify=0 (full payload) and for invalid minify values would help prevent regressions and ensure the endpoint doesn’t 500 on bad input.
🧪 CI InsightsHere's what we observed from your CI run for 9310a45. 🟢 All jobs passed!But CI Insights is watching 👀 |
Description
The
topicsendpoint was changed as part of MDL to provide a "minified" response, without including image data. This impacted our users who relied on this endpoint providing images.The goal of this PR is to allow for a backward-compatible
minifyparameter (optional). The default isminify=1, which is fully compatible with the expected and desired behavior for Sefaria.org of thetopics/endpoint returned minified responses. Developers can now call this endpoint with?minify=0to get a response that is not "minified" and includes the image data as before.Code Changes
reader/views.py- Adding an optionalminifyparameter, that defaults to1so it's non-breaking for the current desired behavior on the site.docs/openAPI.json- Updating the API reference documentation.