diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 4ce169509d..b9fbf28ac9 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -8,6 +8,7 @@ Changelog * Improvements to the layout of the admin menu footer. * Added thousands separator for counters on dashboard * Added contextual links to admin notification messages + * When copying pages, it is now possible to specify a place to copy to (Timo Rieber) * Fix: It is no longer possible to have the explorer and settings menu open at the same time diff --git a/CONTRIBUTORS.rst b/CONTRIBUTORS.rst index 19211d3a07..c41c77bb32 100644 --- a/CONTRIBUTORS.rst +++ b/CONTRIBUTORS.rst @@ -36,6 +36,7 @@ Contributors * Eric Drechsel * Alejandro Giacometti * linibou +* Timo Rieber Translators diff --git a/docs/releases/0.9.rst b/docs/releases/0.9.rst index 91fd626f98..d36fdce57a 100644 --- a/docs/releases/0.9.rst +++ b/docs/releases/0.9.rst @@ -17,6 +17,7 @@ Minor features * Improvements to the layout of the admin menu footer. * Added thousands separator for counters on dashboard * Added contextual links to admin notification messages + * When copying pages, it is now possible to specify a place to copy to Bug fixes diff --git a/wagtail/wagtailadmin/forms.py b/wagtail/wagtailadmin/forms.py index bcbd39918c..f62d7ae319 100644 --- a/wagtail/wagtailadmin/forms.py +++ b/wagtail/wagtailadmin/forms.py @@ -3,6 +3,8 @@ from django.contrib.auth import get_user_model from django.contrib.auth.forms import AuthenticationForm, PasswordResetForm from django.utils.translation import ugettext as _ from django.utils.translation import ungettext, ugettext_lazy +from wagtail.wagtailadmin.widgets import AdminPageChooser +from wagtail.wagtailcore.models import Page class SearchForm(forms.Form): @@ -94,6 +96,13 @@ class CopyForm(forms.Form): self.fields['new_title'] = forms.CharField(initial=self.page.title, label=_("New title")) self.fields['new_slug'] = forms.SlugField(initial=self.page.slug, label=_("New slug")) + self.fields['new_parent_page'] = forms.ModelChoiceField( + initial=self.page.get_parent(), + queryset=Page.objects.all(), + widget=AdminPageChooser(), + label=_("New parent page"), + help_text=_("This copy will be a child of this given parent page.") + ) pages_to_copy = self.page.get_descendants(inclusive=True) subpage_count = pages_to_copy.count() - 1 @@ -123,13 +132,24 @@ class CopyForm(forms.Form): required=False, initial=True, label=label, help_text=help_text ) - def clean_new_slug(self): - # Make sure the slug isn't already in use - slug = self.cleaned_data['new_slug'] + def clean(self): + cleaned_data = super(CopyForm, self).clean() - if self.page.get_siblings(inclusive=True).filter(slug=slug).count(): - raise forms.ValidationError(_("This slug is already in use")) - return slug + # Make sure the slug isn't already in use + slug = cleaned_data.get('new_slug') + + # New parent page given in form or parent of source, if parent_page is empty + parent_page = cleaned_data.get('new_parent_page') or self.page.get_parent() + + # Count the pages with the same slug within the context of our copy's parent page + if slug and parent_page.get_children().filter(slug=slug).count(): + self._errors['new_slug'] = self.error_class( + [_("This slug is already in use within the context of its parent page \"%s\"" % parent_page)] + ) + # The slug is no longer valid, hence remove it from cleaned_data + del cleaned_data['new_slug'] + + return cleaned_data class PageViewRestrictionForm(forms.Form): diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/copy.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/copy.html index 1a645c16f2..ef155ef9a8 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/copy.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/copy.html @@ -14,6 +14,12 @@ {% include "wagtailadmin/shared/field_as_li.html" with field=form.new_title %} {% include "wagtailadmin/shared/field_as_li.html" with field=form.new_slug %} +
  • + {% trans "Change page" as choose_another_text_str %} + {% trans "Choose page" as choose_one_text_str %} + {% include "wagtailadmin/edit_handlers/page_chooser_panel.html" with field=form.new_parent_page page=parent_page is_chosen=True choose_one_text_str=choose_one_text_str choose_another_text_str=choose_another_text_str only %} +
  • + {% if form.copy_subpages %} {% include "wagtailadmin/shared/field_as_li.html" with field=form.copy_subpages %} {% endif %} @@ -27,3 +33,7 @@ {% endblock %} + +{% block extra_js %} + {% include "wagtailadmin/pages/_editor_js.html" %} +{% endblock %} diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index ea4834c18e..ccfa67187a 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -1032,6 +1032,7 @@ class TestPageCopy(TestCase, WagtailTestUtils): # Make sure all fields are in the form self.assertContains(response, "New title") self.assertContains(response, "New slug") + self.assertContains(response, "New parent page") self.assertContains(response, "Copy subpages") self.assertContains(response, "Publish copies") @@ -1044,15 +1045,22 @@ class TestPageCopy(TestCase, WagtailTestUtils): self.user.save() # Get copy page - response = self.client.get(reverse('wagtailadmin_pages_copy', args=(self.test_page.id, ))) + post_data = { + 'new_title': "Hello world 2", + 'new_slug': 'hello-world', + 'new_parent_page': str(self.test_page.id), + 'copy_subpages': False, + } + response = self.client.post(reverse('wagtailadmin_pages_copy', args=(self.test_page.id, )), post_data) - # Check that the user recieved a 403 response + # Check that the user received a 403 response self.assertEqual(response.status_code, 403) def test_page_copy_post(self): post_data = { 'new_title': "Hello world 2", 'new_slug': 'hello-world-2', + 'new_parent_page': str(self.root_page.id), 'copy_subpages': False, 'publish_copies': False, } @@ -1081,6 +1089,7 @@ class TestPageCopy(TestCase, WagtailTestUtils): post_data = { 'new_title': "Hello world 2", 'new_slug': 'hello-world-2', + 'new_parent_page': str(self.root_page.id), 'copy_subpages': True, 'publish_copies': False, } @@ -1121,6 +1130,7 @@ class TestPageCopy(TestCase, WagtailTestUtils): post_data = { 'new_title': "Hello world 2", 'new_slug': 'hello-world-2', + 'new_parent_page': str(self.root_page.id), 'copy_subpages': True, 'publish_copies': True, } @@ -1157,13 +1167,30 @@ class TestPageCopy(TestCase, WagtailTestUtils): self.assertFalse(unpublished_child_copy.live) self.assertTrue(unpublished_child_copy.has_unpublished_changes) - def test_page_copy_post_existing_slug(self): - # This tests the existing slug checking on page copy + def test_page_copy_post_new_parent(self): + post_data = { + 'new_title': "Hello world 2", + 'new_slug': 'hello-world-2', + 'new_parent_page': str(self.test_child_page.id), + '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 new parents explore page + self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.test_child_page.id, ))) + + # Check that the page was copied to the correct place + self.assertTrue(Page.objects.filter(slug='hello-world-2').first().get_parent(), self.test_child_page) + + def test_page_copy_post_existing_slug_within_same_parent_page(self): + # This tests the existing slug checking on page copy when not changing the parent page # Attempt to copy the page but forget to change the slug post_data = { 'new_title': "Hello world 2", 'new_slug': 'hello-world', + 'new_parent_page': str(self.root_page.id), 'copy_subpages': False, } response = self.client.post(reverse('wagtailadmin_pages_copy', args=(self.test_page.id, )), post_data) @@ -1172,13 +1199,29 @@ class TestPageCopy(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) # Check that a form error was raised - self.assertFormError(response, 'form', 'new_slug', "This slug is already in use") + self.assertFormError(response, 'form', 'new_slug', "This slug is already in use within the context of its parent page \"Welcome to your new Wagtail site!\"") + + 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 + + # 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': 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.test_child_page.id, ))) def test_page_copy_post_invalid_slug(self): # Attempt to copy the page but set an invalid slug string post_data = { 'new_title': "Hello world 2", 'new_slug': 'hello world!', + 'new_parent_page': str(self.root_page.id), 'copy_subpages': False, } response = self.client.post(reverse('wagtailadmin_pages_copy', args=(self.test_page.id, )), post_data) @@ -1221,6 +1264,7 @@ class TestPageCopy(TestCase, WagtailTestUtils): post_data = { 'new_title': "Hello world 2", 'new_slug': 'hello-world-2', + 'new_parent_page': str(self.root_page.id), 'copy_subpages': True, 'publish_copies': True, } diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index 34d757986b..a30548b15a 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -628,11 +628,9 @@ 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) - 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 + # Parent page defaults to parent of source page + parent_page = page.get_parent() # Check if the user has permission to publish subpages on the parent can_publish = parent_page.permissions_for_user(request.user).can_publish_subpage() @@ -641,31 +639,49 @@ def copy(request, page_id): form = CopyForm(request.POST or None, page=page, can_publish=can_publish) # Check if user is submitting - if request.method == 'POST' and form.is_valid(): - # Copy the page - new_page = page.copy( - recursive=form.cleaned_data.get('copy_subpages'), - update_attrs={ - 'title': form.cleaned_data['new_title'], - 'slug': form.cleaned_data['new_slug'], - }, - keep_live=(can_publish and form.cleaned_data.get('publish_copies')), - ) + if request.method == 'POST': + # Prefill parent_page in case the form is invalid (as prepopulated value for the form field, + # because ModelChoiceField seems to not fall back to the user given value) + parent_page = Page.objects.get(id=request.POST['new_parent_page']) - # Assign user of this request as the owner of all the new pages - new_page.get_descendants(inclusive=True).update(owner=request.user) + if form.is_valid(): + # Receive the parent page (this should never be empty) + if form.cleaned_data['new_parent_page']: + parent_page = form.cleaned_data['new_parent_page'] - # Give a success message back to the user - if form.cleaned_data.get('copy_subpages'): - 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)) + # 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 - # Redirect to explore of parent page - return redirect('wagtailadmin_explore', parent_page.id) + # Re-check if the user has permission to publish subpages on the new parent + can_publish = parent_page.permissions_for_user(request.user).can_publish_subpage() + + # Copy the page + new_page = page.copy( + recursive=form.cleaned_data.get('copy_subpages'), + to=parent_page, + update_attrs={ + 'title': form.cleaned_data['new_title'], + 'slug': form.cleaned_data['new_slug'], + }, + keep_live=(can_publish and form.cleaned_data.get('publish_copies')), + ) + + # 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.get('copy_subpages'): + 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)) + + # Redirect to explore of parent page + return redirect('wagtailadmin_explore', parent_page.id) return render(request, 'wagtailadmin/pages/copy.html', { 'page': page, + 'parent_page': parent_page, 'form': form, })