Skip to content

feat(media): improve media detail and edit templates#38

Merged
PascalRepond merged 1 commit intomainfrom
rep-dev
Jan 4, 2026
Merged

feat(media): improve media detail and edit templates#38
PascalRepond merged 1 commit intomainfrom
rep-dev

Conversation

@PascalRepond
Copy link
Copy Markdown
Owner

  • Enhance the media detail view and editor with better UI elements.
  • Fix adding contributors beginning the same as existing ones.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 4, 2026

📝 Walkthrough

Walkthrough

Adds a media detail view and route, a dedicated detail template, reworks media edit and list templates for linked navigation and multi-column layout, introduces a reusable confirmation modal, updates the star-rating widget to use a badge UI, adds French translations, and includes tests for the detail view.

Changes

Cohort / File(s) Summary
Media Detail Feature
src/core/urls.py, src/core/views.py, src/templates/media_detail.html, src/tests/core/test_views.py
New login-protected media_detail view and URL pattern; template renders two-column detail (cover, metadata, contributors, external link, score, review); tests for access, template, 404, field rendering, and contributor links.
Media Edit UI Restructure
src/templates/media_edit.html, src/static/js/media_edit.js
Replaced single-column edit with multi-column layout, new header/back button/breadcrumbs, save button placement, modal-based delete flow, contributors chips + HTMX search preserved; JS adds "Set to today" handler and removes auto-select-on-Enter for contributor suggestions.
Media List & Partials
src/templates/partials/media-items.html, src/templates/partials/media-cover.html, src/templates/partials/media-score-badge.html, src/templates/partials/media-score-stars.html, src/templates/partials/media-contributors.html
Grid/list covers and titles now link to media detail; cover hover opacity; score badge formatting tweaked; read-only 10-star display added; contributors partial gains use_htmx flag to render HTMX links or plain anchors.
Star Rating Widget
src/core/templates/widgets/star_rating.html
Internal rendering refactor: label area now shows a badge (score + star icon + label) via updateLabelBadge(...); interactions (hover/click/clear/restore) updated to use badge rendering; inline SVG stub and styles added.
Modal & Backup Flow
src/templates/partials/confirm-modal.html, src/templates/backup_manage.html
New reusable confirmation modal partial with configurable text/form binding and danger styling; backup import form now uses modal confirmation instead of inline onsubmit.
Forms & Styling
src/templates/accounts/profile_edit.html, src/theme/static_src/src/styles.css
Error span classes switched to label-text-alt text-error; new .label-text-alt utility added to styles.
Internationalization
src/locale/fr/LC_MESSAGES/django.po
Added/updated French translations (e.g., "Back to list", "Cover of", "External metadata", "Today", "Set to today", "Delete Media", "Score:", "out of 10") and adjusted wrapping/context references.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant DjangoView as Server/View
  participant DB as Database
  note over Server/View,DB `#f3f4f6`: New media_detail flow
  Client->>Server/View: GET /media/<pk>/
  Server/View->>DB: Query Media(pk)
  DB-->>Server/View: Media instance (or 404)
  alt Media found
    Server/View->>Server/View: render media_detail.html (includes score/read-only stars, contributors partials)
    Server/View-->>Client: 200 HTML response
  else Not found
    Server/View-->>Client: 404 response
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: improvements to media detail view and edit templates with new features and UI enhancements.
Description check ✅ Passed The description relates to the changeset, covering UI enhancements to media detail and edit templates, though it lacks detail about contributor fixes.
Docstring Coverage ✅ Passed Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc4c2cd and 5cb280c.

📒 Files selected for processing (17)
  • src/core/templates/widgets/star_rating.html
  • src/core/urls.py
  • src/core/views.py
  • src/locale/fr/LC_MESSAGES/django.po
  • src/static/js/media_edit.js
  • src/templates/accounts/profile_edit.html
  • src/templates/backup_manage.html
  • src/templates/media_detail.html
  • src/templates/media_edit.html
  • src/templates/partials/confirm-modal.html
  • src/templates/partials/media-contributors.html
  • src/templates/partials/media-cover.html
  • src/templates/partials/media-items.html
  • src/templates/partials/media-score-badge.html
  • src/templates/partials/media-score-stars.html
  • src/tests/core/test_views.py
  • src/theme/static_src/src/styles.css
🚧 Files skipped from review as they are similar to previous changes (8)
  • src/templates/partials/media-cover.html
  • src/templates/partials/media-score-stars.html
  • src/locale/fr/LC_MESSAGES/django.po
  • src/core/views.py
  • src/templates/backup_manage.html
  • src/core/urls.py
  • src/templates/accounts/profile_edit.html
  • src/templates/partials/media-score-badge.html
