Merge branch 'pr-838'

Conflicts: CHANGELOG.txt CONTRIBUTORS.rst docs/releases/0.9.rst
2014-12-02 14:56:49 +00:00 · 2014-12-02 14:56:49 +00:00 · 1fc4302f3a
commit 1fc4302f3a
--- 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


--- a/CONTRIBUTORS.rst
+++ b/CONTRIBUTORS.rst
@ -36,6 +36,7 @@ Contributors
 * Eric Drechsel
 * Alejandro Giacometti
 * linibou
+* Timo Rieber


 Translators
--- 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
--- 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):
--- 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 %}

+                <li class="{% if form.new_parent_page.field.required %}required{% endif %}">
+                    {% 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 %}
+                </li>
+
                {% if form.copy_subpages %}
                    {% include "wagtailadmin/shared/field_as_li.html" with field=form.copy_subpages %}
                {% endif %}
@ -27,3 +33,7 @@
        </form>
    </div>
 {% endblock %}
+
+{% block extra_js %}
+    {% include "wagtailadmin/pages/_editor_js.html" %}
+{% endblock %}
--- 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,
        }
--- 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,
    })