Merge pull request #772 from WesternFriend/magazine-tests

Magazine tests

magazine/models.py

@@ -3,7 +3,10 @@
 
 import arrow
 from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator
+from django.core.paginator import Page as PaginatorPage
 from django.db import models
+from django.db.models import QuerySet
+from django.http import HttpRequest
 from django_flatpickr.widgets import DatePickerInput
 from modelcluster.contrib.taggit import ClusterTaggableManager  # type: ignore
 from modelcluster.fields import ParentalKey  # type: ignore
@@ -115,7 +118,7 @@ def get_context(self, request, *args, **kwargs):
         return context
 
 
-class MagazineIssue(DrupalFields, Page):
+class MagazineIssue(DrupalFields, Page):  # type: ignore
     cover_image = models.ForeignKey(
         "wagtailimages.Image",
         on_delete=models.SET_NULL,
@@ -131,19 +134,31 @@ class MagazineIssue(DrupalFields, Page):
     drupal_node_id = models.PositiveIntegerField(null=True, blank=True, db_index=True)
 
     @property
-    def featured_articles(self):
+    def featured_articles(self) -> QuerySet["MagazineArticle"]:
         # Return a cursor of related articles that are featured
         return (
             MagazineArticle.objects.child_of(self).filter(is_featured=True).specific()
         )
 
     @property
-    def publication_end_date(self):
+    def articles_by_department(self) -> QuerySet["MagazineArticle"]:
+        # Return a cursor of child articles ordered by department
+        return (
+            MagazineArticle.objects.child_of(self).live().order_by("department__title")
+        )
+
+    @property
+    def publication_end_date(self) -> datetime.date | None:
+        """Return the first day of the month after the publication date.
+
+        NOTE: we can return any day in the following month,
+        since we only use the year and month components
+        """
         if self.publication_date:
             # TODO: try to find a cleaner way to add a month to the publication date
-            # I.e. the 'add a month' approach may be flawed altogether.
-            # Note: this should probably add more than one month,
-            # since the magazine is not published monthly
+            # We add 31 days here since we can't add a month directly
+            # 31 days is a safe upper bound for adding a month
+            # since the publication date will be at least 28 days prior
             return self.publication_date + timedelta(days=+31)
         return None
 
@@ -162,15 +177,7 @@ class Meta:
             models.Index(fields=["drupal_node_id"]),
         ]
 
-    def get_context(self, request, *args, **kwargs):
-        context = super().get_context(request)
-        context["articles_by_department"] = (
-            MagazineArticle.objects.child_of(self).live().order_by("department__title")
-        )
-
-        return context
-
-    def get_sitemap_urls(self):
+    def get_sitemap_urls(self) -> list[dict]:
         return [{"location": self.full_url, "lastmod": self.latest_revision_created_at}]
 
 
@@ -185,7 +192,12 @@ class MagazineArticleTag(TaggedItemBase):
 class MagazineTagIndexPage(Page):
     max_count = 1
 
-    def get_context(self, request, *args, **kwargs):
+    def get_context(
+        self,
+        request: HttpRequest,
+        *args: tuple,
+        **kwargs: dict,
+    ) -> dict:
         tag = request.GET.get("tag")
         articles = MagazineArticle.objects.filter(tags__name=tag)
 
@@ -200,10 +212,16 @@ class MagazineDepartmentIndexPage(Page):
 
     content_panels = Page.content_panels + [FieldPanel("intro")]
 
+    parent_page_types = ["MagazineIndexPage"]
     subpage_types: list[str] = ["MagazineDepartment"]
     max_count = 1
 
-    def get_context(self, request, *args, **kwargs):
+    def get_context(
+        self,
+        request: HttpRequest,
+        *args: tuple,
+        **kwargs: dict,
+    ) -> dict:
         departments = MagazineDepartment.objects.all()
 
         context = super().get_context(request)
@@ -226,15 +244,15 @@ class MagazineDepartment(Page):
     autocomplete_search_field = "title"
 
     # TODO: remove if not using autocomplete
-    def autocomplete_label(self):
+    def autocomplete_label(self) -> str:
         return self.title
 
     # TODO: remove if not using autocomplete
-    def __str__(self):
+    def __str__(self) -> str:
         return self.title
 
 
