Add pre_page_move and post_page_move signals

* Documentation for pre_page_move and post_page_move signals
* Resolves #2728

CHANGELOG.txt

@@ -11,6 +11,7 @@ Changelog
  * Redirect to previous url when deleting/copying/unpublish a page and modify this url via the relevant hooks (Ascani Carlo)
  * `AbstractEmailForm` will use `SHORT_DATETIME_FORMAT` and `SHORT_DATE_FORMAT` Django settings to format date/time values in email (Haydn Greatnews)
  * `AbstractEmailForm` now has a separate method (`render_email`) to build up email content on submission emails (Haydn Greatnews)
+ * Add `pre_page_move` and `post_page_move` signals (Andy Babic)
  * Fix: Ensure link to add a new user works when no users are visible in the users list (LB (Ben Johnston))
  * Fix: `AbstractEmailForm` saved submission fields are now aligned with the email content fields, `form.cleaned_data` will be used instead of `form.fields` (Haydn Greatnews)
  * Fix: Removed ARIA `role="table"` from TableBlock output (Thibaud Colas)

docs/reference/signals.rst

@@ -10,14 +10,14 @@ Signals are useful for creating side-effects from page publish/unpublish events.
 For example, you could use signals to send publish notifications to a messaging service, or ``POST`` messages to another app that's consuming the API, such as a static site generator.
 
 
-page_published
---------------
+``page_published``
+------------------
 
 This signal is emitted from a ``PageRevision`` when a revision is set to `published`.
 
-:sender: The page ``class``
+:sender: The page ``class``.
 :instance: The specific ``Page`` instance.
-:revision: The ``PageRevision`` that was published
+:revision: The ``PageRevision`` that was published.
 :kwargs: Any other arguments passed to ``page_published.send()``.
 
 To listen to a signal, implement ``page_published.connect(receiver, sender, **kwargs)``. Here's a simple
@@ -74,11 +74,60 @@ Wagtail provides access to a list of registered page types through the ``get_pag
 Read the :ref:`Django documentation <connecting-to-specific-signals>` for more information about specifying senders.
 
 
-page_unpublished
-----------------
+``page_unpublished``
+--------------------
 
 This signal is emitted from a ``Page`` when the page is unpublished.
 
-:sender: The page ``class``
+:sender: The page ``class``.
 :instance: The specific ``Page`` instance.
 :kwargs: Any other arguments passed to ``page_unpublished.send()``
+
+
+``pre_page_move`` and ``post_page_move``
+------------------------------------------
+
+.. versionadded:: 2.10
+
+These signals are emitted from a ``Page`` immediately before and after it is moved.
+
+Subscribe to ``pre_page_move`` if you need to know values BEFORE any database changes are applied. For example: Getting the page's previous URL, or that of its descendants.
+
+Subscribe to ``post_page_move`` if you need to know values AFTER database changes have been applied. For example: Getting the page's new URL, or that of its descendants.
+
+The following arguments are emitted for both signals:
+
+:sender: The page ``class``.
+:instance: The specific ``Page`` instance.
+:parent_page_before: The parent page of ``instance`` **before** moving.
+:parent_page_after: The parent page of ``instance`` **after** moving.
+:url_path_before: The value of ``instance.url_path`` **before** moving.
+:url_path_after: The value of ``instance.url_path`` **after** moving.
+:kwargs: Any other arguments passed to ``pre_page_move.send()`` or ``post_page_move.send()``.
+
+
+Distinguishing between a 'move' and a 'reorder'
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The signal can be emitted as a result of a page being moved to a different section (a 'move'), or as a result of a page being moved to a different position within the same section (a 'reorder'). Knowing the difference between the two can be particularly useful, because only a 'move' affects a page's URL (and that of its descendants), whereas a 'reorder' only affects the natural page order; which is probably less impactful.
+
+The best way to distinguish between a 'move' and 'reorder' is to compare the ``url_path_before`` and ``url_path_after`` values. For example:
+
+.. code-block:: python
+
+    from wagtail.core.signals import pre_page_move
+    from wagtail.contrib.frontend_cache.utils import purge_page_from_cache
+
+    # Clear a page's old URLs from the cache when it moves to a different section
+    def clear_page_url_from_cache_on_move(sender, **kwargs):
+
+        if kwargs['url_path_before'] == kwargs['url_path_after']:
+            # No URLs are changing :) nothing to do here!
+            return
+
+        # The page is moving to a new section (possibly even a new site)
+        # so clear old URL(s) from the cache
+        purge_page_from_cache(kwargs['instance'])
+
+    # Register a receiver
+    pre_page_move.connect(clear_old_page_urls_from_cache)

