Skip to content

fix: serve documentation assets inline so SVG images render - #21

Merged
barredterra merged 1 commit into
version-15from
fix-svg-asset-mime
Aug 16, 2026
Merged

fix: serve documentation assets inline so SVG images render#21
barredterra merged 1 commit into
version-15from
fix-svg-asset-mime

Conversation

@barredterra

Copy link
Copy Markdown
Member

No description provided.

@greptile-apps

greptile-apps Bot commented Aug 15, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking opportunity to make the regression test verify the actual HTTP response behavior.

The implementation uses Frappe's supported inline download-response metadata, but the test would not detect a failure in translating that metadata into the MIME and disposition headers required for SVG rendering.

Files Needing Attention: compendium/tests/test_docs.py

Fix all with Greploop

Fix All in Cursor

Prompt To Fix All With AI
### Issue 1
compendium/tests/test_docs.py:184-189
**Test skips HTTP response conversion**

This regression test inspects only the intermediate `frappe.response` dictionary, so it will continue to pass when Frappe fails to translate that metadata into the MIME type and inline Content-Disposition required for SVG rendering. Exercise the response conversion and assert the resulting HTTP headers to cover the behavior this change fixes.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix: serve documentation assets inline s..." | Re-trigger Greptile

Comment on lines +184 to +189
):
get_asset("page", "diagram.svg", locale="en")
self.assertEqual(frappe.response["type"], "download")
self.assertEqual(frappe.response["display_content_as"], "inline")
self.assertEqual(frappe.response["filename"], "diagram.svg")
self.assertEqual(frappe.response["filecontent"], svg)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Test skips HTTP response conversion

This regression test inspects only the intermediate frappe.response dictionary, so it will continue to pass when Frappe fails to translate that metadata into the MIME type and inline Content-Disposition required for SVG rendering. Exercise the response conversion and assert the resulting HTTP headers to cover the behavior this change fixes.

Prompt To Fix With AI
This is a comment left during a code review.
Path: compendium/tests/test_docs.py
Line: 184-189

Comment:
**Test skips HTTP response conversion**

This regression test inspects only the intermediate `frappe.response` dictionary, so it will continue to pass when Frappe fails to translate that metadata into the MIME type and inline Content-Disposition required for SVG rendering. Exercise the response conversion and assert the resulting HTTP headers to cover the behavior this change fixes.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Cursor

@barredterra
barredterra merged commit fa2f992 into version-15 Aug 16, 2026
4 checks passed
@barredterra
barredterra deleted the fix-svg-asset-mime branch August 16, 2026 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant