Improve logging

- on create: only create and pushlish, no need for revision save
- on edit -> publish, log the draft save and publish separately
- on edit -> submit for moderation, ditto
- on deleting parent, log children deletion
- workflow:
  - log task resubmission (workflow resume)
  - log workflow cancellation

wagtail/admin/tests/test_audit_log.py

@@ -221,3 +221,45 @@ class TestAuditLogAdmin(TestCase, WagtailTestUtils):
         self.assertEqual(response.status_code, 200)
         history_url = reverse('wagtailadmin_pages:history', args=[self.hello_page.id])
         self.assertContains(response, history_url)
+
+    def test_create_and_publish_does_not_log_revision_save(self):
+        self.login(user=self.administrator)
+        post_data = {
+            'title': "New page!",
+            'content': "Some content",
+            'slug': 'hello-world-redux',
+            'action-publish': 'action-publish',
+        }
+        response = self.client.post(
+            reverse('wagtailadmin_pages:add', args=('tests', 'simplepage', self.root_page.id)),
+            post_data, follow=True
+        )
+        self.assertEqual(response.status_code, 200)
+
+        page_id = Page.objects.get(path__startswith=self.root_page.path, slug='hello-world-redux').id
+
+        self.assertListEqual(
+            list(PageLogEntry.objects.filter(page=page_id).values_list('action', flat=True)),
+            ['wagtail.publish', 'wagtail.create']
+        )
+
+    def test_revert_and_publish_logs_reversion_and_publish(self):
+        revision = self.hello_page.save_revision(user=self.editor)
+        self.hello_page.save_revision(user=self.editor)
+
+        self.login(user=self.administrator)
+        response = self.client.post(
+            reverse('wagtailadmin_pages:edit', args=(self.hello_page.id, )),
+            {
+                'title': "Hello World!",
+                'content': "another hello",
+                'slug': 'hello-world',
+                'revision': revision.id, 'action-publish': 'action-publish'}, follow=True
+        )
+        self.assertEqual(response.status_code, 200)
+
+        entries = PageLogEntry.objects.filter(page=self.hello_page).values_list('action', flat=True)
+        self.assertListEqual(
+            list(entries),
+            ['wagtail.publish', 'wagtail.rename', 'wagtail.revert', 'wagtail.create']
+        )

wagtail/admin/views/pages.py

