Merge pull request 'Fix Wagtail article publish regressions' (#58) from fix/issue-57-admin-publish-regressions into main

Reviewed-on: #58

apps/blog/models.py

@@ -222,7 +222,24 @@ class ArticlePageAdminForm(WagtailAdminPageForm):
             self.initial["category"] = default_category.pk
 
     def clean(self):
+        cleaned_data = getattr(self, "cleaned_data", {})
+        self._apply_defaults(cleaned_data)
+        self.cleaned_data = cleaned_data
+
         cleaned_data = super().clean()
+        self._apply_defaults(cleaned_data)
+
+        if not cleaned_data.get("slug"):
+            self.add_error("slug", "Slug is required.")
+        if not cleaned_data.get("author"):
+            self.add_error("author", "Author is required.")
+        if not cleaned_data.get("category"):
+            self.add_error("category", "Category is required.")
+        if not cleaned_data.get("summary"):
+            self.add_error("summary", "Summary is required.")
+        return cleaned_data
+
+    def _apply_defaults(self, cleaned_data: dict[str, Any]) -> dict[str, Any]:
         title = (cleaned_data.get("title") or "").strip()
 
         if not cleaned_data.get("slug") and title:
@@ -237,14 +254,6 @@ class ArticlePageAdminForm(WagtailAdminPageForm):
                 max_chars=self.SUMMARY_MAX_CHARS,
             ) or title
 
-        if not cleaned_data.get("slug"):
-            self.add_error("slug", "Slug is required.")
-        if not cleaned_data.get("author"):
-            self.add_error("author", "Author is required.")
-        if not cleaned_data.get("category"):
-            self.add_error("category", "Category is required.")
-        if not cleaned_data.get("summary"):
-            self.add_error("summary", "Summary is required.")
         return cleaned_data
 
     def _get_default_author(self, *, create: bool) -> Author | None:
@@ -374,9 +383,20 @@ class ArticlePage(SeoMixin, Page):
             self.summary = _generate_summary_from_stream(self.body) or self.title
         if not self.published_date and self.first_published_at:
             self.published_date = self.first_published_at
+        if self._should_refresh_read_time():
             self.read_time_mins = self._compute_read_time()
         return super().save(*args, **kwargs)
 
+    def _should_refresh_read_time(self) -> bool:
+        if not self.pk:
+            return True
+
+        previous = type(self).objects.only("body").filter(pk=self.pk).first()
+        if previous is None:
+            return True
+
+        return previous.body_text != self.body_text
+
     def _compute_read_time(self) -> int:
         words = []
         for block in self.body:

apps/blog/tests/test_admin_experience.py

@@ -2,6 +2,9 @@ from datetime import timedelta
 from types import SimpleNamespace
 
 import pytest
+from django.contrib import messages
+from django.contrib.messages.storage.fallback import FallbackStorage
+from django.contrib.sessions.middleware import SessionMiddleware
 from django.test import override_settings
 from django.utils import timezone
 
