diff --git a/CHANGELOG.txt b/CHANGELOG.txt index b156595e6d..6e6a25e481 100644 --- a/CHANGELOG.txt +++ b/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) diff --git a/docs/releases/2.10.rst b/docs/releases/2.10.rst index d60ce04e27..9352b9c387 100644 --- a/docs/releases/2.10.rst +++ b/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 diff --git a/wagtail/admin/tests/pages/test_move_page.py b/wagtail/admin/tests/pages/test_move_page.py index 9a92f61a2c..6b1d350b8f 100644 --- a/wagtail/admin/tests/pages/test_move_page.py +++ b/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)) diff --git a/wagtail/core/models.py b/wagtail/core/models.py index 036f9a8422..388ef44938 100644 --- a/wagtail/core/models.py +++ b/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