Merge pull request #386 from torchbox/fix/slow-blog-posts

Fix slow blog posts

Author: helenb

tbx/blog/models.py

@@ -178,36 +178,19 @@ def get_related_blog_posts(self):
                 "format-webp|fill-286x286",
             ),
         )
-        queryset = (
-            BlogPage.objects.live()
+        return (
+            # Assumption that blog posts for the same division
+            # will be under the same blog index page.
+            BlogPage.objects.sibling_of(self)
+            .live()
             .public()
             .defer_streamfields()
-            .prefetch_related(
-                "authors__author",
-                "related_sectors",
-                "related_services",
-                prefetch_author_images,
-            )
+            .prefetch_related("authors__author", prefetch_author_images)
             .distinct()
             .order_by("-date")
             .exclude(pk=self.pk)
         )
 
-        if final_division := getattr(self, "final_division", None):
-            # Ideally this would be implemented at the database-level but it's
-            # very hard to implement the final_division logic at the ORM level
-            def filter_fn(page):
-                return page.final_division == final_division
-
-            queryset = list(filter(filter_fn, queryset))
-        else:
-            queryset = queryset.filter(
-                Q(related_sectors__in=self.sectors)
-                | Q(related_services__in=self.services)
-            )
-
-        return queryset
-
     @cached_property
     def related_blog_posts(self):
         return self.get_related_blog_posts()[:3]

tbx/blog/tests/test_models.py

@@ -3,14 +3,14 @@
 from django.core.paginator import Page as PaginatorPage
 
 from wagtail.coreutils import get_dummy_request
-from wagtail.models import PageViewRestriction
+from wagtail.models import PageViewRestriction, Site
 from wagtail.test.utils import WagtailPageTestCase
 
 from faker import Faker
-from wagtail_factories import PageFactory
 
 from tbx.blog.factories import BlogIndexPageFactory, BlogPageFactory
 from tbx.blog.models import BlogPage
+from tbx.core.factories import HomePageFactory
 from tbx.divisions.factories import DivisionPageFactory
 from tbx.taxonomy.factories import SectorFactory, ServiceFactory
 
@@ -64,29 +64,28 @@ def test_create(self):
         self.assertEqual(another_blog_post.related_services.count(), 3)
         self.assertEqual(another_blog_post.related_sectors.count(), 2)
 
-    def test_related_blog_posts_same_division(self):
-        sector = SectorFactory.create()
-        # The logic of `final_division` skips ancestors with depth <= 2,
-        # So we create the division page with enough parents to be at depth 3:
-        division = DivisionPageFactory.create(parent=PageFactory(parent=PageFactory()))
-        blog_post = BlogPageFactory(division=division, related_sectors=[sector])
-        BlogPageFactory(title="same sector", division=None, related_sectors=[sector])
-        BlogPageFactory(
-            title="same division (direct)", division=division, related_sectors=[]
-        )
-        BlogPageFactory(
-            title="same division (direct) same sector",
-            division=division,
-            related_sectors=[sector],
-        )
-        BlogPageFactory(title="same division (parent)", parent=division)
+
+class TestBlogPage(WagtailPageTestCase):
+    def test_related_blog_posts(self):
+        site = Site.objects.get(is_default_site=True)
+        root = site.root_page.specific
+        homepage = HomePageFactory(parent=root)
+        division = DivisionPageFactory(parent=homepage, title="Charity")
+        blog_index = BlogIndexPageFactory(parent=division)
+
+        other_division = DivisionPageFactory(parent=homepage, title="Public")
+        other_index = BlogIndexPageFactory(parent=other_division)
+        BlogPageFactory(parent=other_index)
+
+        blog_post = BlogPageFactory(parent=blog_index, title="Blog Post 1")
+        BlogPageFactory(parent=blog_index, title="Blog Post 2")
+        BlogPageFactory(parent=blog_index, title="Blog Post 3")
 
         self.assertQuerySetEqual(
             blog_post.related_blog_posts,
             [
-                "same division (direct)",
-                "same division (direct) same sector",
-                "same division (parent)",
+                "Blog Post 2",
+                "Blog Post 3",
             ],
             transform=attrgetter("title"),
             ordered=False,

tbx/taxonomy/tests/test_models.py

@@ -1,3 +1,5 @@
+from unittest import skip
+
 from wagtail.models import Site
 from wagtail.test.utils import WagtailPageTestCase
 
@@ -78,6 +80,7 @@ def test_blog_post_taxonomies_tags(self):
         self.assertListEqual(list(self.blog_page_service.tags), [services[1]])
         self.assertListEqual(list(self.blog_page_sector.tags), [sectors[1]])
 
+    @skip("Related services in blog posts are currently disabled")
     def test_related_blog_posts_services(self):
         """
         Tests the `related_blog_posts` property on the `BlogPage` model using the Service taxonomy
@@ -104,6 +107,7 @@ def test_related_blog_posts_services(self):
         # so there should be 3 related blog posts
         self.assertEqual(len(blog_post3.related_blog_posts), 3)
 
+    @skip("Related sectors in blog posts are currently disabled")
     def test_related_blog_posts_sectors(self):
         """
         Tests the `related_blog_posts` property on the `BlogPage` model using the Sectors taxonomy

tbx/work/models.py

@@ -309,33 +309,19 @@ def get_related_works(self):
             ),
         )
 
