diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 708acc3006..8fd3847a82 100644 --- a/CHANGELOG.txt +++ b/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) diff --git a/docs/releases/4.1.1.md b/docs/releases/4.1.1.md index 4751aee9f4..5415c173f2 100644 --- a/docs/releases/4.1.1.md +++ b/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) diff --git a/wagtail/admin/tests/test_reports_views.py b/wagtail/admin/tests/test_reports_views.py index fc5acfb0a5..920b7eaf87 100644 --- a/wagtail/admin/tests/test_reports_views.py +++ b/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"] diff --git a/wagtail/admin/views/reports/aging_pages.py b/wagtail/admin/views/reports/aging_pages.py index 832446e5d3..9a2e84fc8a 100644 --- a/wagtail/admin/views/reports/aging_pages.py +++ b/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): diff --git a/wagtail/models/audit_log.py b/wagtail/models/audit_log.py index 98e6a7c10b..4e7310680d 100644 --- a/wagtail/models/audit_log.py +++ b/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() diff --git a/wagtail/users/utils.py b/wagtail/users/utils.py index a6e5766f50..55b32fe51c 100644 --- a/wagtail/users/utils.py +++ b/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}