fix: resolve 5 PR review blockers for comments v2
Some checks failed
CI / nightly-e2e (pull_request) Has been skipped
CI / deploy (pull_request) Has been skipped
CI / pr-e2e (pull_request) Failing after 2m7s
CI / ci (pull_request) Failing after 2m43s

1. Reply HTMX target: server sends HX-Retarget/HX-Reswap headers to
   insert replies inside parent comment's .replies-container div
2. Empty thread swap target: always render #comments-list container
   even when no approved comments exist
3. Reaction hydration: add _annotate_reaction_counts() helper that
   hydrates reaction_counts and user_reacted on comments in
   get_context(), comment_poll(), and single-comment responses
4. HTMX error swap: return 200 instead of 422 for form errors since
   HTMX 2 doesn't swap 4xx responses by default
5. Vary header: use patch_vary_headers() instead of direct assignment
   to avoid overwriting existing Vary directives

Also fixes _get_session_key() to handle missing session attribute
(e.g. from RequestFactory in performance tests).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Mark
2026-03-03 23:24:20 +00:00
parent a118df487d
commit 88ce59aecc
5 changed files with 75 additions and 12 deletions

View File

@@ -306,11 +306,16 @@ class ArticlePage(SeoMixin, Page):
from django.conf import settings from django.conf import settings
from apps.comments.models import Comment from apps.comments.models import Comment
from apps.comments.views import _annotate_reaction_counts, _get_session_key
approved_replies = Comment.objects.filter(is_approved=True).select_related("parent") approved_replies = Comment.objects.filter(is_approved=True).select_related("parent")
ctx["approved_comments"] = self.comments.filter(is_approved=True, parent__isnull=True).prefetch_related( comments = list(
Prefetch("replies", queryset=approved_replies) self.comments.filter(is_approved=True, parent__isnull=True).prefetch_related(
Prefetch("replies", queryset=approved_replies)
)
) )
_annotate_reaction_counts(comments, _get_session_key(request))
ctx["approved_comments"] = comments
ctx["turnstile_site_key"] = getattr(settings, "TURNSTILE_SITE_KEY", "") ctx["turnstile_site_key"] = getattr(settings, "TURNSTILE_SITE_KEY", "")
return ctx return ctx

View File

