From 71089dc6d5e2cd0ebe09486a782b03562fef617c Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 11 Jul 2014 11:21:41 +0100 Subject: [PATCH] Added option to copy form to publish copied pages. This is not displayed to users without publish permissions. --- wagtail/wagtailadmin/forms.py | 8 +- .../templates/wagtailadmin/pages/copy.html | 11 +- .../wagtailadmin/tests/test_pages_views.py | 185 +++++++++++++++++- wagtail/wagtailadmin/views/pages.py | 19 +- 4 files changed, 204 insertions(+), 19 deletions(-) diff --git a/wagtail/wagtailadmin/forms.py b/wagtail/wagtailadmin/forms.py index 49fa2053c..a8da2525e 100644 --- a/wagtail/wagtailadmin/forms.py +++ b/wagtail/wagtailadmin/forms.py @@ -78,12 +78,8 @@ class PasswordResetForm(PasswordResetForm): return cleaned_data -YES_OR_NO = ( - (True, ugettext_lazy("Yes")), - (False, ugettext_lazy("No")), -) - class CopyForm(forms.Form): new_title = forms.CharField() new_slug = forms.CharField() - copy_subpages = forms.BooleanField(widget=forms.RadioSelect(choices=(YES_OR_NO)), required=False) + copy_subpages = forms.BooleanField(required=False) + publish_copies = forms.BooleanField(required=False) diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/copy.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/copy.html index d93f9e9b9..019bee779 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/copy.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/copy.html @@ -14,10 +14,17 @@ {% include "wagtailadmin/shared/field_as_li.html" with field=form.new_title %} {% include "wagtailadmin/shared/field_as_li.html" with field=form.new_slug %} - {% if subpage_count %} + {% if pages_to_copy.count > 1 %} {% include "wagtailadmin/shared/field_as_li.html" with field=form.copy_subpages %}
  • - {% blocktrans %}This will copy {{ subpage_count }} subpages.{% endblocktrans %} + {% blocktrans with copy_page_count=pages_to_copy.count|add:'-1' %}This will copy {{ copy_page_count }} subpages.{% endblocktrans %} +
    • + {% endif %} + + {% if can_publish %} + {% include "wagtailadmin/shared/field_as_li.html" with field=form.publish_copies %} +
  • + {% blocktrans with live_page_count=pages_to_copy.live.count %}{{ live_page_count }} of the pages being copied are live. Would you like to publish their copies?{% endblocktrans %}
    • {% endif %} diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index e9a3a32a1..651ef703a 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -2,7 +2,7 @@ from datetime import timedelta from django.test import TestCase from django.core.urlresolvers import reverse -from django.contrib.auth.models import User, Permission +from django.contrib.auth.models import User, Group, Permission from django.core import mail from django.core.paginator import Paginator from django.utils import timezone @@ -796,19 +796,41 @@ class TestPageCopy(TestCase, WagtailTestUtils): self.root_page = Page.objects.get(id=2) # Create a page - self.test_page = SimplePage() - self.test_page.title = "Hello world!" - self.test_page.slug = "hello-world" - self.root_page.add_child(instance=self.test_page) + self.test_page = self.root_page.add_child(instance=SimplePage( + title="Hello world!", + slug='hello-world', + live=True, + )) + + # Create a couple of child pages + self.test_child_page = self.test_page.add_child(instance=SimplePage( + title="Child page", + slug='child-page', + live=True, + )) + + self.test_unpublished_child_page = self.test_page.add_child(instance=SimplePage( + title="Unpublished Child page", + slug='unpublished-child-page', + live=False, + )) # Login self.user = self.login() def test_page_copy(self): response = self.client.get(reverse('wagtailadmin_pages_copy', args=(self.test_page.id, ))) + + # Check response self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailadmin/pages/copy.html') + # Make sure all fields are in the form + self.assertContains(response, "New title") + self.assertContains(response, "New slug") + self.assertContains(response, "Copy subpages") + self.assertContains(response, "Publish copies") + def test_page_copy_bad_permissions(self): # Remove privileges from user self.user.is_superuser = False @@ -828,17 +850,101 @@ class TestPageCopy(TestCase, WagtailTestUtils): 'new_title': "Hello world 2", 'new_slug': 'hello-world-2', 'copy_subpages': False, + 'publish_copies': False, } response = self.client.post(reverse('wagtailadmin_pages_copy', args=(self.test_page.id, )), post_data) # Check that the user was redirected to the parents explore page self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) + # Get copy + page_copy = self.root_page.get_children().filter(slug='hello-world-2').first() + # Check that the copy exists - self.assertTrue(self.root_page.get_children().filter(slug='hello-world-2').exists()) + self.assertNotEqual(page_copy, None) + + # Check that the copy is not live + self.assertFalse(page_copy.live) # Check that the owner of the page is set correctly - self.assertEqual(self.root_page.get_children().filter(slug='hello-world-2').first().owner, self.user) + self.assertEqual(page_copy.owner, self.user) + + # Check that the children were not copied + self.assertEqual(page_copy.get_children().count(), 0) + + def test_page_copy_post_copy_subpages(self): + post_data = { + 'new_title': "Hello world 2", + 'new_slug': 'hello-world-2', + 'copy_subpages': True, + 'publish_copies': False, + } + response = self.client.post(reverse('wagtailadmin_pages_copy', args=(self.test_page.id, )), post_data) + + # Check that the user was redirected to the parents explore page + self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) + + # Get copy + page_copy = self.root_page.get_children().filter(slug='hello-world-2').first() + + # Check that the copy exists + self.assertNotEqual(page_copy, None) + + # Check that the copy is not live + self.assertFalse(page_copy.live) + + # Check that the owner of the page is set correctly + self.assertEqual(page_copy.owner, self.user) + + # Check that the children were copied + self.assertEqual(page_copy.get_children().count(), 2) + + # Check the the child pages + # Neither of them should be live + child_copy = page_copy.get_children().filter(slug='child-page').first() + self.assertNotEqual(child_copy, None) + self.assertFalse(child_copy.live) + + unpublished_child_copy = page_copy.get_children().filter(slug='unpublished-child-page').first() + self.assertNotEqual(unpublished_child_copy, None) + self.assertFalse(unpublished_child_copy.live) + + def test_page_copy_post_copy_subpages_publish_copies(self): + post_data = { + 'new_title': "Hello world 2", + 'new_slug': 'hello-world-2', + 'copy_subpages': True, + 'publish_copies': True, + } + response = self.client.post(reverse('wagtailadmin_pages_copy', args=(self.test_page.id, )), post_data) + + # Check that the user was redirected to the parents explore page + self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) + + # Get copy + page_copy = self.root_page.get_children().filter(slug='hello-world-2').first() + + # Check that the copy exists + self.assertNotEqual(page_copy, None) + + # Check that the copy is live + self.assertTrue(page_copy.live) + + # Check that the owner of the page is set correctly + self.assertEqual(page_copy.owner, self.user) + + # Check that the children were copied + self.assertEqual(page_copy.get_children().count(), 2) + + # Check the the child pages + # The child_copy should be live but the unpublished_child_copy shouldn't + child_copy = page_copy.get_children().filter(slug='child-page').first() + self.assertNotEqual(child_copy, None) + self.assertTrue(child_copy.live) + + unpublished_child_copy = page_copy.get_children().filter(slug='unpublished-child-page').first() + self.assertNotEqual(unpublished_child_copy, None) + self.assertFalse(unpublished_child_copy.live) def test_page_copy_post_existing_slug(self): # This tests the existing slug checking on page copy @@ -857,6 +963,71 @@ class TestPageCopy(TestCase, WagtailTestUtils): # Check that a form error was raised self.assertFormError(response, 'form', 'new_slug', "This slug is already in use") + def test_page_copy_no_publish_permission(self): + # Turn user into an editor who can add pages but not publish them + self.user.is_superuser = False + self.user.groups.add( + Group.objects.get(name="Editors"), + ) + self.user.save() + + # Get copy page + response = self.client.get(reverse('wagtailadmin_pages_copy', args=(self.test_page.id, ))) + + # The user should have access to the copy page + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/pages/copy.html') + + # Make sure the "publish copies" field is hidden + self.assertNotContains(response, "Publish copies") + + def test_page_copy_no_publish_permission_post_copy_subpages_publish_copies(self): + # This tests that unprivileged users cannot publish copied pages even if they hack their browser + + # Turn user into an editor who can add pages but not publish them + self.user.is_superuser = False + self.user.groups.add( + Group.objects.get(name="Editors"), + ) + self.user.save() + + # Post + post_data = { + 'new_title': "Hello world 2", + 'new_slug': 'hello-world-2', + 'copy_subpages': True, + 'publish_copies': True, + } + response = self.client.post(reverse('wagtailadmin_pages_copy', args=(self.test_page.id, )), post_data) + + # Check that the user was redirected to the parents explore page + self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) + + # Get copy + page_copy = self.root_page.get_children().filter(slug='hello-world-2').first() + + # Check that the copy exists + self.assertNotEqual(page_copy, None) + + # Check that the copy is not live + self.assertFalse(page_copy.live) + + # Check that the owner of the page is set correctly + self.assertEqual(page_copy.owner, self.user) + + # Check that the children were copied + self.assertEqual(page_copy.get_children().count(), 2) + + # Check the the child pages + # Neither of them should be live + child_copy = page_copy.get_children().filter(slug='child-page').first() + self.assertNotEqual(child_copy, None) + self.assertFalse(child_copy.live) + + unpublished_child_copy = page_copy.get_children().filter(slug='unpublished-child-page').first() + self.assertNotEqual(unpublished_child_copy, None) + self.assertFalse(unpublished_child_copy.live) + class TestPageUnpublish(TestCase, WagtailTestUtils): def setUp(self): diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index d0dd21e2b..01110ccb0 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -635,18 +635,21 @@ def set_page_position(request, page_to_move_id): @permission_required('wagtailadmin.access_admin') def copy(request, page_id): page = Page.objects.get(id=page_id) - subpage_count = page.get_descendants().count() parent_page = page.get_parent() # Make sure this user has permission to add subpages on the parent if not parent_page.permissions_for_user(request.user).can_add_subpage(): raise PermissionDenied + # Check if the user has permission to publish subpages on the parent + can_publish = parent_page.permissions_for_user(request.user).can_publish_subpage() + # Create the form form = CopyForm(request.POST or None, initial={ 'new_title': page.title, 'new_slug': page.slug, 'copy_subpages': True, + 'publish_copies': True, }) # Stick an extra validator into the form to make sure that the slug is not already in use @@ -668,12 +671,19 @@ def copy(request, page_id): } ) - # Assign usef of this request as the owner of all the new pages + # Check if we should keep copied subpages published + publish_copies = can_publish and form.cleaned_data['publish_copies'] + + # Unpublish copied pages if we need to + if not publish_copies: + new_page.get_descendants(inclusive=True).update(live=False) + + # Assign user of this request as the owner of all the new pages new_page.get_descendants(inclusive=True).update(owner=request.user) # Give a success message back to the user if form.cleaned_data['copy_subpages']: - messages.success(request, _("Page '{0}' and {1} subpages copied.").format(page.title, subpage_count)) + messages.success(request, _("Page '{0}' and {1} subpages copied.").format(page.title, new_page.get_descendants().count())) else: messages.success(request, _("Page '{0}' copied.").format(page.title)) @@ -682,8 +692,9 @@ def copy(request, page_id): return render(request, 'wagtailadmin/pages/copy.html', { 'page': page, - 'subpage_count': subpage_count, + 'pages_to_copy': page.get_descendants(inclusive=True), 'parent_page': parent_page, + 'can_publish': can_publish, 'form': form, })