change the admin page move interface to a page chooser panel

- fixes #8647

CHANGELOG.txt

@@ -40,6 +40,7 @@ Changelog
  * Implement Fuzzy matching for Elasticsearch (Nick Smith)
  * Rename `Page.get_latest_revision_as_page` to `Page.get_latest_revision_as_object` (Sage Abdullah)
  * Cache model permission codenames in PermissionHelper (Tidiane Dia)
+ * Selecting a new parent page for moving a page now uses the chooser modal which allows searching (Viggo de Vries)
  * Fix: Typo in `ResumeWorkflowActionFormatter` message (Stefan Hammer)
  * Fix: Throw a meaningful error when saving an image to an unrecognised image format (Christian Franke)
  * Fix: Remove extra padding for headers with breadcrumbs on mobile viewport (Steven Steinwand)

CONTRIBUTORS.rst

@@ -606,6 +606,7 @@ Contributors
 * Hugh Rawlinson
 * Noble Mittal
 * Oliver Parker
+* Viggo de Vries
 
 Translators
 ===========

docs/releases/4.0.md

@@ -48,6 +48,7 @@ When using a queryset to render a list of images, you can now use the ``prefetch
  * Add `add_to_admin_menu` option for ModelAdmin (Oliver Parker)
  * Implement [Fuzzy matching](fuzzy_matching) for Elasticsearch (Nick Smith)
  * Cache model permission codenames in `PermissionHelper` (Tidiane Dia)
+ * Selecting a new parent page for moving a page now uses the chooser modal which allows searching (Viggo de Vries)
 
 ### Bug fixes
 

wagtail/admin/forms/pages.py

@@ -212,3 +212,24 @@ class WagtailAdminPageForm(WagtailAdminModelForm):
             del cleaned_data["first_published_at"]
 
         return cleaned_data
+
+
+class MoveForm(forms.Form):
+    def __init__(self, *args, **kwargs):
+        self.page_to_move = kwargs.pop("page_to_move")
+        self.target_parent_models = kwargs.pop("target_parent_models")
+
+        super().__init__(*args, **kwargs)
+
+        self.fields["new_parent_page"] = forms.ModelChoiceField(
+            initial=self.page_to_move.get_parent(),
+            queryset=Page.objects.all(),
+            widget=widgets.AdminPageMoveChooser(
+                can_choose_root=True,
+                user_perms="move_to",
+                target_models=self.target_parent_models,
+                pages_to_move=[self.page_to_move.pk],
+            ),
+            label=_("New parent page"),
+            help_text=_("Select a new parent for this page."),
+        )

wagtail/admin/templates/wagtailadmin/pages/move_choose_destination.html

@@ -1,19 +1,30 @@
 {% extends "wagtailadmin/base.html" %}
-{% load i18n wagtailadmin_tags %}
-{% block titletag %}{% blocktrans trimmed with title=page_to_move.specific_deferred.get_admin_display_title %}Select a new parent page for {{ title }}{% endblocktrans %}{% endblock %}
+{% load i18n %}
+{% block titletag %}{% blocktrans trimmed with title=page_to_move.specific_deferred.get_admin_display_title %}Move {{ title }}{% endblocktrans %}{% endblock %}
 {% block content %}
-    <header class="header">
-        {% move_breadcrumb page_to_move viewed_page %}
-        <div class="row">
-            <h1 class="header__title">
-                {% icon name="doc-empty-inverse" %}
-                {% blocktrans trimmed with title=page_to_move.specific_deferred.get_admin_display_title %}Select a new parent page for <span>{{ title }}</span>{% endblocktrans %}
-            </h1>
-        </div>
-    </header>
+    {% trans "Move" as header_title %}
+    {% include "wagtailadmin/shared/header.html" with title=header_title subtitle=page_to_move.specific_deferred.get_admin_display_title icon="doc-empty-inverse" %}
+
     <div class="nice-padding">
