diff --git a/wagtail/core/migrations/0052_pagelogentry.py b/wagtail/core/migrations/0052_pagelogentry.py index 19f2d8b6ad..2adf62c3d7 100644 --- a/wagtail/core/migrations/0052_pagelogentry.py +++ b/wagtail/core/migrations/0052_pagelogentry.py @@ -26,8 +26,8 @@ class Migration(migrations.Migration): ('deleted', models.BooleanField(default=False)), ('content_type', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to='contenttypes.ContentType', verbose_name='content type')), ('page', models.ForeignKey(db_constraint=False, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='wagtailcore.Page')), - ('revision', models.ForeignKey(db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='wagtailcore.PageRevision')), - ('user', models.ForeignKey(db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to=settings.AUTH_USER_MODEL)), + ('revision', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='wagtailcore.PageRevision')), + ('user', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to=settings.AUTH_USER_MODEL)), ], options={ 'verbose_name': 'page log entry', diff --git a/wagtail/core/models.py b/wagtail/core/models.py index 39f35e81a2..355225dd5d 100644 --- a/wagtail/core/models.py +++ b/wagtail/core/models.py @@ -43,6 +43,7 @@ from treebeard.mp_tree import MP_Node from wagtail.core.fields import StreamField from wagtail.core.forms import TaskStateCommentForm +from wagtail.core.logging import page_log_action_registry from wagtail.core.query import PageQuerySet, TreeQuerySet from wagtail.core.signals import ( page_published, page_unpublished, post_page_move, pre_page_move, task_approved, task_cancelled, @@ -4720,6 +4721,7 @@ class BaseLogEntry(models.Model): user = models.ForeignKey( settings.AUTH_USER_MODEL, null=True, # Null if actioned by system + blank=True, on_delete=models.DO_NOTHING, db_constraint=False, related_name='+', @@ -4731,12 +4733,24 @@ class BaseLogEntry(models.Model): objects = BaseLogEntryManager() + action_registry = None + class Meta: abstract = True verbose_name = _('log entry') verbose_name_plural = _('log entries') ordering = ['-timestamp'] + def save(self, *args, **kwargs): + self.full_clean() + return super().save(*args, **kwargs) + + def clean(self): + self.action_registry.scan_for_actions() + + if self.action not in self.action_registry.actions: + raise ValidationError({'action': _("The log action '{}' has not been registered.").format(self.action)}) + def __str__(self): return "LogEntry %d: '%s' on '%s'" % ( self.pk, self.action, self.object_verbose_name() @@ -4804,6 +4818,7 @@ class PageLogEntry(BaseLogEntry): revision = models.ForeignKey( 'wagtailcore.PageRevision', null=True, + blank=True, on_delete=models.DO_NOTHING, db_constraint=False, related_name='+', @@ -4811,6 +4826,8 @@ class PageLogEntry(BaseLogEntry): objects = PageLogEntryManager() + action_registry = page_log_action_registry + class Meta: ordering = ['-timestamp', '-id'] verbose_name = _('page log entry') diff --git a/wagtail/core/tests/test_audit_log.py b/wagtail/core/tests/test_audit_log.py index cafd60803b..1237006ee5 100644 --- a/wagtail/core/tests/test_audit_log.py +++ b/wagtail/core/tests/test_audit_log.py @@ -2,6 +2,7 @@ from datetime import datetime, timedelta from django.conf import settings from django.contrib.auth import get_user_model +from django.core.exceptions import ValidationError from django.test import TestCase from django.utils import timezone from freezegun import freeze_time @@ -30,7 +31,7 @@ class TestAuditLogManager(TestCase, WagtailTestUtils): with freeze_time(now): entry = PageLogEntry.objects.log_action( - self.page, 'test', user=self.user + self.page, 'wagtail.edit', user=self.user ) self.assertEqual(entry.content_type, self.page.content_type) @@ -38,8 +39,8 @@ class TestAuditLogManager(TestCase, WagtailTestUtils): self.assertEqual(entry.timestamp, now) def test_get_for_model(self): - PageLogEntry.objects.log_action(self.page, 'test') - PageLogEntry.objects.log_action(self.simple_page, 'test') + PageLogEntry.objects.log_action(self.page, 'wagtail.edit') + PageLogEntry.objects.log_action(self.simple_page, 'wagtail.edit') entries = PageLogEntry.objects.get_for_model(SimplePage) self.assertEqual(entries.count(), 2) @@ -86,8 +87,8 @@ class TestAuditLog(TestCase): self.assertEqual(PageLogEntry.objects.filter(action='wagtail.edit').count(), 1) # passing a string for the action should log this. - self.home_page.save_revision(log_action='wagtail.custom_action') - self.assertEqual(PageLogEntry.objects.filter(action='wagtail.custom_action').count(), 1) + self.home_page.save_revision(log_action='wagtail.revert') + self.assertEqual(PageLogEntry.objects.filter(action='wagtail.revert').count(), 1) def test_page_publish(self): revision = self.home_page.save_revision() @@ -325,8 +326,23 @@ class TestAuditLogHooks(TestCase, WagtailTestUtils): actions = log_actions.get_actions() self.assertIn('wagtail.create', actions) + def test_action_must_be_registered(self): + # We check actions are registered to let developers know if they have forgotten to register + # a new action or made a spelling mistake. It's not intended as a database-level constraint. + with self.assertRaises(ValidationError) as e: + PageLogEntry.objects.log_action(self.root_page, action='test.custom_action') + + self.assertEqual(e.exception.message_dict, { + 'action': ["The log action 'test.custom_action' has not been registered."] + }) + def test_action_format_message(self): - log_entry = PageLogEntry.objects.log_action(self.root_page, action='test.custom_action') + # All new logs should pass our validation, but older logs or logs that were added in bulk + # may be invalid. + # Using LogEntry.objects.update, we can bypass the on save validation. + log_entry = PageLogEntry.objects.log_action(self.root_page, action='wagtail.create') + PageLogEntry.objects.update(action='test.custom_action') + log_entry.refresh_from_db() log_actions = LogActionRegistry('register_log_actions') self.assertEqual(log_actions.format_message(log_entry), "Unknown test.custom_action")