Conversation
This reverts commit 258104a.
There was a problem hiding this comment.
Pull request overview
This PR reverts a prior change that added special handling for SVG sponsor logos, restoring the previous behavior of always generating PNG thumbnails via sorl-thumbnail.
Changes:
- Remove SVG-specific branching in the public sponsors list partial and always render thumbnails for
web_logo. - Remove SVG-specific branching in the Django admin logo renderers and revert to template-based thumbnail rendering.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/sponsors/templates/sponsors/partials/sponsors-list.html | Reverts conditional SVG rendering; always uses {% thumbnail %} for sponsor web_logo images. |
| apps/sponsors/admin.py | Reverts SVG bypass logic in admin logo display methods; returns to thumbnail-based rendering for logos. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| html = "{% load thumbnail %}{% thumbnail sponsor.web_logo '150x150' format='PNG' quality=100 as im %}<img src='{{ im.url}}'/>{% endthumbnail %}" | ||
| template = Template(html) | ||
| context = Context({"img": img}) | ||
| return mark_safe(template.render(context)) # noqa: S308 | ||
| context = Context({"sponsor": obj.sponsor}) | ||
| html = template.render(context) | ||
| return mark_safe(html) # noqa: S308 |
There was a problem hiding this comment.
get_sponsor_web_logo now always renders a {% thumbnail sponsor.web_logo %} block without guarding for empty/missing files. In this codebase, Sponsor instances can exist without an actual web_logo file (e.g., tests use model_bakery without _create_files=True), which will cause sorl-thumbnail to raise and break the admin changelist/detail rendering. Add an early return (e.g., "---") when obj.sponsor.web_logo is falsy, or otherwise handle missing files before invoking thumbnail.
| img = obj.sponsor.print_logo | ||
| if not img: | ||
| return "---" | ||
| if img.name and img.name.lower().endswith(".svg"): | ||
| return format_html( | ||
| '<img src="{}" style="max-width:150px;max-height:150px"/>', | ||
| img.url, | ||
| html = "---" | ||
| if img: | ||
| template = Template( | ||
| "{% load thumbnail %}{% thumbnail img '150x150' format='PNG' quality=100 as im %}<img src='{{ im.url}}'/>{% endthumbnail %}" | ||
| ) | ||
| template = Template( | ||
| "{% load thumbnail %}{% thumbnail img '150x150' format='PNG' quality=100 as im %}<img src='{{ im.url}}'/>{% endthumbnail %}" | ||
| ) | ||
| context = Context({"img": img}) | ||
| return mark_safe(template.render(context)) # noqa: S308 | ||
| context = Context({"img": img}) | ||
| html = mark_safe(template.render(context)) # noqa: S308 | ||
| return html |
There was a problem hiding this comment.
get_sponsor_print_logo thumbnails obj.sponsor.print_logo, but print_logo explicitly allows svg and eps/epsfepsi uploads. With only Pillow/sorl-thumbnail configured, SVG/EPS inputs will fail thumbnail generation and can crash the admin page. Handle non-raster formats explicitly (e.g., render SVG via direct <img src=...> or provide a download link; skip thumbnailing for EPS), and only call {% thumbnail %} for raster images like PNG/JPG.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a943b6d6bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| template = Template( | ||
| "{% load thumbnail %}{% thumbnail img '150x150' format='PNG' quality=100 as im %}<img src='{{ im.url}}'/>{% endthumbnail %}" | ||
| ) |
There was a problem hiding this comment.
Sponsors can still upload SVG print logos (apps/sponsors/models/sponsors.py:52-59, and apps/sponsors/tests/test_forms.py:427-440 covers that path), but this revert now sends every non-empty print_logo through {% thumbnail ... format='PNG' %} again. That removes the only SVG bypass, so opening the sponsorship admin views for any sponsor using the documented SVG format can reproduce the original Sentry failure instead of rendering the uploaded file.
Useful? React with 👍 / 👎.
This reverts commit 258104a.