@@ -245,10 +245,7 @@ def create(request, content_type_app_name, content_type_model_name, parent_page_
             parent_page.add_child(instance=page)
 
             # Save revision
-            revision = page.save_revision(
-                user=request.user,
-                log_action=not (is_publishing and is_submitting)
-            )
+            revision = page.save_revision(user=request.user, log_action=False)
 
             # Publish
             if is_publishing:
@@ -512,7 +509,7 @@ def edit(request, page_id):
             # Save revision
             revision = page.save_revision(
                 user=request.user,
-                log_action=not (is_publishing or is_submitting),
+                log_action=True,  # Always log the new revision on edit
                 previous_revision=(previous_revision if is_reverting else None)
             )
             # store submitted go_live_at for messaging below
@@ -548,7 +545,7 @@ def edit(request, page_id):
                 else:
                     # Otherwise start a new workflow
                     workflow = page.get_workflow()
-                    workflow.start(page, request.user, has_content_changes=has_content_changes)
+                    workflow.start(page, request.user)
 
         # Notifications
         if is_publishing:

wagtail/admin/wagtail_hooks.py

@@ -814,17 +814,6 @@ def register_core_log_actions(actions):
         except KeyError:
             return _('Reverted to previous revision')
 
-    def schedule_revert_message(data):
-        try:
-            return format_lazy(
-                _('Scheduled revision {revision_id} from {created_at} for publishing at {go_live_at}.'),
-                revision_id=data['revision']['id'],
-                created_at=data['revision']['created'],
-                go_live_at=data['revision']['go_live_at']
-            )
-        except KeyError:
-            return _('Revision scheduled for publishing')
-
     def copy_message(data):
         try:
             return format_lazy(
@@ -905,11 +894,21 @@ def register_core_log_actions(actions):
         except KeyError:
             return _('Removed view restriction')
 
+    def rename_message(data):
+        try:
+            return format_lazy(
+                _("Renamed from '{old}' to '{new}'"),
+                old=data['title']['old'],
+                new=data['title']['new'],
+            )
+        except KeyError:
+            return _('Renamed')
+
+    actions.register_action('wagtail.rename', _('Rename'), rename_message)
     actions.register_action('wagtail.revert', _('Revert'), revert_message)
-    actions.register_action('wagtail.schedule.revert', _('Schedule revert'), schedule_revert_message)
     actions.register_action('wagtail.copy', _('Copy'), copy_message)
     actions.register_action('wagtail.move', _('Move'), move_message)
-    actions.register_action('wagtail.schedule.publish', _("Schedule publication"), schedule_publish_message)
+    actions.register_action('wagtail.publish.schedule', _("Schedule publication"), schedule_publish_message)
     actions.register_action('wagtail.schedule.cancel', _("Unschedule publication"), unschedule_publish_message)
     actions.register_action('wagtail.view_restriction.create', _("Add view restrictions"), add_view_restriction)
     actions.register_action('wagtail.view_restriction.edit', _("Update view restrictions"), edit_view_restriction)
@@ -948,12 +947,33 @@ def register_workflow_log_actions(actions):
     def workflow_reject_message(data):
         try:
             return format_lazy(
-                _("Rejected at '{task}'. Workflow complete"),
+                _("Rejected at '{task}'. Changes requested"),
                 task=data['workflow']['task']['title'],
             )
         except (KeyError, TypeError):
             return _('Workflow task rejected. Workflow complete')
 
+    def workflow_resume_message(data):
+        try:
+            return format_lazy(
+                _("Resubmitted '{task}'. Workflow resumed'"),
+                task=data['workflow']['task']['title'],
+            )
+        except (KeyError, TypeError):
+            return _('Workflow task resubmitted. Workflow resumed')
+
+    def workflow_cancel_message(data):
+        try:
+            return format_lazy(
+                _("Cancelled '{workflow}' at '{task}'"),
+                workflow=data['workflow']['title'],
+                task=data['workflow']['task']['title'],
+            )
+        except (KeyError, TypeError):
+            return _('Workflow cancelled')
+
     actions.register_action('wagtail.workflow.start', _('Workflow: start'), workflow_start_message)
     actions.register_action('wagtail.workflow.approve', _('Workflow: approve task'), workflow_approve_message)
     actions.register_action('wagtail.workflow.reject', _('Workflow: reject task'), workflow_reject_message)
+    actions.register_action('wagtail.workflow.resume', _('Workflow: resume task'), workflow_resume_message)
+    actions.register_action('wagtail.workflow.cancel', _('Workflow: cancel'), workflow_cancel_message)

wagtail/core/management/commands/publish_scheduled_pages.py

@@ -105,4 +105,4 @@ class Command(BaseCommand):
             for rp in revs_for_publishing:
                 # just run publish for the revision -- since the approved go
                 # live datetime is before now it will make the page live
-                rp.publish()
+                rp.publish(user=rp.user)

wagtail/core/models.py

@@ -671,18 +671,23 @@ class Page(MultiTableCopyMixin, AbstractPage, index.Indexed, ClusterableModel, m
                 self.url_path
             )
 
-            PageLogEntry.objects.log_action(
-                instance=self,
-                action='wagtail.create',
-                user=user or self.owner,
-                content_changed=True,
-            )
-        elif log_action and isinstance(log_action, str):
-            PageLogEntry.objects.log_action(
-                instance=self,
-                action=log_action,
-                user=user
-            )
+        if log_action is not None:
+            # The default for log_action is False. i.e. don't log unless specifically instructed
+            # Page creation is a special case that we want logged by default, but allow skipping it
+            # explicitly by passing log_action=None
+            if is_new:
+                PageLogEntry.objects.log_action(
+                    instance=self,
+                    action='wagtail.create',
+                    user=user or self.owner,
+                    content_changed=True,
+                )
+            elif log_action:
+                PageLogEntry.objects.log_action(
+                    instance=self,
+                    action=log_action,
+                    user=user
+                )
 
         return result
 
@@ -693,13 +698,19 @@ class Page(MultiTableCopyMixin, AbstractPage, index.Indexed, ClusterableModel, m
         if type(self) is Page:
             user = kwargs.pop('user', None)
 
+            def log_deletion(page, user):
+                PageLogEntry.objects.log_action(
+                    instance=page,
+                    action='wagtail.delete',
+                    user=user,
+                    deleted=True,
+                )
+            if self.get_children().exists():
+                for child in self.get_children():
+                    log_deletion(child.specific, user)
+            log_deletion(self.specific, user)
+
             # this is a Page instance, so carry on as we were
-            PageLogEntry.objects.log_action(
-                instance=self.specific,
-                action='wagtail.delete',
-                user=user,
-                deleted=True,
-            )
             return super().delete(*args, **kwargs)
         else:
             # retrieve an actual Page instance and delete that instead of self
@@ -905,7 +916,6 @@ class Page(MultiTableCopyMixin, AbstractPage, index.Indexed, ClusterableModel, m
         # Log
         logger.info("Page edited: \"%s\" id=%d revision_id=%d", self.title, self.id, revision.id)
         if log_action:
-
             if not previous_revision:
                 PageLogEntry.objects.log_action(
                     instance=self,
@@ -1409,10 +1419,10 @@ class Page(MultiTableCopyMixin, AbstractPage, index.Indexed, ClusterableModel, m
         logger.info("Page moved: \"%s\" id=%d path=%s", self.title, self.id, new_url_path)
 
     def copy(self, recursive=False, to=None, update_attrs=None, copy_revisions=True, keep_live=True, user=None,
-             process_child_object=None, exclude_fields=None, log_action=True):
+             process_child_object=None, exclude_fields=None, log_action='wagtail.copy'):
         """
         Copies a given page
-        :param log_action flag for logging the action. Pass False to skip logging.
+        :param log_action flag for logging the action. Pass None to skip logging.
             Can be passed an action string. Defaults to 'wagtail.copy'
         """
 
@@ -1493,7 +1503,7 @@ class Page(MultiTableCopyMixin, AbstractPage, index.Indexed, ClusterableModel, m
             parent = specific_self.get_parent()
             PageLogEntry.objects.log_action(
                 instance=page_copy,
-                action=log_action if isinstance(log_action, str) else 'wagtail.copy',
+                action=log_action,
                 user=user,
                 data={
                     'page': {
@@ -1505,6 +1515,14 @@ class Page(MultiTableCopyMixin, AbstractPage, index.Indexed, ClusterableModel, m
                     'keep_live': page_copy.live and keep_live
                 },
             )
+            if page_copy.live and keep_live:
+                # Log the publish if the use chose to keep the copied page live
+                PageLogEntry.objects.log_action(
+                    instance=page_copy,
+                    action='wagtail.publish',
+                    user=user,
+                    revision=latest_revision_as_page_revision,
+                )
         logger.info("Page copied: \"%s\" id=%d from=%d", page_copy.title, page_copy.id, self.id)
 
         # Copy child pages
@@ -2004,22 +2022,15 @@ class PageRevision(models.Model):
         """
         page = self.as_page_object()
 
-        def log_scheduling_action(revision, previous_revision=None, user=None, changed=changed):
-            if not previous_revision:
-                action = 'wagtail.schedule.publish'
-                revision_for_data = self
-            else:
-                action = 'wagtail.schedule.revert'
-                revision_for_data = previous_revision
-
+        def log_scheduling_action(revision, user=None, changed=changed):
             PageLogEntry.objects.log_action(
                 instance=page,
-                action=action,
+                action='wagtail.publish.schedule',
                 user=user,
                 data={
                     'revision': {
-                        'id': revision_for_data.id,
-                        'created': revision_for_data.created_at.strftime("%d %b %Y %H:%M"),
+                        'id': revision.id,
+                        'created': revision.created_at.strftime("%d %b %Y %H:%M"),
                         'go_live_at': page.go_live_at.strftime("%d %b %Y %H:%M"),
                         'has_live_version': page.live,
                     }
@@ -2039,7 +2050,7 @@ class PageRevision(models.Model):
             if page.live_revision:
                 # Log scheduled publishing
                 if log_action:
-                    log_scheduling_action(self, previous_revision, user, changed)
+                    log_scheduling_action(self, user, changed)
 
                 return
             # if we have a go_live in the future don't make the page live
@@ -2061,6 +2072,16 @@ class PageRevision(models.Model):
 
             if page.first_published_at is None:
                 page.first_published_at = now
+
+            if previous_revision:
+                previous_revision_page = previous_revision.as_page_object()
+                old_page_title = previous_revision_page.title if page.title != previous_revision_page.title else None
+            else:
+                try:
+                    previous = self.get_previous()
+                except PageRevision.DoesNotExist:
+                    previous = None
+                old_page_title = previous.page.title if previous and page.title != previous.page.title else None
         else:
             # Unset live_revision if the page is going live in the future
             page.live_revision = None
@@ -2073,20 +2094,33 @@ class PageRevision(models.Model):
             page_published.send(sender=page.specific_class, instance=page.specific, revision=self)
 
             if log_action:
-                if not previous_revision:
-                    action = 'wagtail.publish'
-                    data = None
-                else:
-                    action = 'wagtail.revert'
+                data = None
+                if previous_revision:
                     data = {
                         'revision': {
                             'id': previous_revision.id,
                             'created': previous_revision.created_at.strftime("%d %b %Y %H:%M")
                         }
                     }
+
+                if old_page_title:
+                    data = data or {}
+                    data['title'] = {
+                        'old': old_page_title,
+                        'new': page.title,
+                    }
+
+                    PageLogEntry.objects.log_action(
+                        instance=page,
+                        action='wagtail.rename',
+                        user=user,
+                        data=data,
+                        revision=self,
+                    )
+
                 PageLogEntry.objects.log_action(
                     instance=page,
-                    action=action,
+                    action='wagtail.publish',
                     user=user,
                     data=data,
                     revision=self,
@@ -2104,7 +2138,7 @@ class PageRevision(models.Model):
             )
 
             if log_action:
-                log_scheduling_action(self, previous_revision, user, changed)
+                log_scheduling_action(self, user, changed)
 
     def get_previous(self):
         return self.get_previous_by_created_at(page=self.page)
@@ -3012,7 +3046,7 @@ class Workflow(ClusterableModel):
         return Task.objects.filter(workflow_tasks__workflow=self).order_by('workflow_tasks__sort_order')
 
     @transaction.atomic
-    def start(self, page, user, has_content_changes=True):
+    def start(self, page, user):
         """Initiates a workflow by creating an instance of ``WorkflowState``"""
         state = WorkflowState(page=page, workflow=self, status=WorkflowState.STATUS_IN_PROGRESS, requested_by=user)
         state.save()
@@ -3034,11 +3068,11 @@ class Workflow(ClusterableModel):
                     'title': self.name,
                     'status': state.status,
                     'next': next_task_data,
+                    'task_state_id': state.current_task_state.id if state.current_task_state else None,
                 }
             },
             revision=page.get_latest_revision(),
             user=user,
-            content_changed=has_content_changes,
         )
 
         return state
@@ -3184,11 +3218,31 @@ class WorkflowState(models.Model):
         """Put a STATUS_NEEDS_CHANGES workflow state back into STATUS_IN_PROGRESS, and restart the current task"""
         if self.status != self.STATUS_NEEDS_CHANGES:
             raise PermissionDenied
-        next_task = self.current_task_state.task
+        revision = self.current_task_state.page_revision
+        current_task_state = self.current_task_state
         self.current_task_state = None
         self.status = self.STATUS_IN_PROGRESS
         self.save()
-        return self.update(user=user, next_task=next_task)
+
+        PageLogEntry.objects.log_action(
+            instance=self.page.specific,
+            action='wagtail.workflow.resume',
+            data={
+                'workflow': {
+                    'id': self.workflow_id,
+                    'title': self.workflow.name,
+                    'status': self.status,
+                    'task_state_id': current_task_state.id,
+                    'task': {
+                        'id': current_task_state.task.id,
+                        'title': current_task_state.task.name,
+                    },
+                }
+            },
+            revision=revision,
+            user=user,
+        )
+        return self.update(user=user, next_task=current_task_state.task)
 
     def user_can_cancel(self, user):
         if self.page.locked and self.page.locked_by != user:
@@ -3246,6 +3300,26 @@ class WorkflowState(models.Model):
             raise PermissionDenied
         self.status = self.STATUS_CANCELLED
         self.save()
+
+        PageLogEntry.objects.log_action(
+            instance=self.page.specific,
+            action='wagtail.workflow.cancel',
+            data={
+                'workflow': {
+                    'id': self.workflow_id,
+                    'title': self.workflow.name,
+                    'status': self.status,
+                    'task_state_id': self.current_task_state.id,
+                    'task': {
+                        'id': self.current_task_state.task.id,
+                        'title': self.current_task_state.task.name,
+                    },
+                }
+            },
+            revision=self.current_task_state.page_revision,
+            user=user,
+        )
+
         for state in self.task_states.filter(status=TaskState.STATUS_IN_PROGRESS):
             # Cancel all in progress task states
             state.specific.cancel(user=user)
@@ -3258,7 +3332,7 @@ class WorkflowState(models.Model):
             raise PermissionDenied
         self.status = self.STATUS_APPROVED
         self.save()
-        self.on_finish()
+        self.on_finish(user=user)
         workflow_approved.send(sender=self.__class__, instance=self, user=user)
 
     def copy_approved_task_states_to_revision(self, revision):
@@ -3414,7 +3488,7 @@ class TaskState(MultiTableCopyMixin, models.Model):
         self.comment = comment
         self.save()
 
-        self.log_action(user, 'approve')
+        self.log_state_change_action(user, 'approve')
         if update:
             self.workflow_state.update(user=user)
         task_approved.send(sender=self.specific.__class__, instance=self.specific, user=user)
@@ -3431,7 +3505,7 @@ class TaskState(MultiTableCopyMixin, models.Model):
         self.comment = comment
         self.save()
 
-        self.log_action(user, 'reject')
+        self.log_state_change_action(user, 'reject')
         if update:
             self.workflow_state.update(user=user)
         task_rejected.send(sender=self.specific.__class__, instance=self.specific, user=user)
@@ -3485,7 +3559,7 @@ class TaskState(MultiTableCopyMixin, models.Model):
         """
         return self.comment
 
-    def log_action(self, user, action):
+    def log_state_change_action(self, user, action):
         """Log the approval/rejection action"""
         page = self.page_revision.as_page_object()
         next_task = self.workflow_state.get_next_task()
@@ -3504,12 +3578,14 @@ class TaskState(MultiTableCopyMixin, models.Model):
                     'id': self.workflow_state.workflow.id,
                     'title': self.workflow_state.workflow.name,
                     'status': self.status,
+                    'task_state_id': self.id,
                     'task': {
                         'id': self.task.id,
                         'title': self.task.name,
                     },
                     'next': next_task_data,
-                }
+                },
+                'comment': self.get_comment()
             },
             revision=self.page_revision
         )

wagtail/core/tests/test_audit_log.py

@@ -58,13 +58,15 @@ class TestAuditLog(TestCase):
             instance=SimplePage(title="Homepage", slug="home2", content="hello")
         )
 
+        PageLogEntry.objects.all().delete()  # clean up the log entries here.
+
     def test_page_create(self):
-        self.assertEqual(PageLogEntry.objects.count(), 1)  # homepage
+        self.assertEqual(PageLogEntry.objects.count(), 0)  # homepage
 
         page = self.home_page.add_child(
             instance=SimplePage(title="Hello", slug="my-page", content="world")
         )
-        self.assertEqual(PageLogEntry.objects.count(), 2)
+        self.assertEqual(PageLogEntry.objects.count(), 1)
         log_entry = PageLogEntry.objects.order_by('pk').last()
         self.assertEqual(log_entry.action, 'wagtail.create')
         self.assertEqual(log_entry.page_id, page.id)
@@ -74,12 +76,12 @@ class TestAuditLog(TestCase):
     def test_page_edit(self):
         # Directly saving a revision should not yield a log entry
         self.home_page.save_revision()
-        self.assertEqual(PageLogEntry.objects.count(), 1)
+        self.assertEqual(PageLogEntry.objects.count(), 0)
 
         # Explicitly ask to record the revision change
         self.home_page.save_revision(log_action=True)
-        self.assertEqual(PageLogEntry.objects.count(), 2)
-        self.assertEqual(PageLogEntry.objects.filter(action='wagtail.create').count(), 1)
+        self.assertEqual(PageLogEntry.objects.count(), 1)
+        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')
@@ -88,12 +90,32 @@ class TestAuditLog(TestCase):
     def test_page_publish(self):
         revision = self.home_page.save_revision()
         revision.publish()
-        self.assertEqual(PageLogEntry.objects.count(), 2)
+        self.assertEqual(PageLogEntry.objects.count(), 1)
         self.assertEqual(PageLogEntry.objects.filter(action='wagtail.publish').count(), 1)
 
+    def test_page_rename(self):
+        # Should not log a name change when publishing the first revision
+        revision = self.home_page.save_revision()
+        self.home_page.title = "Old title"
+        self.home_page.save()
+        revision.publish()
+
+        self.assertEqual(PageLogEntry.objects.filter(action='wagtail.publish').count(), 1)
+        self.assertEqual(PageLogEntry.objects.filter(action='wagtail.rename').count(), 0)
+
+        # Now, check the rename is logged
+        revision = self.home_page.save_revision()
+        self.home_page.title = "New title"
+        self.home_page.save()
+        revision.publish()
+
+        self.assertEqual(PageLogEntry.objects.count(), 3)
+        self.assertEqual(PageLogEntry.objects.filter(action='wagtail.publish').count(), 2)
+        self.assertEqual(PageLogEntry.objects.filter(action='wagtail.rename').count(), 1)
+
     def test_page_unpublish(self):
         self.home_page.unpublish()
-        self.assertEqual(PageLogEntry.objects.count(), 2)
+        self.assertEqual(PageLogEntry.objects.count(), 1)
         self.assertEqual(PageLogEntry.objects.filter(action='wagtail.unpublish').count(), 1)
 
     def test_revision_revert(self):
@@ -111,20 +133,23 @@ class TestAuditLog(TestCase):
         revision = self.home_page.save_revision()
         revision.publish()
 
-        log_entries = PageLogEntry.objects.filter(action='wagtail.schedule.publish')
+        log_entries = PageLogEntry.objects.filter(action='wagtail.publish.schedule')
         self.assertEqual(log_entries.count(), 1)
         self.assertEqual(log_entries[0].data['revision']['id'], revision.id)
         self.assertEqual(log_entries[0].data['revision']['go_live_at'], go_live_at.strftime("%d %b %Y %H:%M"))
 
     def test_revision_schedule_revert(self):
         revision1 = self.home_page.save_revision()
-        self.home_page.save_revision()
+        revision2 = self.home_page.save_revision()
 
         self.home_page.go_live_at = timezone.make_aware(datetime.now() + timedelta(days=1))
-        schedule_revision = self.home_page.save_revision()
+        schedule_revision = self.home_page.save_revision(log_action=True, previous_revision=revision2)
         schedule_revision.publish(previous_revision=revision1)
 
-        self.assertEqual(PageLogEntry.objects.filter(action='wagtail.schedule.revert').count(), 1)
+        self.assertListEqual(
+            list(PageLogEntry.objects.values_list('action', flat=True)),
+            ['wagtail.publish.schedule', 'wagtail.revert']  # order_by -timestamp, by default
+        )
 
     def test_revision_cancel_schedule(self):
         self.home_page.go_live_at = timezone.make_aware(datetime.now() + timedelta(days=1))
@@ -144,7 +169,11 @@ class TestAuditLog(TestCase):
 
     def test_page_copy(self):
         self.home_page.copy(update_attrs={'title': "About us", 'slug': 'about-us'})
-        self.assertEqual(PageLogEntry.objects.filter(action='wagtail.copy').count(), 1)
+
+        self.assertListEqual(
+            list(PageLogEntry.objects.values_list('action', flat=True)),
+            ['wagtail.publish', 'wagtail.copy', 'wagtail.create']
+        )
 
     def test_page_move(self):
         section = self.root_page.add_child(
@@ -155,19 +184,31 @@ class TestAuditLog(TestCase):
         self.assertEqual(PageLogEntry.objects.filter(action='wagtail.move').count(), 1)
 
     def test_page_delete(self):
-        self.home_page.delete()
+        self.home_page.add_child(
+            instance=SimplePage(title="Child", slug="child-page", content="hello")
+        )
+        child = self.home_page.add_child(
+            instance=SimplePage(title="Another child", slug="child-page-2", content="hello")
+        )
+
+        child.delete()
         self.assertEqual(PageLogEntry.objects.filter(action='wagtail.delete').count(), 1)
 
-    def _create_workflow_and_tasks(self):
+        # check deleting a parent page logs child deletion
+        self.home_page.delete()
+        self.assertEqual(PageLogEntry.objects.filter(action='wagtail.delete').count(), 3)
+        self.assertListEqual(
+            list(PageLogEntry.objects.filter(action='wagtail.delete').values_list('label', flat=True)),
+            ['Homepage (simple page)', 'Child (simple page)', 'Another child (simple page)']
+        )
+
+    def test_workflow_actions(self):
         workflow = Workflow.objects.create(name='test_workflow')
         task_1 = Task.objects.create(name='test_task_1')
         task_2 = Task.objects.create(name='test_task_2')
         WorkflowTask.objects.create(workflow=workflow, task=task_1, sort_order=1)
         WorkflowTask.objects.create(workflow=workflow, task=task_2, sort_order=2)
-        return workflow, task_1, task_2
 
-    def test_workflow_actions(self):
-        workflow, _, _ = self._create_workflow_and_tasks()
         self.home_page.save_revision()
         user = get_user_model().objects.first()
         workflow_state = workflow.start(self.home_page, user)
@@ -179,6 +220,7 @@ class TestAuditLog(TestCase):
                 'id': workflow.id,
                 'title': workflow.name,
                 'status': workflow_state.status,
+                'task_state_id': workflow_state.current_task_state_id,
                 'next': {
                     'id': workflow_state.current_task_state.task.id,
                     'title': workflow_state.current_task_state.task.name,
@@ -200,6 +242,7 @@ class TestAuditLog(TestCase):
                         'id': workflow.id,
                         'title': workflow.name,
                         'status': task_state.status,
+                        'task_state_id': task_state.id,
                         'task': {
                             'id': task_state.task.id,
                             'title': task_state.task.name,
@@ -208,9 +251,27 @@ class TestAuditLog(TestCase):
                             'id': workflow_state.current_task_state.task.id,
                             'title': workflow_state.current_task_state.task.name,
                         },
-                    }
+                    },
+                    'comment': '',
                 })
 
+    def test_workflow_completions_logs_publishing_user(self):
+        workflow = Workflow.objects.create(name='test_workflow')
+        task_1 = Task.objects.create(name='test_task_1')
+        WorkflowTask.objects.create(workflow=workflow, task=task_1, sort_order=1)
+
+        self.assertFalse(PageLogEntry.objects.filter(action='wagtail.publish').exists())
+
+        self.home_page.save_revision()
+        user = get_user_model().objects.first()
+        workflow_state = workflow.start(self.home_page, user)
+
+        publisher = get_user_model().objects.last()
+        task_state = workflow_state.current_task_state
+        task_state.task.on_action(task_state, user=None, action_name='approve')
+
+        self.assertEqual(PageLogEntry.objects.get(action='wagtail.publish').user, publisher)
+
     def test_page_privacy(self):
         restriction = PageViewRestriction.objects.create(page=self.home_page)
         self.assertEqual(PageLogEntry.objects.filter(action='wagtail.view_restriction.create').count(), 1)

wagtail/core/workflows.py

@@ -23,9 +23,9 @@ def get_task_types(task_class=None):
     return TASK_TYPES
 
 
-def publish_workflow_state(workflow_state):
+def publish_workflow_state(workflow_state, user=None):
     # publish the Page associated with a WorkflowState
     if workflow_state.current_task_state:
-        workflow_state.current_task_state.page_revision.publish()
+        workflow_state.current_task_state.page_revision.publish(user=user)
     else:
-        workflow_state.page.get_latest_revision().publish()
+        workflow_state.page.get_latest_revision().publish(user=user)