🧰 Additional context used
🧠 Learnings (5)
📚 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
  • src/templates/partials/media-items.html
  • src/templates/partials/confirm-modal.html
  • src/templates/partials/media-contributors.html
  • src/templates/media_detail.html
📚 Learning: 2026-01-03T21:16:52.649Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-items.html:67-67
Timestamp: 2026-01-03T21:16:52.649Z
Learning: In HTML templates (e.g., src/templates/partials/media-items.html), Tailwind CSS v4.1.11 supports arbitrary numeric utilities (max-w-{n}, w-{n}, h-{n}) that map to calc(var(--spacing) * n). These should be considered valid Tailwind classes and not flagged as invalid. Ensure review tooling and linters treat such numeric utilities as allowed, and apply this guidance to other HTML/template files in the repository.

Applied to files:

  • src/templates/media_edit.html
  • src/templates/partials/media-items.html
  • src/templates/partials/confirm-modal.html
  • src/templates/partials/media-contributors.html
  • src/core/templates/widgets/star_rating.html
  • src/templates/media_detail.html
📚 Learning: 2026-01-04T08:55:43.091Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-score-badge.html:1-9
Timestamp: 2026-01-04T08:55:43.091Z
Learning: In Django projects, configuring heroicons in TEMPLATES builtins makes heroicons.templatetags.heroicons available in all templates without needing {% load heroicons %}. For template reviews in this codebase, assume heroicon tags work globally in HTML templates under templates/, so avoid requiring explicit loads. If a template still uses {% load heroicons %}, assess whether the load is unnecessary and can be removed; ensure no conflicts arise from global tags.

Applied to files:

  • src/templates/media_edit.html
  • src/templates/partials/media-items.html
  • src/templates/partials/confirm-modal.html
  • src/templates/partials/media-contributors.html
  • src/core/templates/widgets/star_rating.html
  • src/templates/media_detail.html
📚 Learning: 2025-12-27T18:00:03.595Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 23
File: src/templates/partials/score-readonly.html:2-33
Timestamp: 2025-12-27T18:00:03.595Z
Learning: In src/templates/partials/score-readonly.html, the DaisyUI read-only rating pattern correctly uses `aria-current="true"` on the selected star and applies the same color class (bg-orange-400) to all star elements. DaisyUI's CSS automatically handles which stars are visually filled based on the aria-current position.

Applied to files:

  • src/core/templates/widgets/star_rating.html
📚 Learning: 2026-01-03T21:16:52.649Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-items.html:67-67
Timestamp: 2026-01-03T21:16:52.649Z
Learning: The datakult project uses Tailwind CSS v4.1.11, which supports arbitrary numeric utility classes like max-w-{number}, w-{number}, h-{number}, etc., that map to calc(var(--spacing) * <number>). These numeric utilities are valid and should not be flagged as invalid Tailwind classes.

Applied to files:

  • src/theme/static_src/src/styles.css
🧬 Code graph analysis (3)
src/templates/media_edit.html (2)
src/core/migrations/0003_media_review_media_review_date_media_score_and_more.py (1)
  • Migration (9-41)
src/core/migrations/0005_media_cover_alter_media_score.py (1)
  • Migration (6-23)
src/templates/partials/media-items.html (2)
src/core/migrations/0003_media_review_media_review_date_media_score_and_more.py (1)
  • Migration (9-41)
src/core/migrations/0005_media_cover_alter_media_score.py (1)
  • Migration (6-23)
src/tests/core/test_views.py (2)
src/tests/conftest.py (3)
  • logged_in_client (65-68)
  • media (20-31)
  • agent (12-16)
src/core/models.py (2)
  • Agent (94-105)
  • Media (108-226)
🪛 Ruff (0.14.10)
src/tests/core/test_views.py

700-700: Unused method argument: db

(ARG002)


722-722: Unused method argument: db

(ARG002)

🔇 Additional comments (14)
src/theme/static_src/src/styles.css (1)

31-34: LGTM!

The .label-text-alt class is well-implemented with appropriate Tailwind utilities for form validation error text. The utilities ensure proper text wrapping and layout without overflow issues.

src/templates/partials/media-items.html (2)

9-33: LGTM!

The grid view changes effectively make both the cover and title clickable navigation elements to the detail view. The pointer-events-none on badges correctly prevents them from intercepting clicks, and the hover transition provides good visual feedback.


53-64: LGTM!

The list view changes mirror the grid view implementation, maintaining consistency across view modes. The table structure is preserved correctly, and the hover effects are appropriate.

src/core/templates/widgets/star_rating.html (3)

37-50: Past inconsistency resolved.

The visual spacing inconsistency flagged in the previous review (template vs. JavaScript badge rendering) has been resolved. Both the template (line 43) and JavaScript (line 81) now render the score without the non-breaking space, ensuring consistent display across both paths.


