fix(editor): auto-default article metadata and de-duplicate SEO panels
This commit is contained in:
@@ -8,9 +8,12 @@ from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator
|
||||
from django.db import models
|
||||
from django.db.models import CASCADE, PROTECT, SET_NULL, Prefetch
|
||||
from django.shortcuts import get_object_or_404
|
||||
from django.utils.html import strip_tags
|
||||
from django.utils.text import slugify
|
||||
from modelcluster.contrib.taggit import ClusterTaggableManager
|
||||
from modelcluster.fields import ParentalKey
|
||||
from taggit.models import Tag, TaggedItemBase
|
||||
from wagtail.admin.forms.pages import WagtailAdminPageForm
|
||||
from wagtail.admin.panels import FieldPanel, ObjectList, PageChooserPanel, TabbedInterface
|
||||
from wagtail.contrib.routable_page.models import RoutablePageMixin, route
|
||||
from wagtail.fields import RichTextField, StreamField
|
||||
@@ -18,9 +21,29 @@ from wagtail.models import Page
|
||||
from wagtail.search import index
|
||||
from wagtailseo.models import SeoMixin
|
||||
|
||||
from apps.authors.models import Author
|
||||
from apps.blog.blocks import ARTICLE_BODY_BLOCKS
|
||||
|
||||
|
||||
def _generate_summary_from_stream(body: Any, *, max_chars: int = 220) -> str:
|
||||
parts: list[str] = []
|
||||
if body is None:
|
||||
return ""
|
||||
for block in body:
|
||||
if getattr(block, "block_type", None) == "code":
|
||||
continue
|
||||
value = getattr(block, "value", block)
|
||||
text = value.source if hasattr(value, "source") else str(value)
|
||||
clean_text = strip_tags(text)
|
||||
if clean_text:
|
||||
parts.append(clean_text)
|
||||
summary = re.sub(r"\s+", " ", " ".join(parts)).strip()
|
||||
if len(summary) <= max_chars:
|
||||
return summary
|
||||
truncated = summary[:max_chars].rsplit(" ", 1)[0].strip()
|
||||
return truncated or summary[:max_chars].strip()
|
||||
|
||||
|
||||
class HomePage(Page):
|
||||
featured_article = models.ForeignKey(
|
||||
"blog.ArticlePage", null=True, blank=True, on_delete=SET_NULL, related_name="+"
|
||||
@@ -181,6 +204,93 @@ class TagMetadata(models.Model):
|
||||
return mapping.get(self.colour, self.get_fallback_css())
|
||||
|
||||
|
||||
class ArticlePageAdminForm(WagtailAdminPageForm):
|
||||
SUMMARY_MAX_CHARS = 220
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
super().__init__(*args, **kwargs)
|
||||
for name in ("slug", "author", "category", "summary"):
|
||||
if name in self.fields:
|
||||
self.fields[name].required = False
|
||||
|
||||
default_author = self._get_default_author(create=False)
|
||||
if default_author and not self.initial.get("author"):
|
||||
self.initial["author"] = default_author.pk
|
||||
|
||||
default_category = self._get_default_category(create=False)
|
||||
if default_category and not self.initial.get("category"):
|
||||
self.initial["category"] = default_category.pk
|
||||
|
||||
def clean(self):
|
||||
cleaned_data = super().clean()
|
||||
title = (cleaned_data.get("title") or "").strip()
|
||||
|
||||
if not cleaned_data.get("slug") and title:
|
||||
cleaned_data["slug"] = self._build_unique_page_slug(title)
|
||||
if not cleaned_data.get("author"):
|
||||
cleaned_data["author"] = self._get_default_author(create=True)
|
||||
if not cleaned_data.get("category"):
|
||||
cleaned_data["category"] = self._get_default_category(create=True)
|
||||
if not cleaned_data.get("summary"):
|
||||
cleaned_data["summary"] = _generate_summary_from_stream(
|
||||
cleaned_data.get("body"),
|
||||
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:
|
||||
user = self.for_user
|
||||
if not user or not user.is_authenticated:
|
||||
return None
|
||||
existing = Author.objects.filter(user=user).first()
|
||||
if existing or not create:
|
||||
return existing
|
||||
|
||||
base_name = (user.get_full_name() or user.get_username() or f"user-{user.pk}").strip()
|
||||
base_slug = slugify(base_name) or f"user-{user.pk}"
|
||||
slug = base_slug
|
||||
suffix = 2
|
||||
while Author.objects.filter(slug=slug).exists():
|
||||
slug = f"{base_slug}-{suffix}"
|
||||
suffix += 1
|
||||
return Author.objects.create(user=user, name=base_name, slug=slug)
|
||||
|
||||
def _get_default_category(self, *, create: bool):
|
||||
existing = Category.objects.filter(slug="general").first()
|
||||
if existing or not create:
|
||||
return existing
|
||||
category, _ = Category.objects.get_or_create(
|
||||
slug="general",
|
||||
defaults={"name": "General", "description": "General articles", "colour": "neutral"},
|
||||
)
|
||||
return category
|
||||
|
||||
def _build_unique_page_slug(self, title: str) -> str:
|
||||
base_slug = slugify(title) or "article"
|
||||
parent_page = self.parent_page
|
||||
if parent_page is None and self.instance.pk:
|
||||
parent_page = self.instance.get_parent()
|
||||
if parent_page is None:
|
||||
return base_slug
|
||||
|
||||
sibling_pages = parent_page.get_children().exclude(pk=self.instance.pk)
|
||||
slug = base_slug
|
||||
suffix = 2
|
||||
while sibling_pages.filter(slug=slug).exists():
|
||||
slug = f"{base_slug}-{suffix}"
|
||||
suffix += 1
|
||||
return slug
|
||||
|
||||
|
||||
class ArticlePage(SeoMixin, Page):
|
||||
category = models.ForeignKey("blog.Category", on_delete=PROTECT, related_name="+")
|
||||
author = models.ForeignKey("authors.Author", on_delete=PROTECT)
|
||||
@@ -200,6 +310,7 @@ class ArticlePage(SeoMixin, Page):
|
||||
|
||||
parent_page_types = ["blog.ArticleIndexPage"]
|
||||
subpage_types: list[str] = []
|
||||
base_form_class = ArticlePageAdminForm
|
||||
|
||||
content_panels = [
|
||||
FieldPanel("title"),
|
||||
@@ -226,10 +337,7 @@ class ArticlePage(SeoMixin, Page):
|
||||
ObjectList(content_panels, heading="Content"),
|
||||
ObjectList(metadata_panels, heading="Metadata"),
|
||||
ObjectList(publishing_panels, heading="Publishing"),
|
||||
ObjectList(
|
||||
Page.promote_panels + SeoMixin.seo_panels,
|
||||
heading="SEO",
|
||||
),
|
||||
ObjectList(SeoMixin.seo_panels, heading="SEO"),
|
||||
]
|
||||
)
|
||||
|
||||
@@ -262,6 +370,8 @@ class ArticlePage(SeoMixin, Page):
|
||||
slug="general",
|
||||
defaults={"name": "General", "description": "General articles", "colour": "neutral"},
|
||||
)
|
||||
if not (self.summary or "").strip():
|
||||
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
|
||||
self.read_time_mins = self._compute_read_time()
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
from datetime import timedelta
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
from django.test import override_settings
|
||||
from django.utils import timezone
|
||||
|
||||
from apps.blog.models import ArticleIndexPage, ArticlePage
|
||||
from apps.blog.models import ArticleIndexPage, ArticlePage, ArticlePageAdminForm, Category
|
||||
from apps.blog.tests.factories import AuthorFactory
|
||||
|
||||
|
||||
@@ -273,3 +274,107 @@ def test_article_search_fields_include_summary():
|
||||
f.field_name for f in ArticlePage.search_fields if hasattr(f, "field_name")
|
||||
]
|
||||
assert "summary" in field_names
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_article_admin_form_relaxes_initial_required_fields(article_index, django_user_model):
|
||||
"""Slug/author/category/summary should not block initial draft validation."""
|
||||
user = django_user_model.objects.create_user(
|
||||
username="writer",
|
||||
email="writer@example.com",
|
||||
password="writer-pass",
|
||||
)
|
||||
form_class = ArticlePage.get_edit_handler().get_form_class()
|
||||
form = form_class(parent_page=article_index, for_user=user)
|
||||
|
||||
assert form.fields["slug"].required is False
|
||||
assert form.fields["author"].required is False
|
||||
assert form.fields["category"].required is False
|
||||
assert form.fields["summary"].required is False
|
||||
|
||||
|
||||
@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."""
|
||||
user = django_user_model.objects.create_user(
|
||||
username="writer",
|
||||
email="writer@example.com",
|
||||
password="writer-pass",
|
||||
first_name="Writer",
|
||||
last_name="User",
|
||||
)
|
||||
form_class = ArticlePage.get_edit_handler().get_form_class()
|
||||
form = form_class(parent_page=article_index, for_user=user)
|
||||
|
||||
body = [
|
||||
SimpleNamespace(block_type="code", value=SimpleNamespace(raw_code="print('ignore')")),
|
||||
SimpleNamespace(block_type="rich_text", value=SimpleNamespace(source="<p>Hello world body text.</p>")),
|
||||
]
|
||||
|
||||
def fake_super_clean(_self):
|
||||
_self.cleaned_data = {
|
||||
"title": "Auto Defaults Title",
|
||||
"slug": "",
|
||||
"author": None,
|
||||
"category": None,
|
||||
"summary": "",
|
||||
"body": body,
|
||||
}
|
||||
return _self.cleaned_data
|
||||
|
||||
mro = form.__class__.__mro__
|
||||
super_form_class = mro[mro.index(ArticlePageAdminForm) + 1]
|
||||
monkeypatch.setattr(super_form_class, "clean", fake_super_clean)
|
||||
cleaned = form.clean()
|
||||
|
||||
assert cleaned["slug"] == "auto-defaults-title"
|
||||
assert cleaned["author"] is not None
|
||||
assert cleaned["author"].user_id == user.id
|
||||
assert cleaned["category"] is not None
|
||||
assert cleaned["category"].slug == "general"
|
||||
assert cleaned["summary"] == "Hello world body text."
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_article_seo_tab_fields_not_duplicated():
|
||||
"""SEO tab should include each promote/SEO field only once."""
|
||||
handler = ArticlePage.get_edit_handler()
|
||||
seo_tab = next(panel for panel in handler.children if panel.heading == "SEO")
|
||||
|
||||
def flatten_field_names(panel):
|
||||
names = []
|
||||
for child in panel.children:
|
||||
if hasattr(child, "field_name"):
|
||||
names.append(child.field_name)
|
||||
else:
|
||||
names.extend(flatten_field_names(child))
|
||||
return names
|
||||
|
||||
field_names = flatten_field_names(seo_tab)
|
||||
assert field_names.count("slug") == 1
|
||||
assert field_names.count("seo_title") == 1
|
||||
assert field_names.count("search_description") == 1
|
||||
assert field_names.count("show_in_menus") == 1
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_article_save_autogenerates_summary_when_missing(article_index):
|
||||
"""Model save fallback should generate summary from prose blocks."""
|
||||
category = Category.objects.create(name="Guides", slug="guides")
|
||||
author = AuthorFactory()
|
||||
article = ArticlePage(
|
||||
title="Summary Auto",
|
||||
slug="summary-auto",
|
||||
author=author,
|
||||
category=category,
|
||||
summary="",
|
||||
body=[
|
||||
("code", {"language": "python", "filename": "", "raw_code": "print('skip')"}),
|
||||
("rich_text", "<p>This should become the summary text.</p>"),
|
||||
],
|
||||
)
|
||||
|
||||
article_index.add_child(instance=article)
|
||||
article.save()
|
||||
|
||||
assert article.summary == "This should become the summary text."
|
||||
|
||||
Reference in New Issue
Block a user