-class MagazineArticle(DrupalFields, Page):
+class MagazineArticle(DrupalFields, Page):  # type: ignore
     teaser = RichTextField(  # type: ignore
         blank=True,
         help_text="Try to keep teaser to a couple dozen words.",
@@ -323,7 +341,7 @@ class MagazineArticle(DrupalFields, Page):
     parent_page_types = ["MagazineIssue"]
     subpage_types: list[str] = []
 
-    def get_sitemap_urls(self):
+    def get_sitemap_urls(self) -> list[dict]:
         return [
             {
                 "location": self.full_url,
@@ -333,26 +351,34 @@ def get_sitemap_urls(self):
         ]
 
     @property
-    def is_public_access(self):
+    def is_public_access(self) -> bool:
         """Check whether article should be accessible to all readers or only
         subscribers based on issue publication date."""
         parent_issue = self.get_parent()
 
+        # TODO: try to find a good way to shift the date
+        # without using arrow
+        # so we can remove the arrow dependency since it is only used here
         today = arrow.utcnow()
 
         six_months_ago = today.shift(months=-6).date()
 
         # Issues older than six months are public access
-        return parent_issue.specific.publication_date <= six_months_ago
-
-    def get_context(self, request, *args, **kwargs):
+        return parent_issue.specific.publication_date <= six_months_ago  # type: ignore
+
+    def get_context(
+        self,
+        request: HttpRequest,
+        *args: tuple,
+        **kwargs: dict,
+    ) -> dict:
         context = super().get_context(request)
 
         # Check whether user is subscriber
         # make sure they are authenticated first,
         # to avoid checking for "is_subscriber" on anonymous user
         user_is_subscriber = (
-            request.user.is_authenticated and request.user.is_subscriber
+            request.user.is_authenticated and request.user.is_subscriber  # type: ignore
         )
 
         # Subscribers and superusers can always view full articles
@@ -361,7 +387,7 @@ def get_context(self, request, *args, **kwargs):
         # user can view full article if any of these conditions is True
         context["user_can_view_full_article"] = (
             user_is_subscriber
-            or request.user.is_superuser
+            or request.user.is_superuser  # type: ignore
             or self.is_public_access
             or self.is_featured
         )
@@ -462,7 +488,7 @@ class Meta:
         ]
 
 
-class ArchiveIssue(DrupalFields, Page):
+class ArchiveIssue(DrupalFields, Page):  # type: ignore
     publication_date = models.DateField(
         null=True,
         help_text="Please select the first day of the publication month",
@@ -510,12 +536,15 @@ class DeepArchiveIndexPage(Page):
     parent_page_types = ["MagazineIndexPage"]
     subpage_types: list[str] = ["ArchiveIssue"]
 
-    def get_publication_years(self):
+    def get_publication_years(self) -> list[int]:
         publication_dates = ArchiveIssue.objects.dates("publication_date", "year")
 
         return [publication_date.year for publication_date in publication_dates]
 
-    def get_filtered_archive_issues(self, request):
+    def get_filtered_archive_issues(
+        self,
+        request: HttpRequest,
+    ) -> QuerySet[ArchiveIssue]:
         # Check if any query string is available
         query = request.GET.dict()
 
@@ -530,32 +559,42 @@ def get_filtered_archive_issues(self, request):
 
         return ArchiveIssue.objects.all().filter(**facets)
 
-    def get_paginated_archive_issues(self, archive_issues, request):
+    def get_paginated_archive_issues(
+        self,
+        request: HttpRequest,
+        archive_issues: QuerySet[ArchiveIssue],
+    ) -> PaginatorPage:
         items_per_page = 9
 
         paginator = Paginator(archive_issues, items_per_page)
 
         archive_issues_page = request.GET.get("page")
 
-        try:
-            paginated_archive_issues = paginator.page(archive_issues_page)
-        except PageNotAnInteger:
-            # If page is not an integer, deliver first page.
-            paginated_archive_issues = paginator.page(1)
-        except EmptyPage:
-            # If page is out of range (e.g. 9999), deliver last page of results.
-            paginated_archive_issues = paginator.page(paginator.num_pages)
-
-        return paginated_archive_issues
-
-    def get_context(self, request, *args, **kwargs):
+        # Make sure page is numeric and less than or equal to the total pages
+        if (
+            archive_issues_page
+            and archive_issues_page.isdigit()
+            and int(archive_issues_page) <= paginator.num_pages
+        ):
+            paginator_page_number = int(archive_issues_page)
+        else:
+            paginator_page_number = 1
+
+        return paginator.page(paginator_page_number)
+
+    def get_context(
+        self,
+        request: HttpRequest,
+        *args: tuple,
+        **kwargs: dict,
+    ) -> dict:
         context = super().get_context(request)
 
         archive_issues = self.get_filtered_archive_issues(request)
 
         paginated_archive_issues = self.get_paginated_archive_issues(
-            archive_issues,
             request,
+            archive_issues,
         )
 
         context["archive_issues"] = paginated_archive_issues

magazine/panels.py

@@ -8,10 +8,10 @@ class NestedInlinePanel(InlinePanel):
     Issue: https://github.com/wagtail/wagtail/issues/5126
     """
 
-    def widget_overrides(self):
-        widgets = {}
-        child_edit_handler = self.get_child_edit_handler()
-        for handler_class in child_edit_handler.children:
-            widgets.update(handler_class.widget_overrides())
-        widget_overrides = {self.relation_name: widgets}
-        return widget_overrides
+    def widget_overrides(self) -> dict:
+        widgets = {}  # pragma: no cover
+        child_edit_handler = self.get_child_edit_handler()  # pragma: no cover
+        for handler_class in child_edit_handler.children:  # pragma: no cover
+            widgets.update(handler_class.widget_overrides())  # pragma: no cover
+        widget_overrides = {self.relation_name: widgets}  # pragma: no cover
+        return widget_overrides  # pragma: no cover

magazine/templates/magazine/magazine_issue.html

@@ -31,7 +31,7 @@ <h2 class="h3 ms-4">Featured Articles</h2>
                 </ul>
             {% endif %}
 
-            {% regroup articles_by_department by specific.department as departments %}
+            {% regroup page.specific.articles_by_department by specific.department as departments %}
 
             {% for department in departments %}
                 <h2 class="h3 ms-4">

magazine/tests.py

@@ -1 +1,104 @@
-# Create your tests here.
+import datetime
+from django.test import TestCase
+from wagtail.models import Page, Site
+from home.models import HomePage
+from .models import (
+    MagazineDepartmentIndexPage,
+    MagazineDepartment,
+    MagazineIssue,
+    MagazineIndexPage,
+    MagazineArticle,
+)
+
+
+class MagazineIssueTest(TestCase):
+    def setUp(self) -> None:
+        site_root = Page.objects.get(id=2)
+
+        self.home_page = HomePage(title="Home")
+        site_root.add_child(instance=self.home_page)
+
+        Site.objects.all().update(root_page=self.home_page)
+
+        self.magazine_index = MagazineIndexPage(title="Magazine")
+        self.home_page.add_child(instance=self.magazine_index)
+
+        self.magazine_issue = MagazineIssue(
+            title="Issue 1",
+            issue_number=1,
+            publication_date="2020-01-01",
+        )
+
+        self.magazine_department_index = MagazineDepartmentIndexPage(
+            title="Departments",
+        )
+        self.magazine_index.add_child(instance=self.magazine_department_index)
+        self.magazine_department_one = MagazineDepartment(
+            title="Department 1",
+        )
+        self.magazine_department_two = MagazineDepartment(
+            title="Department 2",
+        )
+
+        self.magazine_department_index.add_child(instance=self.magazine_department_one)
+        self.magazine_department_index.add_child(instance=self.magazine_department_two)
+        self.magazine_index.add_child(instance=self.magazine_issue)
+        self.magazine_article_one = self.magazine_issue.add_child(
+            instance=MagazineArticle(
+                title="Article 1",
+                department=self.magazine_department_two,
+                is_featured=True,
+            ),
+        )
+        self.magazine_article_two = self.magazine_issue.add_child(
+            instance=MagazineArticle(
+                title="Article 2",
+                department=self.magazine_department_one,
+                is_featured=False,
+            ),
+        )
+
+    def test_featured_articles(self) -> None:
+        """Test that the featured_articles property returns the correct
+        articles."""
+        self.assertEqual(
+            list(self.magazine_issue.featured_articles),
+            [self.magazine_article_one],
+        )
+
+    def test_articles_by_department(self) -> None:
+        """Test that the articles_by_department property returns the correct
+        articles."""
+        self.assertEqual(
+            list(self.magazine_issue.articles_by_department),
+            [
+                self.magazine_article_two,
+                self.magazine_article_one,
+            ],
+        )
+
+    def test_publication_end_date(self) -> None:
+        """Test that the publication_end_date property returns the correct
+        date."""
+        self.assertEqual(
+            self.magazine_issue.publication_end_date,
+            datetime.date(2020, 2, 1),
+        )
+
+    def test_get_sitemap_urls(self) -> None:
+        """Validate the output of get_sitemap_urls."""
+
+        expected_last_mod = None
+        expected_location_contains = "/magazine/issue-1/"
+
+        sitemap_urls = self.magazine_issue.get_sitemap_urls()
+
+        self.assertEqual(len(sitemap_urls), 1)
+        self.assertEqual(
+            sitemap_urls[0]["lastmod"],
+            expected_last_mod,
+        )
+        self.assertIn(
+            expected_location_contains,
+            sitemap_urls[0]["location"],
+        )