chore: Improve test coverage (#395)

* Add wagtail-factories dependency

* Refactor and extend existing tests

* More tests for ietf.forms

* Skip blocks that are impractical to test

* Tests for bibliography

* Tests for cache purging

* Tests for megamenu

* Blog and IESG statement filtering

* Tests for snippets

* Remove unnecessary OrderedSet implementation

* Remove unnecessary `HomePageBase` class

* Tests for search and 500 page

* Convert tests to pytest format

* Tests for IAB pages

* Normalize tests to pytest style

* Add comment to bibliography extraction code

* Use `LazyAttribute` to generate `slug` values

* Add factory fakers for required fields

* Import `signal_handlers` inside `ready()`

* Better documentation

* More comments

---------

Co-authored-by: Kesara Rathnayake <[email protected]>

CONTRIBUTING.md

@@ -14,7 +14,13 @@ See the [installation instructions](README.md#install)
 
 ### Testing
 
-Wagtail is based on Django, and there are many Django-style tests typically named `tests.py` to test templates. These verify that the templates can be compiled (that they don't have syntax errors) and that they are inserting variables.
+Wagtail is based on Django, and there are many tests, in each app, typically named `tests.py` or `tests/test_*.py`, to test templates and business logic. These verify that the pages render without error and that they contain the expected values.
+
+The testsuite uses [pytest](https://docs.pytest.org/) and [pytest-django](https://pytest-django.readthedocs.io/). You can run the testsuite locally:
+
+```bash
+pytest
+```
 
 ## Frontend Development
 

ietf/announcements/factories.py

@@ -0,0 +1,23 @@
+import factory
+import wagtail_factories
+
+from ietf.utils.factories import StandardBlockFactory
+from .models import IABAnnouncementIndexPage, IABAnnouncementPage
+
+
+class IABAnnouncementPageFactory(wagtail_factories.PageFactory):
+    title = factory.Faker("name")
+    date = factory.Faker("date")
+    introduction = factory.Faker("paragraph")
+    body = wagtail_factories.StreamFieldFactory(StandardBlockFactory)
+
+    class Meta:  # type: ignore
+        model = IABAnnouncementPage
+
+
+class IABAnnouncementIndexPageFactory(wagtail_factories.PageFactory):
+    title = factory.Faker("name")
+    introduction = factory.Faker("paragraph")
+
+    class Meta:  # type: ignore
+        model = IABAnnouncementIndexPage

ietf/announcements/tests.py

@@ -0,0 +1,79 @@
+from datetime import timedelta
+from bs4 import BeautifulSoup
+from django.test import Client
+from django.utils import timezone
+
+import pytest
+
+from ietf.home.models import IABHomePage
+from .factories import IABAnnouncementIndexPageFactory, IABAnnouncementPageFactory
+from .models import IABAnnouncementIndexPage, IABAnnouncementPage
+
+pytestmark = pytest.mark.django_db
+
+
+class TestIABAnnouncement:
+    @pytest.fixture(autouse=True)
+    def set_up(self, iab_home: IABHomePage, client: Client):
+        self.home = iab_home
+        self.client = client
+
+        self.index: IABAnnouncementIndexPage = IABAnnouncementIndexPageFactory(
+            parent=self.home,
+        )  # type: ignore
+
+        now = timezone.now()
+
+        self.announcement_1: IABAnnouncementPage = IABAnnouncementPageFactory(
+            parent=self.index,
+            date=now - timedelta(days=10),
+        )  # type: ignore
+
+        self.announcement_2: IABAnnouncementPage = IABAnnouncementPageFactory(
+            parent=self.index,
+            date=now - timedelta(days=8),
+        )  # type: ignore
+
+        self.announcement_3: IABAnnouncementPage = IABAnnouncementPageFactory(
+            parent=self.index,
+            date=now - timedelta(days=4),
+            body__0__heading="Heading in body Streamfield",
+        )  # type: ignore
+
+        self.announcement_4: IABAnnouncementPage = IABAnnouncementPageFactory(
+            parent=self.index,
+            date=now,
+        )  # type: ignore
+
+    def test_announcement_page(self):
+        response = self.client.get(self.announcement_3.url)
+        assert response.status_code == 200
+        html = response.content.decode()
+
+        assert self.announcement_3.title in html
+        assert self.announcement_3.body[0].value in html
+        assert self.announcement_3.introduction in html
+
+    def test_homepage(self):
+        """ The two most recent announcements are shown on the homepage """
+        response = self.client.get(self.home.url)
+        assert response.status_code == 200
+        html = response.content.decode()
+
+        assert f'href="{self.announcement_3.url}"' in html
+        assert self.announcement_3.title in html
+        assert f'href="{self.announcement_4.url}"' in html
+        assert self.announcement_4.title in html
+
+    def test_index_page(self):
+        response = self.client.get(self.index.url)
+        assert response.status_code == 200
+        html = response.content.decode()
+        soup = BeautifulSoup(html, "html.parser")
+        links = [a.get_text().strip() for a in soup.select("#content .container h2 a")]
+        assert links == [
+            self.announcement_4.title,
+            self.announcement_3.title,
+            self.announcement_2.title,
+            self.announcement_1.title,
+        ]

ietf/bibliography/models.py

@@ -8,8 +8,6 @@
 from django.template.loader import get_template
 from wagtail.models import Page
 
-from ietf.utils import OrderedSet
-
 
 class BibliographyItem(models.Model):
     """
@@ -101,7 +99,7 @@ def render(self, request=None):
         else:
             return str(object)
 
-    def __str__(self):
+    def __str__(self):  # pragma: no cover
         return "Bibliography Item #{}: {}".format(self.ordering, self.content_object)
 
 
@@ -159,9 +157,11 @@ def save(self, *args, **kwargs):
                 )
                 for content_field, prepared_content_field in self.CONTENT_FIELD_MAP.items()
             }
-            tags = OrderedSet(all_soup.find_all("a", attrs={"data-app": True}))
 
-            for tag in tags:
+            # Look for <a> nodes that are tagged with bibliographic markup,
+            # create BibliographyItem records, and turn the <a> nodes into
+            # footnote links.
+            for index, tag in enumerate(all_soup.find_all("a", attrs={"data-app": True})):
                 app = tag["data-app"]
                 model = tag["data-linktype"]
                 obj_id = tag["data-id"]
@@ -187,7 +187,7 @@ def save(self, *args, **kwargs):
                     }
                 item = BibliographyItem.objects.create(
                     page=self,
-                    ordering=list(tags).index(tag) + 1,
+                    ordering=index + 1,
                     content_key=model,
                     content_identifier=obj_id,
                     **object_details

ietf/bibliography/tests.py

@@ -0,0 +1,137 @@
+import pytest
+from django.contrib.contenttypes.models import ContentType
+from django.test import Client
+from django.urls import reverse
+
+from ietf.bibliography.models import BibliographyItem
+from ietf.home.models import HomePage
+from ietf.snippets.models import RFC
+from ietf.standard.factories import StandardIndexPageFactory, StandardPageFactory
+from ietf.standard.models import StandardIndexPage, StandardPage
+
+pytestmark = pytest.mark.django_db
+
+
+class TestBibliography:
+    @pytest.fixture(autouse=True)
+    def set_up(self, home: HomePage, client: Client):
+        self.home = home
+        self.client = client
+
+        self.rfc_2026 = RFC.objects.create(
+            name="draft-ietf-poised95-std-proc-3",
+            title="The Internet Standards Process -- Revision 3",
+            rfc="2026",
+        )
+
+        self.standard_index: StandardIndexPage = StandardIndexPageFactory(
+            parent=self.home,
+        )  # type: ignore
+
+        self.standard_page: StandardPage = StandardPageFactory(
+            parent=self.standard_index,
+        )  # type: ignore
+        self.standard_page.in_depth = [
+            {
+                "type": "raw_html",
+                "value": (
+                    f'<a data-app="snippets" data-id="{self.rfc_2026.pk}"'
+                    ' data-linktype="rfc">The Standards RFC</a>'
+                ),
+            }
+        ]
+        self.standard_page.save()
+
+    def test_bibliography_item_created(self):
+        """
+        Make sure that a BibliographyItem record was created when
+        `self.standard_page` was created in `set_up()`.
+        """
+        assert BibliographyItem.objects.count() == 1
+        item = BibliographyItem.objects.get()
+        assert item.content_object == self.rfc_2026
+
+    def test_referenced_types(self, admin_client):
+        """
+        Admin view that shows which object types might be referenced in content
+        pages.
+        """
+        rfc_content_type = ContentType.objects.get_for_model(RFC)
+        response = admin_client.get(reverse("referenced_types"))
+        assert response.status_code == 200
+        html = response.content.decode()
+        assert reverse("referenced_objects", args=[rfc_content_type.pk]) in html
+        assert "snippets | RFC" in html
+
+    def test_referenced_objects(self, admin_client):
+        """
+        Admin view that shows which objects are being referenced as
+        bibliography items in content pages.
+        """
+        rfc_content_type = ContentType.objects.get_for_model(RFC)
+        response = admin_client.get(
+            reverse("referenced_objects", args=[rfc_content_type.pk])
+        )
+        assert response.status_code == 200
+        html = response.content.decode()
+        assert reverse(
+            "referencing_pages", args=[rfc_content_type.pk, self.rfc_2026.pk]
+        ) in html
+        assert "RFC 2026" in html
+
+    def test_referencing_pages(self, admin_client):
+        """
+        Admin view that shows which pages are referencing a given object.
+        """
+        rfc_content_type = ContentType.objects.get_for_model(RFC)
+        response = admin_client.get(
+            reverse("referencing_pages", args=[rfc_content_type.pk, self.rfc_2026.pk])
+        )
+        assert response.status_code == 200
+        html = response.content.decode()
+        assert self.standard_page.title in html
+
+    def test_render_page(self, client):
+        """
+        The title of the referenced object should be displayed in the page.
+        """
+        response = client.get(self.standard_page.url)
+        assert response.status_code == 200
+        html = response.content.decode()
+        assert "RFC 2026" in html
+
+    def test_render_page_reference_removed(self, client):
+        """
+        The target of a BibliographyItem was deleted. It should be displayed as
+        such.
+        """
+        self.rfc_2026.delete()
+        self.standard_page.save()
+        response = client.get(self.standard_page.url)
+        assert response.status_code == 200
+        html = response.content.decode()
+        assert "RFC 2026" not in html
+        assert "(removed)" in html
+
+    def test_update_fields_partial_raises_exception(self):
+        """
+        Updating the `key_info` and `in_depth` fields, without also updating
+        the corresponding `prepared_*` fields, is not allowed. The prepared
+        fields contain properly formatted footnotes and are meant to be
+        displayed to the visitor.
+        """
+        with pytest.raises(ValueError) as error:
+            self.standard_page.save(update_fields=["key_info", "in_depth"])
+
+        assert error.match("Either all prepared content fields must be updated or none")
+
+    def test_update_fields_with_all_prepared_fields_succeeds(self):
+        """
+        Updating the `key_info` and `in_depth` fields, while also updating
+        the corresponding `prepared_*` fields, should work fine.
+        """
+        self.standard_page.save(
+            update_fields=[
+                "key_info", "in_depth", "prepared_key_info", "prepared_in_depth"
+            ]
+        )

ietf/blog/factories.py

@@ -0,0 +1,22 @@
+import factory
+import wagtail_factories
+
+from ietf.utils.factories import StandardBlockFactory
+
+from .models import BlogIndexPage, BlogPage
+
+
+class BlogPageFactory(wagtail_factories.PageFactory):
+    title = factory.Faker("name")
+    introduction = factory.Faker("paragraph")
+    body = wagtail_factories.StreamFieldFactory(StandardBlockFactory)
+
+    class Meta:  # type: ignore
+        model = BlogPage
+
+
+class BlogIndexPageFactory(wagtail_factories.PageFactory):
+    title = factory.Faker("name")
+
+    class Meta:  # type: ignore
+        model = BlogIndexPage

ietf/blog/models.py

@@ -1,5 +1,5 @@
 from dataclasses import dataclass
-from datetime import datetime
+from datetime import datetime, time
 from functools import partial
 
 from django.core.exceptions import ObjectDoesNotExist
@@ -9,6 +9,7 @@
 from django.shortcuts import get_object_or_404, redirect, render
 from django.utils import functional
 from django.utils.safestring import mark_safe
+from django.utils.timezone import make_aware
 from modelcluster.fields import ParentalKey
 from wagtail.admin.panels import FieldPanel, InlinePanel
 from wagtail.contrib.routable_page.models import RoutablePageMixin, route
@@ -21,6 +22,12 @@
 from ..utils.blocks import StandardBlock
 from ..utils.models import FeedSettings, PromoteMixin
 
+IESG_STATEMENT_TOPIC_ID = "7"
+
+
+def make_date_aware(value):
+    return make_aware(datetime.combine(value, time()))
+
 
 def ordered_live_annotated_blogs(sibling=None):
     blogs = BlogPage.objects.live().prefetch_related("authors")
@@ -33,11 +40,11 @@ def ordered_live_annotated_blogs(sibling=None):
 
 
 def filter_pages_by_date_from(pages, date_from):
-    return pages.filter(d__gte=date_from)
+    return pages.filter(d__gte=make_date_aware(date_from))
 
 
 def filter_pages_by_date_to(pages, date_to):
-    return pages.filter(d__lte=date_to)
+    return pages.filter(d__lte=make_date_aware(date_to))
 
 
 def parse_date_search_input(date):
@@ -161,13 +168,6 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.filter_topic = None
 
-    @property
-    def first_author(self):
-        try:
-            return self.authors.first().author
-        except AttributeError:
-            return self.authors.none()
-
     @property
     def date(self):
         return self.date_published or self.first_published_at
@@ -416,7 +416,7 @@ def redirect_first(self, request, slug=None, *args, **kwargs):
         # IESG statements were moved under the IESG about/groups page. Queries to the
         # base /blog/ page that used a query string to filter for IESG statements can't
         # be redirected through ordinary redirection, so we're doing it here.
-        if request.GET.get("primary_topic") == "7":
+        if request.GET.get("primary_topic") == IESG_STATEMENT_TOPIC_ID:
             query_string = ""
             topic = request.GET.get("secondary_topic")
             if topic:
@@ -428,7 +428,7 @@ def redirect_first(self, request, slug=None, *args, **kwargs):
             date_to = request.GET.get("date_to")
             if date_to:
                 separator = "&" if query_string else ""
-                query_string = query_string + separator + "date_to" + date_to
+                query_string = query_string + separator + "date_to=" + date_to
             target_url = "/about/groups/iesg/statements"
             if query_string:
                 target_url = target_url + "?" + query_string