Raise an error on save if a LogEntry is created with invalid action

This is just to catch mistakes as they're made. Not intended for database consistency.

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',

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')

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")