Merge pull request #1170 from WesternFriend/tags-n1-query

Enhance article templates and optimize database queries for tagged pages

Author: brylie

library/models.py

@@ -72,22 +72,21 @@ class LibraryItem(DrupalFields, Page):  # type: ignore
 
     @classmethod
     def get_queryset(cls):
-        related_fields = [
+        related_prefetch = [
             "authors__author",
             "topics__topic",
-            "item_audience",
-            "item_genre",
-            "item_medium",
-            "item_time_period",
-            "tags__tag",
+            "tags",
         ]
         return (
-            super()
-            .get_queryset()
-            .filter(live=True)
-            .prefetch_related(
-                *related_fields,
+            cls.objects.live()
+            .defer_streamfields()
+            .select_related(
+                "item_audience",
+                "item_genre",
+                "item_medium",
+                "item_time_period",
             )
+            .prefetch_related(*related_prefetch)
         )
 
     content_panels = Page.content_panels + [

magazine/models.py

@@ -299,25 +299,13 @@ def get_context(
     ) -> dict:
         context = super().get_context(request)
 
-        context["articles"] = (
-            MagazineArticle.objects.filter(
-                department__title=self.title,
-            )
-            .live()
-            .prefetch_related(
-                "authors__author",
-                "issue",
-            )
-        )
-
         articles = (
             MagazineArticle.objects.filter(
                 department__title=self.title,
             )
             .live()
             .prefetch_related(
                 "authors__author",
-                "issue",
             )
         )
 
@@ -365,9 +353,16 @@ class MagazineArticle(DrupalFields, Page):  # type: ignore
 
     @classmethod
     def get_queryset(cls):
-        """Prefetch authors and tags for performance."""
-        related_fields = ["authors__author", "tags__tag", "department"]
-        return super().get_queryset().prefetch_related(*related_fields)
+        """Optimize related fetches for listings.
+
+        Note: Use cls.objects rather than super().get_queryset() because the
+        base class does not provide a classmethod get_queryset.
+        """
+        return (
+            cls.objects.defer_streamfields()
+            .select_related("department")
+            .prefetch_related("authors__author", "tags")
+        )
 
     class Meta:
         verbose_name = "Magazine Article"

magazine/templates/magazine/magazine_article_summary.html

@@ -1,4 +1,4 @@
-{% load wagtailcore_tags %}
+{% load i18n wagtailcore_tags %}
 
 <article class="card bg-base-100 shadow-sm hover:shadow-md transition-shadow mb-4">
     <div class="card-body">
@@ -8,29 +8,42 @@ <h3 class="card-title">
             </a>
         </h3>
 
-        {% if article.authors.count %}
-            <div class="mb-2">
-                <span class="font-medium">Author(s):</span>
-                {% for author in article.authors.all %}
-                    {% if author.author.live %}
-                        <a href="{% pageurl author.author %}">{{ author.author }}</a>{% if not forloop.last %},{% endif %}
-                    {% else %}
-                        {{ author.author }}{% if not forloop.last %},{% endif %}
-                    {% endif %}
-                {% endfor %}
-            </div>
-        {% endif %}
+        {% with author_links=article.authors.all %}
+            {% if author_links %}
+                <div class="mb-2">
+                    <span id="authors-label-{{ article.pk }}" class="font-medium">{% translate "Authors" %}:</span>
+                    <ul class="inline list-none p-0 m-0" aria-labelledby="authors-label-{{ article.pk }}">
+                        {% for author in author_links %}
+                            <li class="inline">
+                                {% if author.author.live %}
+                                    <a href="{% pageurl author.author %}">{{ author.author }}</a>
+                                {% else %}
+                                    {{ author.author }}
+                                {% endif %}
+                                {% if not forloop.last %}, {% endif %}
+                            </li>
+                        {% endfor %}
+                    </ul>
+                </div>
+            {% endif %}
+        {% endwith %}
 
         <div class="prose">
             {{ article.teaser|richtext }}
         </div>
 
-        <div class="card-actions">
-            <span>
-                Issue: <a href="{% pageurl article.get_parent %}" class="link">
-                    {{ article.get_parent }} ({{ article.get_parent.specific.publication_date|date:"F Y" }})
-                </a>
-            </span>
-        </div>
+        {% with issue=article.get_parent.specific %}
+            <div class="card-actions">
+                <span>
+                    <span class="font-medium">{% translate "Issue" %}:</span>
+                    <a href="{% pageurl issue %}" class="link">{{ issue }}</a>
+                    {% if issue.publication_date %}
+                        (<time datetime="{{ issue.publication_date|date:'Y-m-d' }}">
+                            {{ issue.publication_date|date:"F Y" }}
+                        </time>)
+                    {% endif %}
+                </span>
+            </div>
+        {% endwith %}
     </div>
 </article>

magazine/templates/search/magazine_article.html

@@ -0,0 +1 @@
+{% include "magazine/magazine_article_summary.html" with article=entity %}

magazine/templates/search/magazine_issue.html

@@ -0,0 +1,16 @@
+{% load wagtailcore_tags %}
+<article class="card bg-base-100 shadow-sm hover:shadow-md transition-shadow mb-4">
+  <div class="card-body">
+    <h3 class="card-title">
+      <a href="{% pageurl entity %}">{{ entity.title }}</a>
+    </h3>
+    {% if entity.specific.publication_date %}
+      <p class="text-sm text-base-content/70">
+        Published
+        <time datetime="{{ entity.specific.publication_date|date:'Y-m-d' }}">
+          {{ entity.specific.publication_date|date:"F Y" }}
+        </time>
+      </p>
+    {% endif %}
+  </div>
+</article>

tags/tests.py

@@ -1,14 +1,16 @@
+import re
 from django.test import TestCase
 from django.urls import reverse
 from taggit.models import Tag
 from library.models import LibraryItem
 from library.factories import LibraryItemFactory
 from magazine.factories import MagazineArticleFactory
-from magazine.models import MagazineArticle
+from magazine.models import MagazineArticle, MagazineArticleAuthor
 from news.factories import NewsItemFactory
 from news.models import NewsItem
 from wf_pages.factories import WfPageFactory
 from wf_pages.models import WfPage
+from contact.factories import PersonFactory
 
 
 class TaggedPageListViewQuerysetAndContentOrderTest(TestCase):
@@ -157,3 +159,65 @@ def test_pagination_second_page(self):
             paginated_context.page.previous_page_number(),
             expected_previous_page,
         )
+
+
+class TaggedPageListViewTemplateRenderingTest(TestCase):
+    def setUp(self):
+        self.tag = Tag.objects.create(name="Authors Tag", slug="authors-tag")
+        self.url = reverse("tags:tagged_page_list", kwargs={"tag": self.tag.slug})
+
+        # Create a magazine article and attach authors
+        self.article = MagazineArticleFactory(title="Article With Authors")
+        self.article.tags.add(self.tag)
+        self.article.save()
+
+        self.person_live = PersonFactory()
+        # Publish person_live so template renders it as a link
+        self.person_live.save_revision().publish()
+
+        self.person_not_live = PersonFactory()
+        # Ensure non-live stays unpublished for template logic
+        self.person_not_live.live = False
+        self.person_not_live.save()
+
+        MagazineArticleAuthor.objects.create(
+            article=self.article,
+            author=self.person_live,
+        )
+        MagazineArticleAuthor.objects.create(
+            article=self.article,
+            author=self.person_not_live,
+        )
+
+    def test_authors_and_issue_render_with_accessible_markup(self):
+        response = self.client.get(self.url)
+        html = response.content.decode()
+
+        # Authors label and list present with dynamic id
+        self.assertIn("Authors", html)
+        # Extract actual authors-label id from the span
+        m = re.search(
+            r"<span[^>]*id=\"(authors-label-[^\"]+)\"[^>]*>\s*Authors\s*:</span>",
+            html,
+        )
+        self.assertIsNotNone(m, "Could not find authors label span with id")
+        label_id = m.group(1)
+        # Verify aria-labelledby matches the extracted id
+        self.assertIn(f'aria-labelledby="{label_id}"', html)
+
+        # Live author should be linked
+        live_title = self.person_live.title
+        self.assertRegex(html, rf"<a[^>]*>\s*{re.escape(live_title)}\s*</a>")
+        # Non-live author should be plain text (no anchor)
+        non_live_title = self.person_not_live.title
+        self.assertIn(non_live_title, html)
+        self.assertIsNone(
+            re.search(rf"<a[^>]*>\s*{re.escape(non_live_title)}\s*</a>", html),
+        )
+
+        # Issue label and machine-readable date present
+        self.assertIn("Issue", html)
+        expected_date = self.article.get_parent().specific.publication_date.strftime(
+            "%Y-%m-%d",
+        )
+        self.assertIn(f'<time datetime="{expected_date}"', html)

tags/views.py

@@ -25,15 +25,42 @@ def get_queryset(self):
         # Get all the pages tagged with the given tag
         filter_condition = Q(tags__slug__in=[tag])
 
-        library_items = LibraryItem.objects.filter(filter_condition).order_by("title")
+        library_items = (
+            LibraryItem.get_queryset()
+            .filter(filter_condition)
+            .live()
+            .public()
+            .select_related("content_type")
+            .prefetch_related("authors__author")
+            # DB-level ordering unnecessary; combined list is sorted below
+        )
 
-        magazine_articles = MagazineArticle.objects.filter(filter_condition).order_by(
-            "title",
+        magazine_articles = (
+            MagazineArticle.get_queryset()
+            .filter(filter_condition)
+            .live()
+            .public()
+            .select_related("content_type")
+            .prefetch_related("authors__author")
+            # DB-level ordering unnecessary; combined list is sorted below
         )
 
-        news_items = NewsItem.objects.filter(filter_condition).order_by("title")
+        news_items = (
+            NewsItem.objects.filter(filter_condition)
+            .live()
+            .public()
+            .select_related("content_type")
+            # DB-level ordering unnecessary; combined list is sorted below
+        )
 
-        pages = WfPage.objects.filter(filter_condition).order_by("title")
+        pages = (
+            WfPage.get_queryset()
+            .filter(filter_condition)
+            .live()
+            .public()
+            .select_related("content_type")
+            # DB-level ordering unnecessary; combined list is sorted below
+        )
 
         combined_queryset_list = list(
             chain(

wf_pages/models.py

@@ -87,7 +87,7 @@ class WfPage(DrupalFields, Page):
     @classmethod
     def get_queryset(cls):
         """Prefetch tags for performance."""
-        return super().get_queryset().prefetch_related("tags__tag")
+        return cls.objects.prefetch_related("tags")
 
     content_panels = Page.content_panels + [
         FieldPanel("body"),