From 6dbae8ca2c33c362148812344c807c43c892a6a2 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 27 Jun 2024 15:36:08 +0100 Subject: [PATCH] Implement `for_instance` on `PageLogEntryManager` (#12093) `BaseLogEntryManager` leaves this unimplemented, and `ModelLogEntryManager` implements it but `PageLogEntryManager` doesn't. The `LogActionRegistry.get_logs_for_instance` method calls this, which means it fails on page models. Currently this is only used on generic views, which aren't used by page models (unless they're also registered as a snippet, which isn't really a supported setup) but LogActionRegistry is supposed to work transparently across log models, so this is clearly a bug. --- CHANGELOG.txt | 1 + docs/releases/6.2.md | 1 + wagtail/models/__init__.py | 3 ++ wagtail/tests/test_audit_log.py | 74 ++++++++++++++++++++++++++++++++- 4 files changed, 77 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 8c3805043f4f..88c4cab1c4fe 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -31,6 +31,7 @@ Changelog * Fix: Disallow null characters in API filter values (Jochen Wersdörfer) * Fix: Fix image preview when Willow optimizers are enabled (Alex Tomkins) * Fix: Ensure external-to-internal link conversion works when the `wagtail_serve` view is on a non-root path (Sage Abdullah) + * Fix: Add missing `for_instance` method to `PageLogEntryManager` (Matt Westcott) * Docs: Remove duplicate section on frontend caching proxies from performance page (Jake Howard) * Docs: Document `restriction_type` field on PageViewRestriction (Shlomo Markowitz) * Docs: Document Wagtail's bug bounty policy (Jake Howard) diff --git a/docs/releases/6.2.md b/docs/releases/6.2.md index 05b2f9d85e42..a1e7cbe81352 100644 --- a/docs/releases/6.2.md +++ b/docs/releases/6.2.md @@ -49,6 +49,7 @@ This feature was implemented by Albina Starykova, with support from the Wagtail * Disallow null characters in API filter values (Jochen Wersdörfer) * Fix image preview when Willow optimizers are enabled (Alex Tomkins) * Ensure external-to-internal link conversion works when the `wagtail_serve` view is on a non-root path (Sage Abdullah) + * Add missing `for_instance` method to `PageLogEntryManager` (Matt Westcott) ### Documentation diff --git a/wagtail/models/__init__.py b/wagtail/models/__init__.py index 1b7dd564dfc6..b7c9a61cfcb8 100644 --- a/wagtail/models/__init__.py +++ b/wagtail/models/__init__.py @@ -4521,6 +4521,9 @@ def viewable_by_user(self, user): return PageLogEntry.objects.filter(q) + def for_instance(self, instance): + return self.filter(page=instance) + class PageLogEntry(BaseLogEntry): page = models.ForeignKey( diff --git a/wagtail/tests/test_audit_log.py b/wagtail/tests/test_audit_log.py index 4faba09f3148..01d50fc2ab71 100644 --- a/wagtail/tests/test_audit_log.py +++ b/wagtail/tests/test_audit_log.py @@ -3,6 +3,7 @@ from django.conf import settings from django.contrib.auth import get_user_model +from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError from django.core.serializers.json import DjangoJSONEncoder from django.test import TestCase @@ -10,6 +11,7 @@ from freezegun import freeze_time from wagtail.log_actions import LogActionRegistry +from wagtail.log_actions import registry as log_registry from wagtail.models import ( Page, PageLogEntry, @@ -36,8 +38,13 @@ def setUp(self): title="Simple page", slug="simple", content="Hello", owner=self.user ) ) + self.snippet_1 = FullFeaturedSnippet.objects.create(text="snippet 1") + self.snippet_2 = FullFeaturedSnippet.objects.create(text="snippet 2") + self.snippet_content_type = ContentType.objects.get_for_model( + FullFeaturedSnippet + ) - def test_log_action(self): + def test_log_action_for_page(self): now = timezone.now() with freeze_time(now): @@ -49,7 +56,19 @@ def test_log_action(self): self.assertEqual(entry.user, self.user) self.assertEqual(entry.timestamp, now) - def test_get_for_model(self): + def test_log_action_for_snippet(self): + now = timezone.now() + + with freeze_time(now): + entry = ModelLogEntry.objects.log_action( + self.snippet_1, "wagtail.edit", user=self.user + ) + + self.assertEqual(entry.content_type, self.snippet_content_type) + self.assertEqual(entry.user, self.user) + self.assertEqual(entry.timestamp, now) + + def test_get_for_page_model(self): PageLogEntry.objects.log_action(self.page, "wagtail.edit") PageLogEntry.objects.log_action(self.simple_page, "wagtail.edit") @@ -59,11 +78,62 @@ def test_get_for_model(self): list(entries), list(PageLogEntry.objects.filter(page=self.simple_page)) ) + def test_get_for_snippet_model(self): + ModelLogEntry.objects.log_action(self.snippet_1, "wagtail.edit") + ModelLogEntry.objects.log_action(self.snippet_2, "wagtail.edit") + + entries = ModelLogEntry.objects.get_for_model(FullFeaturedSnippet) + self.assertEqual(entries.count(), 2) + self.assertListEqual( + list(entries), + list(ModelLogEntry.objects.filter(content_type=self.snippet_content_type)), + ) + def test_get_for_user(self): self.assertEqual( PageLogEntry.objects.get_for_user(self.user).count(), 1 ) # the create from setUp + def test_get_for_page_instance(self): + PageLogEntry.objects.log_action(self.page, "wagtail.edit") + PageLogEntry.objects.log_action(self.simple_page, "wagtail.edit") + other_simple_page = self.page.add_child( + instance=SimplePage( + title="Simple page 2", slug="simple2", content="Hello", owner=self.user + ) + ) + PageLogEntry.objects.log_action(other_simple_page, "wagtail.edit") + + entries = PageLogEntry.objects.for_instance(self.simple_page) + expected_entries = list(PageLogEntry.objects.filter(page=self.simple_page)) + self.assertEqual(entries.count(), 2) + self.assertListEqual(list(entries), expected_entries) + + # should also be able to retrieve entries via the log registry, which + # eliminates the need to know that PageLogEntry is the log entry model + entries = log_registry.get_logs_for_instance(self.simple_page) + self.assertEqual(entries.count(), 2) + self.assertListEqual(list(entries), expected_entries) + + def test_get_for_snippet_instance(self): + ModelLogEntry.objects.log_action(self.snippet_1, "wagtail.edit") + ModelLogEntry.objects.log_action(self.snippet_2, "wagtail.edit") + + entries = ModelLogEntry.objects.for_instance(self.snippet_1) + expected_entries = list( + ModelLogEntry.objects.filter( + content_type=self.snippet_content_type, object_id=self.snippet_1.pk + ) + ) + self.assertEqual(entries.count(), 1) + self.assertListEqual(list(entries), expected_entries) + + # should also be able to retrieve entries via the log registry, which + # eliminates the need to know that ModelLogEntry is the log entry model + entries = log_registry.get_logs_for_instance(self.snippet_1) + self.assertEqual(entries.count(), 1) + self.assertListEqual(list(entries), expected_entries) + class TestAuditLog(TestCase): def setUp(self):