-        queryset = (
-            WorkPage.objects.live()
+        return (
+            # Assumption that work pages for the same division
+            # will be under the same work index page.
+            WorkPage.objects.sibling_of(self)
+            .live()
             .public()
             .defer_streamfields()
-            .prefetch_related(
-                "related_sectors", "related_services", prefetch_listing_images
-            )
+            .prefetch_related(prefetch_listing_images)
             .distinct()
             .order_by(F("date").desc(nulls_last=True))
             .exclude(pk=self.pk)
         )
 
-        if final_division := getattr(self, "final_division", None):
-            # Ideally this would be implemented at the database-level but it's
-            # very hard to implement the final_division logic at the ORM level
-            def filter_fn(page):
-                return page.final_division == final_division
-
-            queryset = list(filter(filter_fn, queryset))
-        else:
-            queryset = queryset.filter(
-                Q(related_sectors__in=self.sectors)
-                | Q(related_services__in=self.services)
-            )
-
-        return queryset
-
     @cached_property
     def related_works(self):
         # get 3 pages with same services and exclude self page

tbx/work/tests/test_models.py

@@ -3,13 +3,12 @@
 from django.core.paginator import Page as PaginatorPage
 
 from wagtail.coreutils import get_dummy_request
-from wagtail.models import PageViewRestriction
+from wagtail.models import PageViewRestriction, Site
 from wagtail.test.utils import WagtailPageTestCase
 
-from wagtail_factories import PageFactory
-
+from tbx.core.factories import HomePageFactory
 from tbx.divisions.factories import DivisionPageFactory
-from tbx.taxonomy.factories import SectorFactory, ServiceFactory
+from tbx.taxonomy.factories import ServiceFactory
 from tbx.work.factories import (
     HistoricalWorkPageFactory,
     WorkIndexPageFactory,
@@ -62,28 +61,25 @@ def test_create(self):
 
 class TestWorkPage(WagtailPageTestCase):
     def test_related_works(self):
-        sector = SectorFactory.create()
-        # The logic of `final_division` skips ancestors with depth <= 2,
-        # So we create the division page with enough parents to be at depth 3:
-        division = DivisionPageFactory.create(parent=PageFactory(parent=PageFactory()))
-        work_page = WorkPageFactory(division=division, related_sectors=[sector])
-        WorkPageFactory(title="same sector", division=None, related_sectors=[sector])
-        WorkPageFactory(
-            title="same division (direct)", division=division, related_sectors=[]
-        )
-        WorkPageFactory(
-            title="same division (direct) same sector",
-            division=division,
-            related_sectors=[sector],
-        )
-        WorkPageFactory(title="same division (parent)", parent=division)
+        site = Site.objects.get(is_default_site=True)
+        root = site.root_page.specific
+        homepage = HomePageFactory(parent=root)
+        division = DivisionPageFactory(parent=homepage, title="Charity")
+        work_index = WorkIndexPageFactory(parent=division)
+
+        other_division = DivisionPageFactory(parent=homepage, title="Public")
+        other_index = WorkIndexPageFactory(parent=other_division)
+        WorkPageFactory(parent=other_index)
+
+        work_page = WorkPageFactory(parent=work_index, title="Work Page 1")
+        WorkPageFactory(parent=work_index, title="Work Page 2")
+        WorkPageFactory(parent=work_index, title="Work Page 3")
 
         self.assertQuerySetEqual(
             work_page.related_works,
             [
-                "same division (direct)",
-                "same division (direct) same sector",
-                "same division (parent)",
+                "Work Page 2",
+                "Work Page 3",
             ],
             transform=attrgetter("title"),
             ordered=False,