@@ -90,14 +90,14 @@ def test_htmx_post_returns_comment_partial_when_turnstile_passes(client, _articl
@pytest.mark.django_db @pytest.mark.django_db
def test_htmx_post_returns_form_with_errors_on_invalid(client, _article): def test_htmx_post_returns_form_with_errors_on_invalid(client, _article):
"""HTMX POST with invalid data returns form partial with HTTP 422.""" """HTMX POST with invalid data returns form partial with HTTP 200 (HTMX 2 requires 2xx for swap)."""
cache.clear() cache.clear()
resp = client.post( resp = client.post(
"/comments/post/", "/comments/post/",
{"article_id": _article.id, "author_name": "T", "author_email": "t@t.com", "body": " ", "honeypot": ""}, {"article_id": _article.id, "author_name": "T", "author_email": "t@t.com", "body": " ", "honeypot": ""},
HTTP_HX_REQUEST="true", HTTP_HX_REQUEST="true",
) )
assert resp.status_code == 422 assert resp.status_code == 200
assert "HX-Request" in resp["Vary"] assert "HX-Request" in resp["Vary"]
assert Comment.objects.count() == 0 assert Comment.objects.count() == 0

View File

@@ -8,9 +8,10 @@ from django.contrib import messages
from django.core.cache import cache from django.core.cache import cache
from django.core.exceptions import ValidationError from django.core.exceptions import ValidationError
from django.db import IntegrityError from django.db import IntegrityError
from django.db.models import Prefetch from django.db.models import Count, Prefetch
from django.http import HttpResponse, JsonResponse from django.http import HttpResponse, JsonResponse
from django.shortcuts import get_object_or_404, redirect, render from django.shortcuts import get_object_or_404, redirect, render
from django.utils.cache import patch_vary_headers
from django.views import View from django.views import View
from django.views.decorators.http import require_GET, require_POST from django.views.decorators.http import require_GET, require_POST
@@ -35,7 +36,7 @@ def _is_htmx(request) -> bool:
def _add_vary_header(response): def _add_vary_header(response):
response["Vary"] = "HX-Request" patch_vary_headers(response, ["HX-Request"])
return response return response
@@ -66,12 +67,61 @@ def _turnstile_enabled() -> bool:
return bool(getattr(settings, "TURNSTILE_SECRET_KEY", "")) return bool(getattr(settings, "TURNSTILE_SECRET_KEY", ""))
def _get_session_key(request) -> str:
session = getattr(request, "session", None)
return (session.session_key or "") if session else ""
def _annotate_reaction_counts(comments, session_key: str = ""):
"""Hydrate each comment with reaction_counts dict and user_reacted set."""
comment_ids = [c.id for c in comments]
if not comment_ids:
return comments
# Aggregate counts per comment per type
counts_qs = (
CommentReaction.objects.filter(comment_id__in=comment_ids)
.values("comment_id", "reaction_type")
.annotate(count=Count("id"))
)
counts_map: dict[int, dict[str, int]] = {}
for row in counts_qs:
counts_map.setdefault(row["comment_id"], {"heart": 0, "plus_one": 0})
counts_map[row["comment_id"]][row["reaction_type"]] = row["count"]
# User's own reactions
user_map: dict[int, set[str]] = {}
if session_key:
user_qs = CommentReaction.objects.filter(
comment_id__in=comment_ids, session_key=session_key
).values_list("comment_id", "reaction_type")
for cid, rtype in user_qs:
user_map.setdefault(cid, set()).add(rtype)
for comment in comments:
comment.reaction_counts = counts_map.get(comment.id, {"heart": 0, "plus_one": 0})
comment.user_reacted = user_map.get(comment.id, set())
return comments
def _comment_template_context(comment, article, request):
"""Build template context for a single comment partial."""
session_key = _get_session_key(request)
_annotate_reaction_counts([comment], session_key)
return {
"comment": comment,
"page": article,
"turnstile_site_key": getattr(settings, "TURNSTILE_SITE_KEY", ""),
}
class CommentCreateView(View): class CommentCreateView(View):
def _render_article_with_errors(self, request, article, form): def _render_article_with_errors(self, request, article, form):
if _is_htmx(request): if _is_htmx(request):
ctx = {"comment_form": form, "page": article} ctx = {"comment_form": form, "page": article}
ctx["turnstile_site_key"] = getattr(settings, "TURNSTILE_SITE_KEY", "") ctx["turnstile_site_key"] = getattr(settings, "TURNSTILE_SITE_KEY", "")
resp = render(request, "comments/_comment_form.html", ctx, status=422) resp = render(request, "comments/_comment_form.html", ctx, status=200)
return _add_vary_header(resp) return _add_vary_header(resp)
context = article.get_context(request) context = article.get_context(request)
context["page"] = article context["page"] = article
@@ -122,11 +172,13 @@ class CommentCreateView(View):
comment.save() comment.save()
if _is_htmx(request): if _is_htmx(request):
ctx = _comment_template_context(comment, article, request)
if comment.is_approved: if comment.is_approved:
resp = render(request, "comments/_comment.html", { resp = render(request, "comments/_comment.html", ctx)
"comment": comment, "page": article, if comment.parent_id:
"turnstile_site_key": getattr(settings, "TURNSTILE_SITE_KEY", ""), # Tell HTMX to retarget: insert reply inside parent comment
}) resp["HX-Retarget"] = f"#comment-{comment.parent_id} .replies-container"
resp["HX-Reswap"] = "beforeend"
else: else:
resp = render(request, "comments/_comment_success.html", { resp = render(request, "comments/_comment_success.html", {
"message": "Your comment has been posted and is awaiting moderation.", "message": "Your comment has been posted and is awaiting moderation.",
@@ -153,12 +205,15 @@ def comment_poll(request, article_id):
after_id = 0 after_id = 0
approved_replies = Comment.objects.filter(is_approved=True).select_related("parent") approved_replies = Comment.objects.filter(is_approved=True).select_related("parent")
comments = ( comments = list(
article.comments.filter(is_approved=True, parent__isnull=True, id__gt=after_id) article.comments.filter(is_approved=True, parent__isnull=True, id__gt=after_id)
.prefetch_related(Prefetch("replies", queryset=approved_replies)) .prefetch_related(Prefetch("replies", queryset=approved_replies))
.order_by("created_at", "id") .order_by("created_at", "id")
) )
session_key = _get_session_key(request)
_annotate_reaction_counts(comments, session_key)
resp = render(request, "comments/_comment_list_inner.html", { resp = render(request, "comments/_comment_list_inner.html", {
"approved_comments": comments, "approved_comments": comments,
"page": article, "page": article,

View File

@@ -146,6 +146,7 @@
{% if approved_comments %} {% if approved_comments %}
{% include "comments/_comment_list.html" %} {% include "comments/_comment_list.html" %}
{% else %} {% else %}
<div id="comments-list" class="space-y-8 mb-12"></div>
<div class="mb-12 p-8 bg-grid-pattern text-center"> <div class="mb-12 p-8 bg-grid-pattern text-center">
<p class="font-mono text-sm text-zinc-500">No comments yet. Be the first to comment.</p> <p class="font-mono text-sm text-zinc-500">No comments yet. Be the first to comment.</p>
</div> </div>

View File

@@ -8,6 +8,7 @@
</div> </div>
<p class="text-zinc-700 dark:text-zinc-300 text-sm leading-relaxed">{{ comment.body }}</p> <p class="text-zinc-700 dark:text-zinc-300 text-sm leading-relaxed">{{ comment.body }}</p>
{% include "comments/_reactions.html" with comment=comment counts=comment.reaction_counts user_reacted=comment.user_reacted %} {% include "comments/_reactions.html" with comment=comment counts=comment.reaction_counts user_reacted=comment.user_reacted %}
<div class="replies-container">
{% for reply in comment.replies.all %} {% for reply in comment.replies.all %}
<article id="comment-{{ reply.id }}" class="mt-6 ml-8 bg-zinc-50 dark:bg-zinc-900 border border-zinc-200 dark:border-zinc-800 border-l-2 border-l-brand-cyan p-4"> <article id="comment-{{ reply.id }}" class="mt-6 ml-8 bg-zinc-50 dark:bg-zinc-900 border border-zinc-200 dark:border-zinc-800 border-l-2 border-l-brand-cyan p-4">
<div class="flex items-center gap-3 mb-2"> <div class="flex items-center gap-3 mb-2">
@@ -20,5 +21,6 @@
<p class="text-zinc-700 dark:text-zinc-300 text-sm leading-relaxed">{{ reply.body }}</p> <p class="text-zinc-700 dark:text-zinc-300 text-sm leading-relaxed">{{ reply.body }}</p>
</article> </article>
{% endfor %} {% endfor %}
</div>
{% include "comments/_reply_form.html" with page=page comment=comment %} {% include "comments/_reply_form.html" with page=page comment=comment %}
</article> </article>