docs/releases/2.10.rst

@@ -20,6 +20,7 @@ Other features
  * Redirect to previous url when deleting/copying/unpublish a page and modify this url via the relevant hooks (Ascani Carlo)
  * ``AbstractEmailForm`` will use ``SHORT_DATETIME_FORMAT`` and ``SHORT_DATE_FORMAT`` Django settings to format date/time values in email (Haydn Greatnews)
  * ``AbstractEmailForm`` now has a separate method (``render_email``) to build up email content on submission emails. See :ref:`form_builder_render_email`. (Haydn Greatnews)
+ * Add `pre_page_move` and `post_page_move` signals. (Andy Babic)
 
 
 Bug fixes

wagtail/admin/tests/pages/test_move_page.py

@@ -1,3 +1,5 @@
+from unittest import mock
+
 from django.contrib.auth.models import Permission
 from django.contrib.messages import constants as message_constants
 from django.http import HttpRequest, HttpResponse
@@ -5,6 +7,7 @@ from django.test import TestCase
 from django.urls import reverse
 
 from wagtail.core.models import Page
+from wagtail.core.signals import post_page_move, pre_page_move
 from wagtail.tests.testapp.models import SimplePage
 from wagtail.tests.utils import WagtailTestUtils
 
@@ -72,6 +75,46 @@ class TestPageMove(TestCase, WagtailTestUtils):
         # Slug should be in error message.
         self.assertIn("{}".format(self.test_page_b.slug), messages[0].message)
 
+    def test_move_triggers_signals(self):
+        # Connect a mock signal handler to pre_page_move and post_page_move signals
+        pre_moved_handler = mock.MagicMock()
+        post_moved_handler = mock.MagicMock()
+
+        pre_page_move.connect(pre_moved_handler)
+        post_page_move.connect(post_moved_handler)
+
+        # Post to view to move page
+        try:
+            self.client.post(
+                reverse('wagtailadmin_pages:move_confirm', args=(self.test_page_a.id, self.section_b.id))
+            )
+        finally:
+            # Disconnect mock handler to prevent cross-test pollution
+            pre_page_move.disconnect(pre_moved_handler)
+            post_page_move.disconnect(post_moved_handler)
+
+        # Check that the pre_page_move signal was fired
+        self.assertEqual(pre_moved_handler.call_count, 1)
+        self.assertTrue(pre_moved_handler.called_with(
+            sender=self.test_page_a.specific_class,
+            instance=self.test_page_a,
+            parent_page_before=self.section_a,
+            parent_page_after=self.section_b,
+            url_path_before='/home/section-a/hello-world/',
+            url_path_after='/home/section-b/hello-world/',
+        ))
+
+        # Check that the post_page_move signal was fired
+        self.assertEqual(post_moved_handler.call_count, 1)
+        self.assertTrue(post_moved_handler.called_with(
+            sender=self.test_page_a.specific_class,
+            instance=self.test_page_a,
+            parent_page_before=self.section_a,
+            parent_page_after=self.section_b,
+            url_path_before='/home/section-a/hello-world/',
+            url_path_after='/home/section-b/hello-world/',
+        ))
+
     def test_page_set_page_position(self):
         response = self.client.get(reverse('wagtailadmin_pages:set_page_position', args=(self.test_page_a.id, )))
         self.assertEqual(response.status_code, 200)