77-90: LGTM!

The updateLabelBadge helper function is a good refactoring that centralizes badge rendering logic. The inline SVG approach is appropriate for JavaScript since Django template tags aren't available, and it matches the heroicons star icon used in the template.


92-157: LGTM!

All label update operations have been consistently refactored to use the updateLabelBadge helper. The implementation is clean, maintainable, and handles all interaction cases (click, hover, restore, clear) correctly.

src/static/js/media_edit.js (3)

3-11: LGTM!

The "Set to today" button implementation is clean and functional. The date formatting using toISOString().split('T')[0] effectively produces the required YYYY-MM-DD format, and the guard check ensures robustness.


37-51: LGTM!

The contributorAlreadyExists function correctly prevents duplicate contributors by checking both existing database entries and dynamically created chips. The case-insensitive, trimmed comparison ensures reliable duplicate detection.


78-89: LGTM!

The Enter key handler now focuses on adding explicitly typed contributors without auto-selecting from the dropdown. This change improves user control and prevents accidental selections.

src/templates/partials/media-contributors.html (1)

1-21: LGTM!

The conditional rendering based on use_htmx is well-implemented. The check {% if use_htmx == False %} correctly defaults to HTMX-enabled links when the parameter is not provided, maintaining backward compatibility while allowing non-HTMX rendering for contexts like the detail view.

src/templates/partials/confirm-modal.html (1)

1-28: LGTM! Well-structured reusable modal.

The modal implementation is clean and follows DaisyUI patterns correctly. The conditional rendering based on form_id provides good flexibility for both standalone and form-bound confirmation flows.

One note: The message|safe filter on Line 14 allows HTML rendering. Ensure that all usages of this modal pass only trusted, system-generated messages (not user input) to prevent XSS vulnerabilities. Based on the PR context, this appears to be the case (deletion confirmations use translated strings).

src/templates/media_detail.html (1)

1-113: LGTM! Clean and well-structured detail view.

The template is well-organized with:

  • Responsive two-column layout that adapts for mobile
  • Proper conditional rendering for optional fields (cover, contributors, external_uri, score, review)
  • Correct use of use_htmx=False for the contributors partial (Line 63), ensuring normal navigation links in the detail view as verified by tests
  • Safe HTML rendering of review_rendered (Line 106), which is sanitized by the markdown validator

The breadcrumb navigation and back button provide good UX for navigation.

src/tests/core/test_views.py (1)

677-737: LGTM! Comprehensive test coverage for the detail view.

The test suite properly validates:

  • Access control (logged-in requirement)
  • Template usage
  • 404 handling for nonexistent media
  • Complete field rendering
  • Contributor link behavior (filtered home page without HTMX)

Regarding the static analysis hints about unused db fixture on Lines 700 and 722: These are false positives. The db fixture is required by pytest-django to enable database access for ORM operations (Agent.objects.create(), Media.objects.create()), even though it's not explicitly referenced in the function body.

src/templates/media_edit.html (1)

16-231: LGTM! Well-executed refactor with improved UX.

The template refactor delivers several improvements:

  • Clear header with navigation breadcrumbs, back button, and prominent Save button
  • Responsive three-column layout that organizes fields logically (cover/metadata, personal data/review, actions)
  • Preserved functionality: Contributor chips and HTMX autocomplete remain intact (Lines 88-112)
  • Proper delete flow: The delete form (Line 221) is correctly wired to the confirmation modal via form_id="delete-form" (Line 228)
  • Comprehensive error handling: All form fields include dedicated error display labels

The layout is well-structured and should provide a better editing experience compared to the previous single-column approach.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (4)
src/templates/partials/media-score-stars.html (1)

1-14: Consider adding accessibility attributes to the rating container.

The star rating display looks correct, but consider enhancing accessibility:

🔎 Suggested accessibility improvements
 {% load i18n %}
 {# Display score with stars (read-only) #}
-<div class="flex items-center gap-3">
+<div class="flex items-center gap-3" role="img" aria-label="{% translate 'Rating:' %} {{ media.score }} {% translate 'out of 10' %}">
   {# Star rating display #}
   <div class="rating rating-lg">
src/core/templates/widgets/star_rating.html (1)

37-51: Remove redundant {% load heroicons %} tag.

Based on the project's global heroicons configuration, the {% load heroicons %} tag at line 44 is unnecessary and can be removed to avoid potential conflicts with globally registered tags.

🔎 Proposed cleanup
         {% if widget.value|add:0 == score %}
           <span class="badge badge-neutral gap-0">
             <span class="score-number">{{ score }}</span>
-            {% load heroicons %}
             {% heroicon_mini "star" class="fill-orange-400 h-4" %}
             -&nbsp;<span class="score-label">{{ label }}</span>
           </span>

Based on learnings, heroicons are globally available in all templates.

src/tests/core/test_views.py (1)

677-737: Remove unused db fixture parameters.

The db fixture on lines 700 and 722 is unnecessary because Django's ORM operations (Agent.objects.create, Media.objects.create) automatically trigger database access in pytest-django. You can safely remove these parameters.

🔎 Proposed fix
-    def test_media_detail_shows_all_fields(self, logged_in_client, db):
+    def test_media_detail_shows_all_fields(self, logged_in_client):
         """The detail view displays all media fields."""
-    def test_media_detail_contributor_links_to_filtered_list(self, logged_in_client, db):
+    def test_media_detail_contributor_links_to_filtered_list(self, logged_in_client):
         """Contributor links in detail view navigate to filtered home page."""

Based on static analysis hints.

src/templates/media_edit.html (1)

174-180: Consider externalizing the inline JavaScript handler.

The "Today" button functionality works correctly, but the inline onclick handler on line 175 could be moved to an external JavaScript file for better separation of concerns and maintainability.

🔎 Optional refactor

In a separate JavaScript file or in contributors.js:

document.addEventListener('DOMContentLoaded', () => {
  const todayBtn = document.querySelector('[data-set-today]');
  if (todayBtn) {
    todayBtn.addEventListener('click', () => {
      document.getElementById('id_review_date').value = new Date().toISOString().split('T')[0];
    });
  }
});

Then update the template:

-                    onclick="document.getElementById('id_review_date').value = new Date().toISOString().split('T')[0]"
                     class="btn btn-xs btn-ghost gap-1"
+                    data-set-today
                     title="{% translate "Set to today" %}">
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16d180a and 22fe68c.

📒 Files selected for processing (15)
  • src/core/templates/widgets/star_rating.html
  • src/core/urls.py
  • src/core/views.py
  • src/locale/fr/LC_MESSAGES/django.po
  • src/static/js/contributors.js
  • src/templates/accounts/profile_edit.html
  • src/templates/media_detail.html
  • src/templates/media_edit.html
  • src/templates/partials/media-contributors.html
  • src/templates/partials/media-cover.html
  • src/templates/partials/media-items.html
  • src/templates/partials/media-score-badge.html
  • src/templates/partials/media-score-stars.html
  • src/tests/core/test_views.py
  • src/theme/static_src/src/styles.css
💤 Files with no reviewable changes (1)
  • src/static/js/contributors.js
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2026-01-02T14:50:26.110Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 32
File: src/config/urls.py:21-31
Timestamp: 2026-01-02T14:50:26.110Z
Learning: In Django projects, avoid using django.views.static.serve in production (i.e., when DEBUG is False). Review any urls.py configurations that route static or media files through this view and replace with a proper static file server (e.g., nginx/Apache) or use a dedicated tool like WhiteNoise for production-ready static file handling. This guidance applies to all Django url configurations, not just a single file.

Applied to files:

  • src/core/urls.py
📚 Learning: 2025-12-27T18:00:03.595Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 23
File: src/templates/partials/score-readonly.html:2-33
Timestamp: 2025-12-27T18:00:03.595Z
Learning: In src/templates/partials/score-readonly.html, the DaisyUI read-only rating pattern correctly uses `aria-current="true"` on the selected star and applies the same color class (bg-orange-400) to all star elements. DaisyUI's CSS automatically handles which stars are visually filled based on the aria-current position.

Applied to files:

  • src/core/templates/widgets/star_rating.html
  • src/templates/partials/media-score-stars.html
📚 Learning: 2026-01-03T21:16:52.649Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-items.html:67-67
Timestamp: 2026-01-03T21:16:52.649Z
Learning: In HTML templates (e.g., src/templates/partials/media-items.html), Tailwind CSS v4.1.11 supports arbitrary numeric utilities (max-w-{n}, w-{n}, h-{n}) that map to calc(var(--spacing) * n). These should be considered valid Tailwind classes and not flagged as invalid. Ensure review tooling and linters treat such numeric utilities as allowed, and apply this guidance to other HTML/template files in the repository.

Applied to files:

  • src/core/templates/widgets/star_rating.html
  • src/templates/partials/media-cover.html
  • src/templates/media_detail.html
  • src/templates/media_edit.html
  • src/templates/partials/media-contributors.html
  • src/templates/accounts/profile_edit.html
  • src/templates/partials/media-score-stars.html
  • src/templates/partials/media-items.html
  • src/templates/partials/media-score-badge.html
📚 Learning: 2026-01-04T08:55:43.091Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-score-badge.html:1-9
Timestamp: 2026-01-04T08:55:43.091Z
Learning: In Django projects, configuring heroicons in TEMPLATES builtins makes heroicons.templatetags.heroicons available in all templates without needing {% load heroicons %}. For template reviews in this codebase, assume heroicon tags work globally in HTML templates under templates/, so avoid requiring explicit loads. If a template still uses {% load heroicons %}, assess whether the load is unnecessary and can be removed; ensure no conflicts arise from global tags.

Applied to files:

  • src/core/templates/widgets/star_rating.html
  • src/templates/partials/media-cover.html
  • src/templates/media_detail.html
  • src/templates/media_edit.html
  • src/templates/partials/media-contributors.html
  • src/templates/accounts/profile_edit.html
  • src/templates/partials/media-score-stars.html
  • src/templates/partials/media-items.html
  • src/templates/partials/media-score-badge.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, 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/partials/media-cover.html
  • src/templates/media_detail.html
  • src/templates/media_edit.html
  • src/templates/partials/media-contributors.html
  • src/templates/accounts/profile_edit.html
  • src/templates/partials/media-score-stars.html
  • src/templates/partials/media-items.html
  • src/templates/partials/media-score-badge.html
📚 Learning: 2026-01-03T21:16:52.649Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-items.html:67-67
Timestamp: 2026-01-03T21:16:52.649Z
Learning: The datakult project uses Tailwind CSS v4.1.11, which supports arbitrary numeric utility classes like max-w-{number}, w-{number}, h-{number}, etc., that map to calc(var(--spacing) * <number>). These numeric utilities are valid and should not be flagged as invalid Tailwind classes.

Applied to files:

  • src/theme/static_src/src/styles.css
🧬 Code graph analysis (3)
src/core/urls.py (1)
src/core/views.py (1)
  • media_detail (135-139)
src/templates/partials/media-cover.html (2)
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/tests/core/test_views.py (2)
src/tests/conftest.py (3)
  • logged_in_client (65-68)
  • media (20-31)
  • agent (12-16)
src/core/models.py (2)
  • Agent (94-105)
  • Media (108-226)
🪛 Ruff (0.14.10)
src/tests/core/test_views.py

700-700: Unused method argument: db

(ARG002)


722-722: Unused method argument: db

(ARG002)

🔇 Additional comments (20)
src/core/urls.py (1)

9-9: LGTM!

The new URL pattern for the media detail view is correctly defined and logically positioned before the edit route.

src/templates/partials/media-cover.html (1)

3-3: LGTM!

The hover and transition effects provide good visual feedback and improve the user experience.

src/core/views.py (2)

134-139: LGTM!

The new media detail view is correctly implemented with login protection and proper error handling via get_object_or_404.


174-174: LGTM!

Redirecting to the detail view after saving improves the user experience by showing the updated media item.

src/theme/static_src/src/styles.css (1)

31-34: LGTM!

The new .label-text-alt class provides appropriate styling for form validation errors, ensuring long error messages wrap properly and don't overflow their containers.

src/templates/accounts/profile_edit.html (1)

30-30: LGTM! Consistent error styling update.

The error message styling has been consistently updated across all form fields to use label-text-alt text-error classes, which aligns with DaisyUI form component patterns.

Also applies to: 40-40, 50-50, 61-61, 114-114, 125-125, 136-136

src/templates/partials/media-contributors.html (1)

1-21: LGTM! Conditional link rendering implemented correctly.

The use_htmx parameter with conditional rendering works as intended:

  • When use_htmx=False: renders normal links to the home page with filters
  • When use_htmx is not provided or True: renders HTMX-enabled links (default)

The logic correctly handles undefined/None values by defaulting to HTMX behavior.

src/core/templates/widgets/star_rating.html (1)

92-156: LGTM! Badge update logic is correctly implemented.

The updateLabelBadge function is properly called in all interaction scenarios:

  • Star click: displays selected rating
  • Hover: shows preview
  • Mouse leave: restores previous state
  • Clear button: removes rating

The event handling logic is sound and maintains consistent state.

src/templates/partials/media-items.html (2)

9-26: LGTM! Clickable media items implemented correctly.

The grid view properly implements multiple clickable areas:

  • Cover image wrapped in link with hover feedback
  • Title wrapped in link
  • Badges use pointer-events-none to avoid blocking clicks

All links point to the same media detail page, providing good UX with increased target areas.

Also applies to: 31-33


53-64: LGTM! List view clickability matches grid view pattern.

The list view consistently applies the same clickable pattern as the grid view, with cover and title both linking to the media detail page.

src/templates/partials/media-score-stars.html (1)

7-7: Handle null score values in template to prevent comparison errors.

The score field is constrained to 1-10 via choices in the Media model, but is optional (null=True, blank=True). The template comparison forloop.counter <= media.score will fail when score is null. Add a null check before the loop or conditional:

{% if media.score %}
  {% for i in "0123456789" %}
    <span class="mask mask-star-2 bg-orange-400" {% if forloop.counter <= media.score %}aria-current="true"{% endif %}></span>
  {% endfor %}
{% endif %}
⛔ Skipped due to learnings
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 23
File: src/templates/partials/score-readonly.html:2-33
Timestamp: 2025-12-27T18:00:03.595Z
Learning: In src/templates/partials/score-readonly.html, the DaisyUI read-only rating pattern correctly uses `aria-current="true"` on the selected star and applies the same color class (bg-orange-400) to all star elements. DaisyUI's CSS automatically handles which stars are visually filled based on the aria-current position.
src/templates/media_detail.html (3)

8-31: LGTM! Well-structured header with good accessibility.

The header section includes proper ARIA labels, breadcrumbs for navigation, and a clean layout. The conditional rendering of the publication year is a nice touch.


33-78: LGTM! Clean metadata presentation with appropriate fallbacks.

The cover card handles missing images gracefully with a placeholder icon. The metadata card conditionally renders contributors and external links, which is appropriate. The use_htmx=False parameter on line 63 correctly disables HTMX for contributor links in the detail view, as confirmed by the test expectations.


106-106: No action required—the |safe filter is appropriate here. The review_rendered field is properly sanitized by django-markdownfield's VALIDATOR_STANDARD, which uses bleach-based HTML filtering to prevent XSS attacks.

src/tests/core/test_views.py (1)

680-698: LGTM! Comprehensive test coverage for the new detail view.

The tests cover all essential scenarios: accessibility when logged in, correct template usage, 404 handling for nonexistent media, and proper rendering of media fields. This aligns well with the new detail view feature.

src/templates/media_edit.html (3)

19-53: LGTM! Improved header layout with better UX.

The new header design places the save button prominently at the top, improving usability. The breadcrumbs and back button provide clear navigation. The conditional rendering for "Edit" vs "Add media" is handled correctly.


84-112: LGTM! Contributors section with proper HTMX integration.

The contributors field is well-implemented with:

  • Visual chips for existing contributors
  • HTMX-powered autocomplete search with proper debouncing (300ms delay)
  • Clear error handling

The HTMX configuration on lines 100-103 correctly implements the search functionality that addresses the PR objective of fixing contributor addition issues.

Based on learnings regarding HTMX behavior in Django forms.


207-231: LGTM! Clean separation of delete functionality.

The delete action is well-implemented:

  • Conditionally rendered only when editing existing media (line 207)
  • Uses a separate hidden form (lines 223-230) linked via the form attribute
  • Includes appropriate confirmation dialog on line 213
  • Properly includes CSRF token

The overall refactor significantly improves the template's organization and maintainability.

src/locale/fr/LC_MESSAGES/django.po (2)

513-536: Review fuzzy translations with a native French speaker.

Lines 514 and 532 are marked as fuzzy (#, fuzzy), indicating the translator wasn't confident about these translations:

  1. Line 517: "Back to list" → "Retour à l'accueil" (literally "Back to home")

    • Consider if "Retour à la liste" would be more appropriate.
  2. Line 535: "External metadata" → "URI externe" (literally "External URI")

    • This may not fully convey the concept of "metadata". Consider "Métadonnées externes" or similar.

Please have a native French speaker review these translations to ensure they accurately convey the intended meaning.


554-561: LGTM! New translation entries for date functionality.

The translations for "Today" (Aujourd'hui) and "Set to today" (Définir à aujourd'hui) are accurate and align well with the new UI functionality in the media edit form.

Comment thread src/core/templates/widgets/star_rating.html
Comment thread src/templates/partials/media-score-badge.html Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (5)
src/static/js/media_edit.js (1)

40-40: Use English for code comments.

These French comments are inconsistent with the rest of the codebase, which uses English. For better maintainability in international teams, consider translating them.

🔎 Proposed translation
-    // Cherche dans les chips existants (depuis la base de données)
+    // Check existing chips (from database)
     const existingChips = Array.from(chips?.querySelectorAll('span[data-id], span[data-name]') || []).some(
       (chip) => (chip.dataset.name || chip.textContent || '').trim().toLowerCase() === lower,
     );
     
-    // Cherche dans les nouveaux contributeurs (créés dynamiquement)
+    // Check new contributors (created dynamically)
     const newContributors = Array.from(document.querySelectorAll('input[name="new_contributors"]')).some(
       (inp) => inp.value.trim().toLowerCase() === lower,
     );

Also applies to: 45-45

src/templates/backup_manage.html (1)

77-100: Modal integration looks good.

The modal-based confirmation flow improves UX over the inline JavaScript confirm dialog. The form validation logic correctly checks validity before opening the modal.

The inline onclick validation could be moved to a separate function for better readability:

🔎 Optional refactor for clarity

Add to the script section:

function validateAndConfirmImport() {
  const form = document.getElementById('import-backup-form');
  if (form.checkValidity()) {
    document.getElementById('confirm-import-modal').checked = true;
  } else {
    form.reportValidity();
  }
}

Then simplify the button:

-              <button type="button"
-                      onclick="this.form.checkValidity() && (document.getElementById('confirm-import-modal').checked = true) || this.form.reportValidity()"
-                      class="btn btn-warning">
+              <button type="button"
+                      onclick="validateAndConfirmImport()"
+                      class="btn btn-warning">
src/core/templates/widgets/star_rating.html (2)

44-45: Consider removing the heroicons load directive.

Based on learnings, heroicons are configured as builtins in this project's TEMPLATES settings, making them globally available. The {% load heroicons %} at line 44 may be unnecessary.


77-90: Inline SVG in JavaScript creates maintenance burden.

The JavaScript function uses a hardcoded inline SVG (lines 82-84) while the template uses the {% heroicon_mini "star" %} tag (line 45). If the heroicon library is updated or the star design changes, only the template will benefit—the JavaScript SVG must be manually synchronized.

Possible approaches to reduce duplication

Option 1: Extract the rendered heroicon SVG into a reusable constant or template that both paths can reference.

Option 2: Use a data attribute or hidden template element to store the icon markup, then clone it in JavaScript rather than hardcoding the SVG.

Option 3: Accept the duplication if the icon is stable and document the need to update both locations if the icon changes.

src/tests/core/test_views.py (1)

700-700: Unused db fixture parameter (nitpick).

The db parameter in these test methods is not directly referenced in the function body. While this is a common pytest-django pattern (the fixture ensures database access is available), you could remove the explicit parameter since the database is already initialized via the agent and media objects being created.

Example cleanup

The static analyzer flags these as unused because pytest-django automatically provides database access when you create model instances. You can safely remove the db parameter from both method signatures if you prefer cleaner signatures, or keep it for explicitness—both approaches work.

Also applies to: 722-722

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 22fe68c and cc4c2cd.

📒 Files selected for processing (17)
  • src/core/templates/widgets/star_rating.html
  • src/core/urls.py
  • src/core/views.py
  • src/locale/fr/LC_MESSAGES/django.po
  • src/static/js/media_edit.js
  • src/templates/accounts/profile_edit.html
  • src/templates/backup_manage.html
  • src/templates/media_detail.html
  • src/templates/media_edit.html
  • src/templates/partials/confirm-modal.html
  • src/templates/partials/media-contributors.html
  • src/templates/partials/media-cover.html
  • src/templates/partials/media-items.html
  • src/templates/partials/media-score-badge.html
  • src/templates/partials/media-score-stars.html
  • src/tests/core/test_views.py
  • src/theme/static_src/src/styles.css
✅ Files skipped from review due to trivial changes (1)
  • src/templates/accounts/profile_edit.html
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/templates/partials/media-score-badge.html
  • src/templates/media_detail.html
  • src/templates/partials/media-score-stars.html
  • src/core/views.py
  • src/core/urls.py
  • src/theme/static_src/src/styles.css
  • src/templates/partials/media-contributors.html
🧰 Additional context used
🧠 Learnings (4)
📚 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/partials/media-items.html
  • src/templates/media_edit.html
  • src/templates/partials/confirm-modal.html
  • src/templates/partials/media-cover.html
  • src/templates/backup_manage.html
📚 Learning: 2026-01-03T21:16:52.649Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-items.html:67-67
Timestamp: 2026-01-03T21:16:52.649Z
Learning: In HTML templates (e.g., src/templates/partials/media-items.html), Tailwind CSS v4.1.11 supports arbitrary numeric utilities (max-w-{n}, w-{n}, h-{n}) that map to calc(var(--spacing) * n). These should be considered valid Tailwind classes and not flagged as invalid. Ensure review tooling and linters treat such numeric utilities as allowed, and apply this guidance to other HTML/template files in the repository.

Applied to files:

  • src/templates/partials/media-items.html
  • src/templates/media_edit.html
  • src/core/templates/widgets/star_rating.html
  • src/templates/partials/confirm-modal.html
  • src/templates/partials/media-cover.html
  • src/templates/backup_manage.html
📚 Learning: 2026-01-04T08:55:43.091Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-score-badge.html:1-9
Timestamp: 2026-01-04T08:55:43.091Z
Learning: In Django projects, configuring heroicons in TEMPLATES builtins makes heroicons.templatetags.heroicons available in all templates without needing {% load heroicons %}. For template reviews in this codebase, assume heroicon tags work globally in HTML templates under templates/, so avoid requiring explicit loads. If a template still uses {% load heroicons %}, assess whether the load is unnecessary and can be removed; ensure no conflicts arise from global tags.

Applied to files:

  • src/templates/partials/media-items.html
  • src/templates/media_edit.html
  • src/core/templates/widgets/star_rating.html
  • src/templates/partials/confirm-modal.html
  • src/templates/partials/media-cover.html
  • src/templates/backup_manage.html
📚 Learning: 2025-12-27T18:00:03.595Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 23
File: src/templates/partials/score-readonly.html:2-33
Timestamp: 2025-12-27T18:00:03.595Z
Learning: In src/templates/partials/score-readonly.html, the DaisyUI read-only rating pattern correctly uses `aria-current="true"` on the selected star and applies the same color class (bg-orange-400) to all star elements. DaisyUI's CSS automatically handles which stars are visually filled based on the aria-current position.

Applied to files:

  • src/core/templates/widgets/star_rating.html
🧬 Code graph analysis (5)
src/static/js/media_edit.js (1)
src/static/js/contributors.js (8)
  • input (2-139)
  • evt (67-85)
  • evt (124-138)
  • evt (103-120)
  • evt (89-94)
  • suggestions (23-23)
  • evt (96-101)
  • name (42-65)
src/templates/media_edit.html (3)
src/core/migrations/0003_media_review_media_review_date_media_score_and_more.py (1)
  • Migration (9-41)
src/core/forms.py (1)
  • MediaForm (34-90)
src/core/migrations/0008_media_review_rendered_alter_media_review.py (1)
  • Migration (24-43)
src/tests/core/test_views.py (2)
src/tests/conftest.py (3)
  • logged_in_client (65-68)
  • media (20-31)
  • agent (12-16)
src/core/models.py (2)
  • Agent (94-105)
  • Media (108-226)
src/core/templates/widgets/star_rating.html (1)
src/static/js/base.js (2)
  • e (125-163)
  • createFilterBadge (65-84)
src/templates/partials/media-cover.html (1)
src/core/migrations/0005_media_cover_alter_media_score.py (1)
  • Migration (6-23)
🪛 Ruff (0.14.10)
src/tests/core/test_views.py

700-700: Unused method argument: db

(ARG002)


722-722: Unused method argument: db

(ARG002)

🔇 Additional comments (9)
src/templates/partials/media-cover.html (1)

3-3: LGTM!

The hover effect enhances the visual feedback when media covers become interactive links to the detail view. The transition is smooth and the opacity change is subtle.

src/static/js/media_edit.js (2)

1-11: LGTM!

The "Set to today" button handler is correctly implemented with appropriate guard checks and proper date formatting.


78-89: Excellent fix for the contributor addition issue!

Removing the automatic click on the first suggestion prevents unwanted auto-selection when adding contributors with names that start similarly to existing ones. Users now have explicit control over whether to select from suggestions or create a new contributor.

src/templates/backup_manage.html (1)

154-155: LGTM!

The confirmation modal is properly integrated with all required parameters. The form_id binding ensures the modal's submit button will trigger the import form submission.

src/templates/partials/media-items.html (2)

9-26: LGTM!

The clickable cover implementation is well done. Wrapping the cover in a link to the detail view and using pointer-events-none on badges correctly prevents badge overlays from interfering with link clicks.


31-33: LGTM!

The title and cover links are consistently implemented across both grid and list views. Providing multiple clickable areas (cover and title) for the same destination enhances UX by giving users more target area.

Also applies to: 53-55, 59-64

src/templates/partials/confirm-modal.html (2)

1-16: LGTM with a note on the safe filter.

The modal structure is well-implemented with proper i18n support. The message|safe filter is necessary for rendering HTML formatting in messages (like the warning in backup_manage.html). Current usage with hardcoded translated strings is secure.

Ensure that future uses of this modal never pass user-generated content as the message parameter, as the |safe filter would expose XSS vulnerabilities.


17-24: LGTM!

The conditional rendering correctly handles both form-bound submissions (when form_id is provided) and standalone scenarios. When no form_id is given, the confirm button receives a predictable id ({{ modal_id }}-confirm) allowing callers to attach custom event handlers.

src/templates/media_edit.html (1)

16-229: LGTM! Well-structured UI refactor.

The new three-column responsive layout with dedicated header, breadcrumbs, and card-based sections significantly improves the editing experience. The conditional delete flow with modal confirmation is clean and follows good UX patterns.

Comment thread src/locale/fr/LC_MESSAGES/django.po Outdated
Comment thread src/locale/fr/LC_MESSAGES/django.po Outdated
- Enhance the media detail view and editor with better UI elements.
- Fix adding contributors beginning the same as existing ones.
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