Fix #3240 - Implement is_descendant_of on copy to check to node

This commit is contained in:
Matheus Bratfisch 2016-12-21 16:35:41 +13:00 committed by Matt Westcott
parent c43cf8ad2d
commit 11e92ab7f6
4 changed files with 52 additions and 2 deletions

View file

@ -2198,6 +2198,19 @@ class TestPageCopy(TestCase, WagtailTestUtils):
"This slug is already in use within the context of its parent page \"Welcome to your new Wagtail site!\""
)
def test_page_copy_post_and_subpages_to_same_tree_branch(self):
# This tests the existing slug checking on page copy when changing the parent page
# Attempt to copy the page and changed the parent page
post_data = {
'new_title': "Hello world 2",
'new_slug': 'hello-world',
'new_parent_page': str(self.test_child_page.id),
'copy_subpages': True,
}
response = self.client.post(reverse('wagtailadmin_pages:copy', args=(self.test_page.id,)), post_data)
self.assertEqual(response.status_code, 403)
def test_page_copy_post_existing_slug_to_another_parent_page(self):
# This tests the existing slug checking on page copy when changing the parent page

View file

@ -811,8 +811,8 @@ def copy(request, page_id):
if form.cleaned_data['new_parent_page']:
parent_page = form.cleaned_data['new_parent_page']
# Make sure this user has permission to add subpages on the parent
if not parent_page.permissions_for_user(request.user).can_add_subpage():
if not page.permissions_for_user(request.user).can_copy_to(parent_page,
form.cleaned_data.get('copy_subpages')):
raise PermissionDenied
# Re-check if the user has permission to publish subpages on the new parent

View file

@ -1094,6 +1094,8 @@ class Page(six.with_metaclass(PageBase, AbstractPage, index.Indexed, Clusterable
setattr(page_copy, field, value)
if to:
if recursive and (to == self or to.is_descendant_of(self)):
raise Exception("You cannot copy a tree branch recursively into itself")
page_copy = to.add_child(instance=page_copy)
else:
page_copy = self.add_sibling(instance=page_copy)
@ -1814,6 +1816,30 @@ class PagePermissionTester(object):
# no publishing required, so the already-tested 'add' permission is sufficient
return True
def can_copy_to(self, destination, recursive=False):
# reject the logically impossible cases first
# recursive can't copy to the same tree otherwise it will be on infinite loop
if recursive and (self.page == destination or destination.is_descendant_of(self.page)):
return False
# shortcut the trivial 'everything' / 'nothing' permissions
if not self.user.is_active:
return False
if self.user.is_superuser:
return True
# Inspect permissions on the destination
destination_perms = self.user_perms.for_page(destination)
if not destination.specific_class.creatable_subpage_models():
return False
# we always need at least add permission in the target
if 'add' not in destination_perms.permissions:
return False
return True
class PageViewRestriction(models.Model):
NONE = 'none'

View file

@ -772,6 +772,17 @@ class TestCopyPage(TestCase):
old_christmas_event.specific.revisions.count(), 1, "Revisions were removed from the original page"
)
def test_copy_page_copies_recursively_to_the_same_tree(self):
events_index = EventIndex.objects.get(url_path='/home/events/')
old_christmas_event = events_index.get_children().filter(slug='christmas').first().specific
old_christmas_event.save_revision()
with self.assertRaises(Exception) as exception:
events_index.copy(
recursive=True, update_attrs={'title': "New events index", 'slug': 'new-events-index'}, to=events_index
)
self.assertEqual(str(exception.exception), "You cannot copy a tree branch recursively into itself")
def test_copy_page_updates_user(self):
event_moderator = get_user_model().objects.get(username='eventmoderator')
christmas_event = EventPage.objects.get(url_path='/home/events/christmas/')