-        {% include "wagtailadmin/pages/listing/_list_move.html" with pages=child_pages parent_page=viewed_page %}
-        {% url 'wagtailadmin_pages:move_choose_destination' page_to_move.id viewed_page.id as pagination_base_url %}
-        {% paginate child_pages base_url=pagination_base_url %}
+        <form action="{% url 'wagtailadmin_pages:move' page_to_move.id %}" method="POST" novalidate>
+            {% csrf_token %}
+
+            <ul class="fields">
+                {% include "wagtailadmin/shared/field_as_li.html" with field=move_form.new_parent_page %}
+            </ul>
+
+            <input type="submit" value="{% trans 'Confirm' %}" class="button">
+        </form>
     </div>
 {% endblock %}
+
+{% block extra_js %}
+    {{ block.super }}
+    {% include "wagtailadmin/pages/_editor_js.html" %}
+    {{ move_form.media.js }}
+{% endblock %}
+
+{% block extra_css %}
+    {{ block.super }}
+    {{ move_form.media.css }}
+{% endblock %}

wagtail/admin/tests/pages/test_move_page.py

@@ -7,7 +7,6 @@ from django.http import HttpRequest, HttpResponse
 from django.test import TestCase
 from django.urls import reverse
 
-from wagtail.admin.navigation import get_explorable_root_page
 from wagtail.models import Page
 from wagtail.signals import post_page_move, pre_page_move
 from wagtail.test.testapp.models import SimplePage
@@ -70,25 +69,6 @@ class TestPageMove(TestCase, WagtailTestUtils):
         )
         self.assertEqual(response.status_code, 200)
 
-    def test_page_move_default_destination(self):
-        response = self.client.get(
-            reverse("wagtailadmin_pages:move", args=(self.test_page_b.id,))
-        )
-        self.assertEqual(response.status_code, 200)
-        # The default destination is the parent of the page being moved
-        self.assertEqual(response.context["viewed_page"].specific, self.section_c)
-
-        cca = get_explorable_root_page(self.user)
-        destinations = self.section_c.get_ancestors().descendant_of(cca)
-        self.assertTrue(destinations.exists())
-
-        for destination_page in destinations:
-            move_url = reverse(
-                "wagtailadmin_pages:move_choose_destination",
-                args=(self.test_page_b.id, destination_page.id),
-            )
-            self.assertContains(response, move_url)
-
     def test_page_move_bad_permissions(self):
         # Remove privileges from user
         self.user.is_superuser = False

wagtail/admin/tests/test_page_chooser.py

@@ -1021,6 +1021,85 @@ class TestCanChoosePage(TestCase, WagtailTestUtils):
         )
         self.assertFalse(result)
 
+    def test_move_to_same_page(self):
+        homepage = Page.objects.get(url_path="/home/")
+        result = can_choose_page(
+            homepage,
+            self.permission_proxy,
+            self.desired_classes,
+            user_perm="move_to",
+            target_pages=[homepage],
+        )
+        self.assertFalse(result)
+
+    def test_move_to_root(self):
+        homepage = Page.objects.get(url_path="/home/")
+        root = Page.objects.get(url_path="/")
+        result = can_choose_page(
+            root,
+            self.permission_proxy,
+            self.desired_classes,
+            user_perm="move_to",
+            target_pages=[homepage],
+        )
+        self.assertTrue(result)
+
+    def test_move_to_page_with_wrong_parent_types(self):
+        board_meetings = Page.objects.get(
+            url_path="/home/events/businessy-events/board-meetings/"
+        )
+        homepage = Page.objects.get(url_path="/home/")
+        result = can_choose_page(
+            homepage,
+            self.permission_proxy,
+            self.desired_classes,
+            user_perm="move_to",
+            target_pages=[board_meetings],
+        )
+        self.assertFalse(result)
+
+    def test_move_to_same_page_bulk(self):
+        homepage = Page.objects.get(url_path="/home/")
+        secret_plans = Page.objects.get(url_path="/home/secret-plans/")
+        result = can_choose_page(
+            homepage,
+            self.permission_proxy,
+            self.desired_classes,
+            user_perm="bulk_move_to",
+            target_pages=[homepage, secret_plans],
+        )
+        self.assertFalse(result)
+
+    def test_move_to_root_bulk(self):
+        homepage = Page.objects.get(url_path="/home/")
+        secret_plans = Page.objects.get(url_path="/home/secret-plans/")
+        root = Page.objects.get(url_path="/")
+        result = can_choose_page(
+            root,
+            self.permission_proxy,
+            self.desired_classes,
+            user_perm="bulk_move_to",
+            target_pages=[homepage, secret_plans],
+        )
+        self.assertTrue(result)
+
+    def test_move_to_page_with_wrong_parent_types_bulk(self):
+        board_meetings = Page.objects.get(
+            url_path="/home/events/businessy-events/board-meetings/"
+        )
+        steal_underpants = Page.objects.get(
+            url_path="/home/secret-plans/steal-underpants/"
+        )
+        homepage = Page.objects.get(url_path="/home/")
+        result = can_choose_page(
+            homepage,
+            self.permission_proxy,
+            self.desired_classes,
+            user_perm="bulk_move_to",
+            target_pages=[board_meetings, steal_underpants],
+        )
+        self.assertTrue(result)
+
 
 @override_settings(WAGTAIL_I18N_ENABLED=True)
 class TestPageChooserLocaleSelector(TestCase, WagtailTestUtils):