@@ -295,7 +298,7 @@ def test_article_admin_form_relaxes_initial_required_fields(article_index, djang
 
 @pytest.mark.django_db
 def test_article_admin_form_clean_applies_defaults(article_index, django_user_model, monkeypatch):
-    """Form clean should auto-fill slug/author/category/summary when omitted."""
+    """Form clean should populate defaults before parent validation runs."""
     user = django_user_model.objects.create_user(
         username="writer",
         email="writer@example.com",
@@ -310,9 +313,7 @@ def test_article_admin_form_clean_applies_defaults(article_index, django_user_mo
         SimpleNamespace(block_type="code", value=SimpleNamespace(raw_code="print('ignore')")),
         SimpleNamespace(block_type="rich_text", value=SimpleNamespace(source="<p>Hello world body text.</p>")),
     ]
-
+    form.cleaned_data = {
-    def fake_super_clean(_self):
-        _self.cleaned_data = {
         "title": "Auto Defaults Title",
         "slug": "",
         "author": None,
@@ -320,6 +321,10 @@ def test_article_admin_form_clean_applies_defaults(article_index, django_user_mo
         "summary": "",
         "body": body,
     }
+    observed = {}
+
+    def fake_super_clean(_self):
+        observed["slug_before_parent_clean"] = _self.cleaned_data.get("slug")
         return _self.cleaned_data
 
     mro = form.__class__.__mro__
@@ -327,6 +332,7 @@ def test_article_admin_form_clean_applies_defaults(article_index, django_user_mo
     monkeypatch.setattr(super_form_class, "clean", fake_super_clean)
     cleaned = form.clean()
 
+    assert observed["slug_before_parent_clean"] == "auto-defaults-title"
     assert cleaned["slug"] == "auto-defaults-title"
     assert cleaned["author"] is not None
     assert cleaned["author"].user_id == user.id
@@ -378,3 +384,20 @@ def test_article_save_autogenerates_summary_when_missing(article_index):
     article.save()
 
     assert article.summary == "This should become the summary text."
+
+
+@pytest.mark.django_db
+def test_article_page_omits_admin_messages_on_frontend(article_page, rf):
+    """Frontend templates should not render admin session messages."""
+    request = rf.get(article_page.url)
+    SessionMiddleware(lambda req: None).process_request(request)
+    request.session.save()
+    setattr(request, "_messages", FallbackStorage(request))
+    messages.success(request, "Page 'Test' has been published.")
+
+    response = article_page.serve(request)
+    response.render()
+    content = response.content.decode()
+
+    assert "Page 'Test' has been published." not in content
+    assert 'aria-label="Messages"' not in content

apps/blog/tests/test_models.py

@@ -59,6 +59,32 @@ def test_article_default_category_is_assigned(home_page):
     assert article.category.slug == "general"
 
 
+@pytest.mark.django_db
+def test_article_read_time_is_not_recomputed_when_body_text_is_unchanged(home_page, monkeypatch):
+    index = ArticleIndexPage(title="Articles", slug="articles")
+    home_page.add_child(instance=index)
+    author = AuthorFactory()
+    article = ArticlePage(
+        title="Stable read time",
+        slug="stable-read-time",
+        author=author,
+        summary="s",
+        body=[("rich_text", "<p>body words</p>")],
+    )
+    index.add_child(instance=article)
+    article.save()
+
+    def fail_compute():
+        raise AssertionError("read time should not be recomputed when body text is unchanged")
+
+    monkeypatch.setattr(article, "_compute_read_time", fail_compute)
+    article.title = "Retitled"
+    article.save()
+    article.refresh_from_db()
+
+    assert article.read_time_mins == 1
+
+
 @pytest.mark.django_db
 def test_category_ordering():
     Category.objects.get_or_create(name="General", slug="general")

apps/comments/tests/test_v2.py

@@ -149,7 +149,7 @@ def test_non_htmx_post_still_redirects(client, _article):
     """Non-HTMX POST continues to redirect (progressive enhancement)."""
     resp = _post_comment(client, _article)
     assert resp.status_code == 302
-    assert resp["Location"].endswith("?commented=1")
+    assert resp["Location"].endswith("?commented=pending")
 
 
 @pytest.mark.django_db

apps/comments/tests/test_views.py

@@ -1,3 +1,5 @@
+from unittest.mock import patch
+
 import pytest
 from django.core.cache import cache
 from django.test import override_settings
@@ -28,10 +30,64 @@ def test_comment_post_flow(client, home_page):
         },
     )
     assert resp.status_code == 302
-    assert resp["Location"].endswith("?commented=1")
+    assert resp["Location"].endswith("?commented=pending")
     assert Comment.objects.count() == 1
 
 
+@pytest.mark.django_db
+def test_comment_post_redirect_banner_renders_on_article_page(client, home_page):
+    cache.clear()
+    index = ArticleIndexPage(title="Articles", slug="articles")
+    home_page.add_child(instance=index)
+    author = AuthorFactory()
+    article = ArticlePage(title="A", slug="a", author=author, summary="s", body=[("rich_text", "<p>body</p>")])
+    index.add_child(instance=article)
+    article.save_revision().publish()
+
+    resp = client.post(
+        "/comments/post/",
+        {
+            "article_id": article.id,
+            "author_name": "Test",
+            "author_email": "test@example.com",
+            "body": "Hello",
+            "honeypot": "",
+        },
+        follow=True,
+    )
+    assert resp.status_code == 200
+    assert b"Your comment has been posted and is awaiting moderation." in resp.content
+
+
+@pytest.mark.django_db
+@override_settings(TURNSTILE_SECRET_KEY="test-secret")
+def test_comment_post_redirect_banner_renders_approved_state(client, home_page):
+    cache.clear()
+    index = ArticleIndexPage(title="Articles", slug="articles")
+    home_page.add_child(instance=index)
+    author = AuthorFactory()
+    article = ArticlePage(title="A", slug="a", author=author, summary="s", body=[("rich_text", "<p>body</p>")])
+    index.add_child(instance=article)
+    article.save_revision().publish()
+
+    with patch("apps.comments.views._verify_turnstile", return_value=True):
+        resp = client.post(
+            "/comments/post/",
+            {
+                "article_id": article.id,
+                "author_name": "Test",
+                "author_email": "test@example.com",
+                "body": "Hello",
+                "honeypot": "",
+                "cf-turnstile-response": "tok",
+            },
+            follow=True,
+        )
+
+    assert resp.status_code == 200
+    assert b"Comment posted!" in resp.content
+
+
 @pytest.mark.django_db
 def test_comment_post_rejected_when_comments_disabled(client, home_page):
     cache.clear()

apps/comments/views.py

@@ -4,7 +4,6 @@ import logging
 
 import requests as http_requests
 from django.conf import settings
-from django.contrib import messages
 from django.core.cache import cache
 from django.core.exceptions import ValidationError
 from django.db import IntegrityError
@@ -41,6 +40,11 @@ def _add_vary_header(response):
     return response
 
 
+def _comment_redirect(article: ArticlePage, *, approved: bool):
+    state = "approved" if approved else "pending"
+    return redirect(f"{article.url}?commented={state}")
+
+
 def _verify_turnstile(token: str, ip: str) -> bool:
     secret = getattr(settings, "TURNSTILE_SECRET_KEY", "")
     if not secret:
@@ -201,7 +205,7 @@ class CommentCreateView(View):
                     return _add_vary_header(
                         render(request, "comments/_comment_success.html", {"message": "Comment posted!"})
                     )
-                return redirect(f"{article.url}?commented=1")
+                return _comment_redirect(article, approved=True)
 
             # Turnstile verification
             turnstile_ok = False
@@ -230,11 +234,7 @@ class CommentCreateView(View):
             if _is_htmx(request):
                 return self._render_htmx_success(request, article, comment)
 
-            messages.success(
+            return _comment_redirect(article, approved=comment.is_approved)
-                request,
-                "Comment posted!" if comment.is_approved else "Your comment is awaiting moderation",
-            )
-            return redirect(f"{article.url}?commented=1")
 
         if _is_htmx(request):
             return self._render_htmx_error(request, article, form)

templates/base.html

@@ -26,13 +26,6 @@
     <div class="fixed inset-0 bg-grid-pattern pointer-events-none z-[-1]"></div>
     {% include 'components/nav.html' %}
     {% include 'components/cookie_banner.html' %}
-    {% if messages %}
-      <section aria-label="Messages" class="max-w-7xl mx-auto px-6 py-2">
-        {% for message in messages %}
-          <p class="font-mono text-sm py-2 px-4 bg-brand-cyan/10 text-brand-cyan border border-brand-cyan/20 mb-2">{{ message }}</p>
-        {% endfor %}
-      </section>
-    {% endif %}
     <main class="flex-grow w-full max-w-7xl mx-auto px-6 py-8">{% block content %}{% endblock %}</main>
     {% include 'components/footer.html' %}
   </body>

templates/blog/article_page.html

@@ -145,6 +145,15 @@
   <p class="mt-2 mb-6 font-mono text-xs uppercase tracking-wider text-zinc-500">
     {{ approved_comments|length }} public comment{{ approved_comments|length|pluralize }}
   </p>
+  {% if request.GET.commented %}
+  <div class="mb-6 rounded-md border border-brand-cyan/20 bg-brand-cyan/10 px-4 py-3 font-mono text-sm text-brand-cyan">
+    {% if request.GET.commented == "approved" %}
+    Comment posted!
+    {% else %}
+    Your comment has been posted and is awaiting moderation.
+    {% endif %}
+  </div>
+  {% endif %}
 
   {% include "comments/_comment_list.html" %}
   <div id="comments-empty-state" class="mb-8 rounded-md border border-zinc-200 bg-zinc-50 p-4 text-center dark:border-zinc-800 dark:bg-zinc-900/40 {% if approved_comments %}hidden{% endif %}">