wagtail/core/models.py

@@ -27,7 +27,7 @@ from modelcluster.models import (
 from treebeard.mp_tree import MP_Node
 
 from wagtail.core.query import PageQuerySet, TreeQuerySet
-from wagtail.core.signals import page_published, page_unpublished
+from wagtail.core.signals import page_published, page_unpublished, post_page_move, pre_page_move
 from wagtail.core.sites import get_site_for_hostname
 from wagtail.core.url_routing import RouteResult
 from wagtail.core.utils import WAGTAIL_APPEND_SLASH, camelcase_to_underscore, resolve_model_string
@@ -1090,19 +1090,58 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
         """
         return (not self.live) and (not self.get_descendants().filter(live=True).exists())
 
-    @transaction.atomic  # only commit when all descendants are properly updated
     def move(self, target, pos=None):
         """
-        Extension to the treebeard 'move' method to ensure that url_path is updated too.
+        Extension to the treebeard 'move' method to ensure that url_path is updated,
+        and to emit a 'pre_page_move' and 'post_page_move' signals.
         """
-        old_url_path = Page.objects.get(id=self.id).url_path
-        super().move(target, pos=pos)
-        # treebeard's move method doesn't actually update the in-memory instance, so we need to work
-        # with a freshly loaded one now
-        new_self = Page.objects.get(id=self.id)
-        new_url_path = new_self.set_url_path(new_self.get_parent())
-        new_self.save()
-        new_self._update_descendant_url_paths(old_url_path, new_url_path)
+        # Determine old and new parents
+        parent_before = self.get_parent()
+        if pos in ('first-child', 'last-child', 'sorted-child'):
+            parent_after = target
+        else:
+            parent_after = target.get_parent()
+
+        # Determine old and new url_paths
+        # Fetching new object to avoid affecting `self`
+        old_self = Page.objects.get(id=self.id)
+        old_url_path = old_self.url_path
+        new_url_path = old_self.set_url_path(parent=parent_after)
+
+        # Emit pre_page_move signal
+        pre_page_move.send(
+            sender=self.specific_class or self.__class__,
+            instance=self,
+            parent_page_before=parent_before,
+            parent_page_after=parent_after,
+            url_path_before=old_url_path,
+            url_path_after=new_url_path,
+        )
+
+        # Only commit when all descendants are properly updated
+        with transaction.atomic():
+            # Allow treebeard to update `path` values
+            super().move(target, pos=pos)
+
+            # Treebeard's move method doesn't actually update the in-memory instance,
+            # so we need to work with a freshly loaded one now
+            new_self = Page.objects.get(id=self.id)
+            new_self.url_path = new_url_path
+            new_self.save()
+
+            # Update descendant paths if url_path has changed
+            if old_url_path != new_url_path:
+                new_self._update_descendant_url_paths(old_url_path, new_url_path)
+
+        # Emit post_page_move signal
+        post_page_move.send(
+            sender=self.specific_class or self.__class__,
+            instance=new_self,
+            parent_page_before=parent_before,
+            parent_page_after=parent_after,
+            url_path_before=old_url_path,
+            url_path_after=new_url_path,
+        )
 
         # Log
         logger.info("Page moved: \"%s\" id=%d path=%s", self.title, self.id, new_url_path)

wagtail/core/signals.py

@@ -2,3 +2,5 @@ from django.dispatch import Signal
 
 page_published = Signal(providing_args=['instance', 'revision'])
 page_unpublished = Signal(providing_args=['instance'])
+pre_page_move = Signal(providing_args=['instance', 'parent_page_before', 'parent_page_after', 'url_path_before', 'url_path_after'])
+post_page_move = Signal(providing_args=['instance', 'parent_page_before', 'parent_page_after', 'url_path_before', 'url_path_after'])