fix: serve documentation assets inline so SVG images render - #21
Conversation
Confidence Score: 4/5The 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 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 |
| ): | ||
| 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) |
There was a problem hiding this 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.
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!
No description provided.