wagtail/admin/urls/pages.py

@@ -54,11 +54,6 @@ urlpatterns = [
     ),
     path("search/", search.search, name="search"),
     path("<int:page_to_move_id>/move/", move.move_choose_destination, name="move"),
-    path(
-        "<int:page_to_move_id>/move/<int:viewed_page_id>/",
-        move.move_choose_destination,
-        name="move_choose_destination",
-    ),
     path(
         "<int:page_to_move_id>/move/<int:destination_id>/confirm/",
         move.move_confirm,

wagtail/admin/views/chooser.py

@@ -82,12 +82,15 @@ def can_choose_page(
     elif not can_choose_root and page.is_root():
         return False
 
-    if user_perm == "move_to":
+    if user_perm in ["move_to", "bulk_move_to"]:
         pages_to_move = target_pages
 
         for page_to_move in pages_to_move:
-            if page == page_to_move or page.is_descendant_of(page_to_move):
+            if page.pk == page_to_move.pk or page.is_descendant_of(page_to_move):
                 return False
+
+            if user_perm == "move_to":
+                return permission_proxy.for_page(page_to_move).can_move_to(page)
     if user_perm == "copy_to":
         return permission_proxy.for_page(page).can_add_subpage()
 
@@ -199,7 +202,7 @@ def browse(request, parent_page_id=None):
 
     can_choose_root = request.GET.get("can_choose_root", False)
     target_pages = Page.objects.filter(
-        pk__in=[int(pk) for pk in request.GET.get("target_pages", "").split(",") if pk]
+        pk__in=[int(pk) for pk in request.GET.getlist("target_pages[]", []) if pk]
     )
 
     match_subclass = request.GET.get("match_subclass", True)

wagtail/admin/views/pages/bulk_actions/move.py

@@ -8,29 +8,6 @@ from wagtail.admin.views.pages.bulk_actions.page_bulk_action import PageBulkActi
 from wagtail.models import Page
 
 
-class BulkMovePageChooser(widgets.AdminPageChooser):
-    def __init__(
-        self, target_models=None, can_choose_root=False, user_perms=None, **kwargs
-    ):
-        self.pages_to_move = kwargs.pop("pages_to_move", [])
-        super().__init__(
-            target_models=target_models,
-            can_choose_root=can_choose_root,
-            user_perms=user_perms,
-            **kwargs,
-        )
-
-    @widgets.AdminPageChooser.client_options.getter
-    def client_options(self):
-        return {
-            "model_names": self.model_names,
-            "can_choose_root": self.can_choose_root,
-            "user_perms": self.user_perms,
-            "target_pages": self.pages_to_move,
-            "match_subclass": False,
-        }
-
-
 class MoveForm(forms.Form):
     def __init__(self, *args, **kwargs):
         destination = kwargs.pop("destination")
@@ -40,9 +17,9 @@ class MoveForm(forms.Form):
         self.fields["chooser"] = forms.ModelChoiceField(
             initial=destination,
             queryset=Page.objects.all(),
-            widget=BulkMovePageChooser(
+            widget=widgets.AdminPageMoveChooser(
                 can_choose_root=True,
-                user_perms="move_to",
+                user_perms="bulk_move_to",
                 target_models=target_parent_models,
                 pages_to_move=pages_to_move,
             ),

wagtail/admin/views/pages/move.py

@@ -1,6 +1,5 @@
 from django.conf import settings
 from django.core.exceptions import PermissionDenied
-from django.core.paginator import Paginator
 from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
 from django.urls import reverse
@@ -9,45 +8,41 @@ from django.utils.translation import gettext as _
 from wagtail import hooks
 from wagtail.actions.move_page import MovePageAction
 from wagtail.admin import messages
+from wagtail.admin.forms.pages import MoveForm
 from wagtail.models import Page
 
 
-def move_choose_destination(request, page_to_move_id, viewed_page_id=None):
+def move_choose_destination(request, page_to_move_id):
     page_to_move = get_object_or_404(Page, id=page_to_move_id)
     page_perms = page_to_move.permissions_for_user(request.user)
     if not page_perms.can_move():
         raise PermissionDenied
 
-    if viewed_page_id:
-        viewed_page = get_object_or_404(Page, id=viewed_page_id)
-    else:
-        viewed_page = page_to_move.get_parent()
+    target_parent_models = set(page_to_move.specific_class.allowed_parent_page_models())
 
-    viewed_page.can_choose = page_perms.can_move_to(viewed_page)
+    move_form = MoveForm(
+        request.POST or None,
+        page_to_move=page_to_move,
+        target_parent_models=target_parent_models,
+    )
 
-    child_pages = []
-    for target in viewed_page.get_children():
-        # can't move the page into itself or its descendants
-        target.can_choose = page_perms.can_move_to(target)
-
-        target.can_descend = (
-            not (target == page_to_move or target.is_child_of(page_to_move))
-            and target.get_children_count()
-        )
-
-        child_pages.append(target)
-
-    # Pagination
-    paginator = Paginator(child_pages, per_page=50)
-    child_pages = paginator.get_page(request.GET.get("p"))
+    if request.method == "POST":
+        if move_form.is_valid():
+            # Receive the new parent page (this should never be empty)
+            if move_form.cleaned_data["new_parent_page"]:
+                new_parent_page = move_form.cleaned_data["new_parent_page"]
+                return redirect(
+                    "wagtailadmin_pages:move_confirm",
+                    page_to_move.id,
+                    new_parent_page.id,
+                )
 
     return TemplateResponse(
         request,
         "wagtailadmin/pages/move_choose_destination.html",
         {
             "page_to_move": page_to_move,
-            "viewed_page": viewed_page,
-            "child_pages": child_pages,
+            "move_form": move_form,
         },
     )
 
@@ -65,9 +60,8 @@ def move_confirm(request, page_to_move_id, destination_id):
             ).format(page_to_move.slug),
         )
         return redirect(
-            "wagtailadmin_pages:move_choose_destination",
+            "wagtailadmin_pages:move",
             page_to_move.id,
-            destination.id,
         )
 
     for fn in hooks.get_hooks("before_move_page"):

wagtail/admin/widgets/chooser.py

@@ -341,4 +341,27 @@ class PageChooserAdapter(WidgetAdapter):
         ]
 
 
+class AdminPageMoveChooser(AdminPageChooser):
+    def __init__(
+        self, target_models=None, can_choose_root=False, user_perms=None, **kwargs
+    ):
+        self.pages_to_move = kwargs.pop("pages_to_move", [])
+        super().__init__(
+            target_models=target_models,
+            can_choose_root=can_choose_root,
+            user_perms=user_perms,
+            **kwargs,
+        )
+
+    @property
+    def client_options(self):
+        return {
+            "model_names": self.model_names,
+            "can_choose_root": self.can_choose_root,
+            "user_perms": self.user_perms,
+            "target_pages": self.pages_to_move,
+            "match_subclass": False,
+        }
+
+
 register(PageChooserAdapter(), AdminPageChooser)