From a87acf5b174c769e04cb3281646a9f7fd5b40990 Mon Sep 17 00:00:00 2001 From: Dan Braghis Date: Fri, 17 Sep 2021 17:17:42 +0100 Subject: [PATCH] Fix page history with deleted user --- CHANGELOG.txt | 1 + docs/releases/2.15.rst | 1 + wagtail/admin/tests/test_audit_log.py | 18 ++++++++++++++++++ wagtail/core/models/audit_log.py | 9 ++++----- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index b018a9d441..23d8a9886e 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -39,6 +39,7 @@ Changelog * Fix: Ensure admin tab JS events are handled on page load (Andrew Stone) * Fix: `EmailNotificationMixin` and `send_notification` should only send emails to active users (Bryan Williams) * Fix: Disable Task confirmation now shows the correct value for quantity of tasks in progress (LB Johnston) + * Fix: Page history now works correctly when it contains changes by a deleted user (Dan Braghis) 2.14.1 (12.08.2021) diff --git a/docs/releases/2.15.rst b/docs/releases/2.15.rst index 6fa304aac8..df1a7aeae3 100644 --- a/docs/releases/2.15.rst +++ b/docs/releases/2.15.rst @@ -54,6 +54,7 @@ Bug fixes * Ensure admin tab JS events are handled on page load (Andrew Stone) * `EmailNotificationMixin` and `send_notification` should only send emails to active users (Bryan Williams) * Disable Task confirmation now shows the correct value for quantity of tasks in progress (LB Johnston) + * Page history now works correctly when it contains changes by a deleted user (Dan Braghis) Upgrade considerations ====================== diff --git a/wagtail/admin/tests/test_audit_log.py b/wagtail/admin/tests/test_audit_log.py index 3984402d0b..2abfeb7bf4 100644 --- a/wagtail/admin/tests/test_audit_log.py +++ b/wagtail/admin/tests/test_audit_log.py @@ -151,6 +151,24 @@ class TestAuditLogAdmin(TestCase, WagtailTestUtils): self.assertContains(response, 'About', 3) # create, save draft, delete self.assertContains(response, 'Deleted', 2) + def test_history_with_deleted_user(self): + self._update_page(self.hello_page) + + expected_deleted_string = f"user {self.editor.pk} (deleted)" + self.editor.delete() + + self.login(user=self.administrator) + + # check page history + response = self.client.get( + reverse('wagtailadmin_pages:history', kwargs={'page_id': self.hello_page.id}) + ) + self.assertContains(response, expected_deleted_string) + + # check site history + response = self.client.get(reverse('wagtailadmin_reports:site_history')) + self.assertContains(response, expected_deleted_string) + def test_edit_form_has_history_link(self): self.hello_page.save_revision() self.login(user=self.editor) diff --git a/wagtail/core/models/audit_log.py b/wagtail/core/models/audit_log.py index a3f4ff3754..33bf969d06 100644 --- a/wagtail/core/models/audit_log.py +++ b/wagtail/core/models/audit_log.py @@ -135,11 +135,10 @@ class BaseLogEntry(models.Model): Defaults to 'system' when none is provided """ if self.user_id: - try: - user = self.user - except self._meta.get_field('user').related_model.DoesNotExist: - # User has been deleted - return _('user %(id)d (deleted)') % {'id': 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} try: full_name = user.get_full_name().strip()