feat: add cover image input to media edit template#24
Conversation
PascalRepond
commented
Dec 28, 2025
PascalRepond
commented
- Added a cover image input field to the media edit template.html
- Updated the layout to accommodate the new input field.
- Ensured proper styling and responsiveness for the new input field.
📝 WalkthroughWalkthroughAdds image compression and validation for media cover uploads (automatic on save), a custom cover widget and template with client-side preview controls, updates media edit layout, new tests for image handling, removes Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Browser)
participant Widget as CoverImageWidget (client)
participant Server as Django View/Form
participant Model as Media Model
participant Storage as File Storage
User->>Widget: Select image file
Widget->>Widget: Render preview (FileReader)
User->>Server: Submit form (multipart POST)
Server->>Model: Instantiate Media with uploaded file
Server->>Model: call Media.save()
alt cover changed
Model->>Model: validate file size / type / EXIF
rect rgb(235, 245, 255)
Note over Model: Image processing (compress_image)
Model->>Model: open image (PIL), convert to RGB if needed
Model->>Model: resize (max 800×800, preserve aspect)
Model->>Model: save as JPEG (quality=85) -> ContentFile
end
Model->>Storage: replace cover file with compressed ContentFile
end
Model->>Server: save completed
Server-->>User: Response (success/error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/tests/core/test_models.py (1)
67-103: Good test coverage for the happy path; consider adding edge case tests.The test correctly verifies automatic compression and aspect ratio preservation. The
dbfixture flagged by Ruff is actually required by pytest-django to enable database access in the test, so the warning is a false positive.However, consider adding tests for:
- RGBA/PNG with transparency conversion
- Invalid/corrupt image handling
- Very small images (no upscaling)
Minor optimization for cleanup
# Cleanup - media.cover.delete() + media.cover.delete(save=False) media.delete()Using
save=Falseavoids triggering another save cycle during cleanup.src/templates/media_edit.html (1)
17-20: Add aria-label for accessibility.The close button should include an
aria-labelattribute to improve accessibility for screen reader users, as the icon alone may not convey sufficient context.🔎 Suggested improvement
<div class="absolute top-4 right-4"> - <a href="{% url 'home' %}" class="btn btn-sm btn-circle btn-ghost">{% heroicon_outline "x-mark" class="w-5 h-5" %}</a> + <a href="{% url 'home' %}" + class="btn btn-sm btn-circle btn-ghost" + aria-label="{% translate "Close" %}"> + {% heroicon_outline "x-mark" class="w-5 h-5" %} + </a> </div>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/copilot-instructions.md.gitignoresrc/core/forms.pysrc/core/models.pysrc/core/templates/widgets/cover_input.htmlsrc/templates/media_edit.htmlsrc/tests/core/test_models.py
💤 Files with no reviewable changes (1)
- .github/copilot-instructions.md
🧰 Additional context used
📓 Path-based instructions (3)
**/*.html
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.html: Prefer HTMX for dynamic interactions rather than vanilla JavaScript
Use Tailwind CSS classes for styling before adding custom CSS
Use HTMX instead of traditional JavaScript AJAX calls
Files:
src/core/templates/widgets/cover_input.htmlsrc/templates/media_edit.html
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Use English docstrings in Python code
Follow PEP 8 style guidelines for Python code
Files:
src/core/models.pysrc/core/forms.pysrc/tests/core/test_models.py
**/models.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Models must have an explicit
__str__method
Files:
src/core/models.py
🧠 Learnings (1)
📚 Learning: 2025-12-26T15:18:46.932Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 21
File: src/templates/accounts/profile_edit.html:23-58
Timestamp: 2025-12-26T15:18:46.932Z
Learning: In Django projects, attributes added to a form field's widget via field.widget.attrs.update(...) in the form's __init__ are rendered when using {{ form.field }} in templates. No explicit attribute definitions are needed in the template. This applies to templates under src/templates in Django apps; ensure you update attrs in __init__ for consistent HTMX behavior.
Applied to files:
src/templates/media_edit.html
🧬 Code graph analysis (3)
src/core/models.py (4)
src/core/migrations/0005_media_cover_alter_media_score.py (1)
Migration(6-23)src/core/migrations/0003_media_review_media_review_date_media_score_and_more.py (1)
Migration(9-41)src/core/migrations/0002_alter_media_contributor.py (1)
Migration(6-18)src/core/migrations/0001_initial.py (1)
Migration(7-34)
src/core/forms.py (1)
src/core/migrations/0005_media_cover_alter_media_score.py (1)
Migration(6-23)
src/tests/core/test_models.py (1)
src/core/models.py (2)
save(150-166)Media(60-166)
🪛 Ruff (0.14.10)
src/tests/core/test_models.py
67-67: Unused method argument: db
(ARG002)
🔇 Additional comments (9)
.gitignore (1)
173-176: LGTM!The addition of the
.claude/directory to.gitignoreis appropriate for excluding LLM tool artifacts from version control.src/core/forms.py (2)
8-11: LGTM!The
CoverImageWidgetclass is well-structured with a clear docstring and proper inheritance fromClearableFileInput. The custom template path is correctly specified.
36-36: LGTM!The integration of
CoverImageWidgetin the form is correct, and the Tailwind/DaisyUI classes are appropriately applied.src/tests/core/test_models.py (1)
8-13: LGTM!The imports are appropriate for testing image compression functionality.
src/core/templates/widgets/cover_input.html (1)
1-90: Good HTML structure with appropriate accessibility attributes.The template structure is well-organized with three clear states (existing preview, new preview, placeholder) and includes proper accessibility attributes like
alttext andtitleattributes.src/templates/media_edit.html (4)
28-36: LGTM!The cover field is properly integrated with the new
CoverImageWidget, includes appropriate error handling, and follows the same structure as other form fields.
47-76: Excellent use of HTMX for dynamic interactions!The contributors section properly implements HTMX for autocomplete search functionality, aligning with the coding guidelines that prefer HTMX over vanilla JavaScript. The dropdown positioning and chip display are well-structured.
132-147: LGTM!The card actions section is well-structured with conditional rendering of the delete button and proper confirmation dialog for destructive actions.
160-160: Filecontributors.jsexists and is properly deployed.The required file is located at
src/static/js/contributors.jswith valid JavaScript implementation. No 404 error risk.
- Added a cover image input field to the media edit template.html - Updated the layout to accommodate the new input field. - Ensured proper styling and responsiveness for the new input field.
PascalRepond
force-pushed
the
rep-dev
branch
from
6bffbbc to
233a004
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/models.py (1)
197-210: LGTM! Save override correctly addresses race condition.The implementation properly resolves the past review concern by using the
_fileattribute check instead of querying the database. This approach:
- Eliminates the TOCTOU race condition
- Improves performance by avoiding unnecessary DB queries
- Reliably detects new file uploads via Django's internal mechanism
Minor cleanup opportunity: The
noqa: SLF001directive on line 206 may be unnecessary if the SLF001 rule (private member access) isn't enabled in your Ruff configuration. Consider removing it if not needed.🔎 Optional cleanup
- if self.cover and hasattr(self.cover, "_file") and self.cover._file: # noqa: SLF001 + if self.cover and hasattr(self.cover, "_file") and self.cover._file:
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/copilot-instructions.md.github/workflows/ci.yml.gitignoresrc/core/forms.pysrc/core/models.pysrc/core/templates/widgets/cover_input.htmlsrc/templates/media_edit.htmlsrc/tests/core/test_models.py
💤 Files with no reviewable changes (2)
- .github/copilot-instructions.md
- .github/workflows/ci.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .gitignore
🧰 Additional context used
📓 Path-based instructions (3)
**/*.html
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.html: Prefer HTMX for dynamic interactions rather than vanilla JavaScript
Use Tailwind CSS classes for styling before adding custom CSS
Use HTMX instead of traditional JavaScript AJAX calls
Files:
src/templates/media_edit.htmlsrc/core/templates/widgets/cover_input.html
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Use English docstrings in Python code
Follow PEP 8 style guidelines for Python code
Files:
src/core/forms.pysrc/tests/core/test_models.pysrc/core/models.py
**/models.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Models must have an explicit
__str__method
Files:
src/core/models.py
🧠 Learnings (2)
📚 Learning: 2025-12-26T15:18:46.932Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 21
File: src/templates/accounts/profile_edit.html:23-58
Timestamp: 2025-12-26T15:18:46.932Z
Learning: In Django projects, attributes added to a form field's widget via field.widget.attrs.update(...) in the form's __init__ are rendered when using {{ form.field }} in templates. No explicit attribute definitions are needed in the template. This applies to templates under src/templates in Django apps; ensure you update attrs in __init__ for consistent HTMX behavior.
Applied to files:
src/templates/media_edit.html
📚 Learning: 2025-12-26T15:18:46.932Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 21
File: src/templates/accounts/profile_edit.html:23-58
Timestamp: 2025-12-26T15:18:46.932Z
Learning: In Django projects, when form field widgets have HTMX attributes added via `field.widget.attrs.update()` in the form's `__init__` method (as in src/accounts/forms.py), those attributes are automatically rendered when using `{{ form.field }}` in templates. This is a valid pattern and does not require explicit attribute definitions in the template.
Applied to files:
src/core/forms.py
🧬 Code graph analysis (1)
src/tests/core/test_models.py (1)
src/core/models.py (2)
compress_image(19-90)save(197-210)
🪛 Ruff (0.14.10)
src/tests/core/test_models.py
67-67: Unused method argument: db
(ARG002)
src/core/models.py
206-206: Unused noqa directive (non-enabled: SLF001)
Remove unused noqa directive
(RUF100)
🔇 Additional comments (14)
src/core/forms.py (3)
8-11: LGTM! Clean widget implementation.The custom widget follows Django best practices with a clear docstring and dedicated template path.
31-43: LGTM! Widget configuration improvements enhance UX.The placeholder updates (especially "YYYY" for pub_year and the date format hint for review_date) provide clearer guidance to users. The integration of CoverImageWidget for the cover field is appropriate.
46-68: LGTM! HTMX validation setup is well-structured.The dynamic field validation configuration correctly:
- Excludes file and M2M fields that don't benefit from real-time validation
- Uses appropriate delay (500ms) to debounce user input
- Targets field-specific error containers
- Includes necessary field context
Based on learnings, this pattern works correctly with Django's template rendering.
src/core/templates/widgets/cover_input.html (2)
1-90: LGTM! Well-structured template with good accessibility.The template provides a clean three-state UI (existing cover, new preview, no preview) with proper:
- Internationalization using
{% translate %}- Semantic HTML structure
- Tailwind CSS styling (as per coding guidelines)
- Accessible button labels and titles
91-165: LGTM! Template variable scope issue has been correctly resolved.The JavaScript functions now properly construct the checkbox ID dynamically using
${widgetName}-clear_id(lines 98, 132), which matches Django's ClearableFileInput convention. This ensures the widget works correctly in formsets or multiple instances.The implementation also includes appropriate defensive null checks before manipulating DOM elements.
Note: Vanilla JavaScript is appropriate here for client-side FileReader operations, as HTMX doesn't handle this use case well.
src/tests/core/test_models.py (3)
8-15: LGTM! Appropriate test imports.The new imports for image handling (BytesIO, SimpleUploadedFile, PIL.Image) and model utilities (MAX_FILE_SIZE_MB, compress_image) are necessary for the comprehensive image compression tests.
67-103: LGTM! Comprehensive test for automatic image compression.The test correctly verifies that:
- Large images are automatically compressed on save
- Dimensions fit within the 800x800 constraint
- Aspect ratio is preserved (1920:1080 → 800:450)
- Proper cleanup occurs
Note: The static analysis warning about unused
dbis a false positive. Thedbfixture is a pytest-django pattern that enables database access formedia.save()even though it's not explicitly referenced in the test body.
106-214: LGTM! Excellent security test coverage.The TestCompressImageSecurity class provides comprehensive coverage of:
- File size limits and decompression bomb protection
- Invalid and corrupted file handling
- Unsupported format rejection
- EXIF orientation preservation
- RGBA to RGB conversion
- Normal image processing for JPEG and PNG
These tests ensure robust image handling and align well with the security validations in
compress_image.src/core/models.py (2)
1-16: LGTM! Well-defined security constants and appropriate imports.The imports and security constants provide a solid foundation for safe image processing:
MAX_IMAGE_PIXELSprotects against decompression bombsMAX_FILE_SIZE_MBlimits upload sizeALLOWED_IMAGE_TYPESrestricts to common, safe web formats
19-90: LGTM! Comprehensive and secure image compression implementation.The
compress_imagefunction excellently addresses all security concerns raised in past reviews:
- ✅ File size validation before processing
- ✅ Decompression bomb protection via
MAX_IMAGE_PIXELS- ✅ Image verification with
img.verify()- ✅ Format whitelist validation
- ✅ EXIF orientation preservation with
ImageOps.exif_transpose- ✅ Proper RGBA/transparency handling
- ✅ Comprehensive error handling for all edge cases
- ✅ Resource cleanup in finally block
The implementation is production-ready with clear documentation and robust error handling.
src/templates/media_edit.html (4)
12-29: LGTM! Clean header structure with good UX.The addition of a close button in the top-right corner improves navigation, and the relative/absolute positioning pattern is appropriate. The conditional title properly reflects the form's mode (edit vs. add).
30-38: LGTM! Cover field properly integrated.The cover field integration follows the established pattern with:
- Consistent label structure using the
field_label.htmlpartial- Widget rendering via
{{ form.cover }}- Error display container matching the HTMX validation target
The CoverImageWidget will render its custom template with preview functionality.
49-78: LGTM! Well-implemented contributors autocomplete.The contributors field provides an intuitive UX with:
- Visual chips for selected contributors
- HTMX-powered autocomplete search (adheres to coding guidelines)
- Proper positioning for the suggestions dropdown
- Consistent error display pattern
The HTMX attributes on the search input are appropriate here since this is a custom autocomplete UI separate from the form field widget.
79-163: LGTM! Consistent field structure and well-organized actions.The remaining form fields follow a consistent pattern with proper:
- Grid-based responsive layout
- Label, widget, and error display structure
- Error containers matching HTMX validation targets (per form's
__init__)The action area is well-structured:
- Conditional Delete button with confirmation dialog
- Separate delete form (good practice for method="post" delete actions)
- Save button with clear labeling
1 participant
