Page can_move permission method to ignore bulk_delete permission check

* bulk_delete exists to avoid allowing users to delete a whole lot of pages all at once by mistake, when they think they're just deleting one. However, the move/delete analogue breaks down when you're talking about moving an entire subtree, because you're *not* directly altering the children of the moved Page.

CHANGELOG.txt

@@ -29,6 +29,7 @@ Changelog
  * Improve performance of public/not_public queries in `PageQuerySet` (Timothy Bautista)
  * Add `add_redirect` static method to `Redirect` class for programmatic redirect creation (Brylie Christopher Oxley, Lacey Williams Henschel)
  * Add reference documentation for `wagtail.contrib.redirects` (LB (Ben Johnston))
+ * `bulk_delete` page permission is no longer required to move pages, even if those pages have children (Robert Rollins, LB (Ben Johnston))
  * Fix: Support IPv6 domain (Alex Gleason, Coen van der Kamp)
  * 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)

docs/releases/2.10.rst

@@ -38,6 +38,7 @@ Other features
  * Improve performance of public/not_public queries in ``PageQuerySet`` (Timothy Bautista)
  * Add ``add_redirect`` static method to ``Redirect`` class for programmatic redirect creation (Brylie Christopher Oxley, Lacey Williams Henschel)
  * Add reference documentation for ``wagtail.contrib.redirects`` See :ref:`redirects`. (LB (Ben Johnston))
+ * ``bulk_delete`` page permission is no longer required to move pages, even if those pages have children (Robert Rollins, LB (Ben Johnston))
 
 
 Bug fixes

wagtail/admin/tests/pages/test_move_page.py

@@ -1,5 +1,6 @@
 from unittest import mock
 
+from django.contrib.auth import get_user_model
 from django.contrib.auth.models import Permission
 from django.contrib.messages import constants as message_constants
 from django.http import HttpRequest, HttpResponse
@@ -13,6 +14,8 @@ from wagtail.tests.utils import WagtailTestUtils
 
 
 class TestPageMove(TestCase, WagtailTestUtils):
+    fixtures = ['test.json']
+
     def setUp(self):
         # Find root page
         self.root_page = Page.objects.get(id=2)
@@ -35,6 +38,16 @@ class TestPageMove(TestCase, WagtailTestUtils):
         self.test_page_b = SimplePage(title="Hello world!", slug="hello-world", content="hello")
         self.section_c.add_child(instance=self.test_page_b)
 
+        # Add unpublished page to the root with a child page
+        self.unpublished_page = SimplePage(title="Unpublished", slug="unpublished", content="hello")
+        sub_page = SimplePage(title="Sub Page", slug="sub-page", content="child")
+        self.root_page.add_child(instance=self.unpublished_page)
+        self.unpublished_page.add_child(instance=sub_page)
+
+        # unpublish pages last (used to validate the edit only permission)
+        self.unpublished_page.unpublish()
+        sub_page.unpublish()
+
         # Login
         self.user = self.login()
 
@@ -56,6 +69,24 @@ class TestPageMove(TestCase, WagtailTestUtils):
         # Check that the user received a 403 response
         self.assertEqual(response.status_code, 403)
 
+    def test_user_without_bulk_delete_permission_can_move(self):
+        # to verify that a user without bulk delete permission is able to move a page with a child page
+
+        self.client.logout()
+        user = get_user_model().objects.get(username='siteeditor')
+        self.login(user)
+
+        # ensure the bulk_delete is not applicable to this user
+        can_bulk_delete = self.test_page_b.permissions_for_user(user).can_delete()
+        self.assertFalse(can_bulk_delete)
+
+        response = self.client.get(
+            reverse('wagtailadmin_pages:move', args=(self.unpublished_page.id, ))
+        )
+
+        self.assertEqual(response.status_code, 200)
+
+
     def test_page_move_confirm(self):
         response = self.client.get(
             reverse('wagtailadmin_pages:move_confirm', args=(self.test_page_a.id, self.section_b.id))

wagtail/core/models.py

@@ -1994,9 +1994,10 @@ class PagePermissionTester:
             or ('add' in self.permissions and self.page.owner_id == self.user.pk)
         )
 
-    def can_delete(self):
+    def can_delete(self, ignore_bulk=False):
         if not self.user.is_active:
             return False
+
         if self.page_is_root:  # root node is not a page and can never be deleted, even by superusers
             return False
 
@@ -2005,7 +2006,7 @@ class PagePermissionTester:
             return True
 
         # if the user does not have bulk_delete permission, they may only delete leaf pages
-        if 'bulk_delete' not in self.permissions and not self.page.is_leaf():
+        if 'bulk_delete' not in self.permissions and not self.page.is_leaf() and not ignore_bulk:
             return False
 
         if 'edit' in self.permissions:
@@ -2090,7 +2091,7 @@ class PagePermissionTester:
         As such, the permission test for 'can this be moved at all?' should be the same as for deletion.
         (Further constraints will then apply on where it can be moved *to*.)
         """
-        return self.can_delete()
+        return self.can_delete(ignore_bulk=True)
 
     def can_copy(self):
         return not self.page_is_root