Skip to content

Commit

Permalink
Implement for_instance on PageLogEntryManager (wagtail#12093)
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
gasman committed Jun 27, 2024
1 parent 03f2618 commit 6dbae8c
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions docs/releases/6.2.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions wagtail/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
74 changes: 72 additions & 2 deletions wagtail/tests/test_audit_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@

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
from django.utils import timezone
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,
Expand All @@ -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):
Expand All @@ -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")

Expand All @@ -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):
Expand Down

0 comments on commit 6dbae8c

Please sign in to comment.