Fix aging pages report KeyError caused when page publisher deleted (#9531)

CHANGELOG.txt

@@ -57,6 +57,7 @@ Changelog
  * Fix: Prevent error in handling null ParentalKeys when populating the reference index (Matt Westcott)
  * Fix: Make sure minimap error indicators follow the minimap scrolling (Thibaud Colas)
  * Fix: Ensure background HTTP request to clear stale preview data correctly respects the `CSRF_HEADER_NAME` setting (Sage Abdullah)
+ * Fix: Prevent error on aging pages report when "Last published by" user has been deleted (Joshua Munn)
 
 
 4.1 LTS (01.11.2022)

docs/releases/4.1.1.md

@@ -24,3 +24,4 @@ depth: 1
  * Prevent error in handling null ParentalKeys when populating the reference index (Matt Westcott)
  * Make sure minimap error indicators follow the minimap scrolling (Thibaud Colas)
  * Ensure background HTTP request to clear stale preview data correctly respects the `CSRF_HEADER_NAME` setting (Sage Abdullah)
+ * Prevent error on aging pages report when "Last published by" user has been deleted (Joshua Munn)

wagtail/admin/tests/test_reports_views.py

@@ -412,6 +412,18 @@ class TestAgingPagesView(TestCase, WagtailTestUtils):
 
         self.assertEqual(worksheet["C2"].number_format, ExcelDateFormatter().get())
 
+    def test_report_renders_when_page_publisher_deleted(self):
+        temp_user = self.create_superuser(
+            "temp", email="temp@user.com", password="tempuser"
+        )
+        expected_deleted_string = f"user {temp_user.pk} (deleted)"
+
+        self.home.save_revision().publish(user=temp_user)
+        temp_user.delete()
+
+        response = self.get()
+        self.assertContains(response, expected_deleted_string)
+
 
 class TestFilteredAgingPagesView(TestCase, WagtailTestUtils):
     fixtures = ["test.json"]

wagtail/admin/views/reports/aging_pages.py

@@ -1,6 +1,7 @@
 import django_filters
 from django.contrib.auth import get_user_model
 from django.contrib.contenttypes.models import ContentType
+from django.core.exceptions import ValidationError
 from django.db.models import OuterRef, Subquery
 from django.utils.translation import gettext_lazy as _
 
@@ -8,6 +9,7 @@ from wagtail.admin.filters import ContentTypeFilter, WagtailFilterSet
 from wagtail.admin.widgets import AdminDateInput
 from wagtail.coreutils import get_content_type_label
 from wagtail.models import Page, PageLogEntry, UserPagePermissionsProxy, get_page_models
+from wagtail.users.utils import get_deleted_user_display_name
 
 from .base import PageReportView
 
@@ -52,24 +54,43 @@ class AgingPagesView(PageReportView):
     def __init__(self, **kwargs):
         super().__init__(**kwargs)
 
+        self.user_model = get_user_model()
         self.custom_field_preprocess = self.custom_field_preprocess.copy()
         self.custom_field_preprocess["content_type"] = {
             self.FORMAT_CSV: get_content_type_label,
             self.FORMAT_XLSX: get_content_type_label,
         }
 
+    def user_id_to_python(self, user_id):
+        return self.user_model._meta.pk.to_python(user_id)
+
+    def add_last_publisher_name_to_page(self, username_mapping, page):
+        if page.last_published_by:
+            # Giving the last_published_by annotation an explicit output_field type
+            # causes an issue with prefetch_workflow_states when the field is a
+            # ConvertedValueField. If the last user to publish the page has been
+            # deleted, we will render their user id in the template, so we call
+            # to_python on the value so that what's rendered matches the developer's
+            # expectation in the case of complex primary key types (e.g. UUIDField).
+            try:
+                user_id_value = self.user_id_to_python(page.last_published_by)
+            except ValidationError:
+                user_id_value = page.last_published_by
+
+            last_published_by_user = username_mapping.get(
+                user_id_value, get_deleted_user_display_name(user_id=user_id_value)
+            )
+            page.last_published_by_user = last_published_by_user
+
     def decorate_paginated_queryset(self, queryset):
-        User = get_user_model()
         user_ids = set(queryset.values_list("last_published_by", flat=True))
 
         username_mapping = {
             user.pk: user.get_username()
-            for user in User.objects.filter(pk__in=user_ids)
+            for user in self.user_model.objects.filter(pk__in=user_ids)
         }
         for page in queryset:
-            if page.last_published_by:
-                page.last_published_by_user = username_mapping[page.last_published_by]
-
+            self.add_last_publisher_name_to_page(username_mapping, page)
         return queryset
 
     def get_queryset(self):

wagtail/models/audit_log.py

@@ -16,6 +16,7 @@ from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
 
 from wagtail.log_actions import registry as log_action_registry
+from wagtail.users.utils import get_deleted_user_display_name
 
 
 class LogEntryQuerySet(models.QuerySet):
@@ -223,8 +224,7 @@ class BaseLogEntry(models.Model):
         if self.user_id:
             user = self.user
             if user is None:
-                # User has been deleted. Using a string placeholder as the user id could be non-numeric
-                return _("user %(id)s (deleted)") % {"id": self.user_id}
+                return get_deleted_user_display_name(self.user_id)
 
             try:
                 full_name = user.get_full_name().strip()

wagtail/users/utils.py

@@ -2,6 +2,7 @@ import hashlib
 
 from django.conf import settings
 from django.utils.http import urlencode
+from django.utils.translation import gettext_lazy as _
 
 from wagtail.compat import AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME
 
@@ -44,3 +45,8 @@ def get_gravatar_url(email, size=50):
     )
 
     return gravatar_url
+
+
+def get_deleted_user_display_name(user_id):
+    # Use a string placeholder as the user id could be non-numeric
+    return _("user %(id)s (deleted)") % {"id": user_id}