From e302c62ef232091209c5744c846163b113783ca8 Mon Sep 17 00:00:00 2001
From: Sage Abdullah <sage.abdullah@torchbox.com>
Date: Fri, 16 Aug 2024 10:32:36 +0100
Subject: [PATCH] Refactor page search view to be closer to universal listings
 implementation

- Use a single class with index_url_name, index_results_url_name,
  template_name, and results_template_name
- Extend generic/listing.html and generic/listing_results.html templates
- Set page_title and header_icon to show breadcrumbs properly
- Set is_searchable = True and keep using the custom search and
  filtering logic with the full get_queryset override to prevent scope
  creep for now
---
 .../templates/wagtailadmin/pages/search.html  | 10 ++--
 .../wagtailadmin/pages/search_results.html    | 50 +++++++++----------
 wagtail/admin/urls/pages.py                   |  6 ++-
 wagtail/admin/views/pages/search.py           | 29 ++++-------
 4 files changed, 42 insertions(+), 53 deletions(-)

diff --git a/wagtail/admin/templates/wagtailadmin/pages/search.html b/wagtail/admin/templates/wagtailadmin/pages/search.html
index b5ed6b6394..86ecdd40d0 100644
--- a/wagtail/admin/templates/wagtailadmin/pages/search.html
+++ b/wagtail/admin/templates/wagtailadmin/pages/search.html
@@ -1,6 +1,5 @@
-{% extends "wagtailadmin/base.html" %}
+{% extends "wagtailadmin/generic/listing.html" %}
 {% load wagtailadmin_tags i18n %}
-{% block titletag %}{% trans 'Search' %}{% endblock %}
 {% block extra_js %}
     {{ block.super }}
     <script>
@@ -10,11 +9,8 @@
 {% endblock %}
 
 {% block content %}
-    {% url 'wagtailadmin_pages:search_results' as search_results_url %}
-    {% include "wagtailadmin/shared/header.html" with title=_("Search") search_url="wagtailadmin_pages:search" icon="search" search_results_url=search_results_url search_target="#page-results" %}
-    <div id="page-results">
-        {% include "wagtailadmin/pages/search_results.html" %}
-    </div>
+    {{ block.super }}
+
     {% trans "Select all pages in listing" as select_all_text %}
     {% include 'wagtailadmin/bulk_actions/footer.html' with app_label='wagtailcore' model_name='page' objects=page_obj select_all_obj_text=select_all_text %}
 {% endblock %}
diff --git a/wagtail/admin/templates/wagtailadmin/pages/search_results.html b/wagtail/admin/templates/wagtailadmin/pages/search_results.html
index d24fa35011..c50474577b 100644
--- a/wagtail/admin/templates/wagtailadmin/pages/search_results.html
+++ b/wagtail/admin/templates/wagtailadmin/pages/search_results.html
@@ -1,16 +1,8 @@
+{% extends "wagtailadmin/generic/listing_results.html" %}
 {% load i18n wagtailadmin_tags %}
-<div class="nice-padding">
-    {% if pages %}
-        <h2 role="alert">
-            {% with page_count=all_pages.count %}
-                {% blocktrans trimmed with counter=page_count|intcomma count counter_val=page_count %}
-                    There is one matching page
-                {% plural %}
-                    There are {{ counter }} matching pages
-                {% endblocktrans %}
-            {% endwith %}
-        </h2>
 
+{% block before_results %}
+    <div class="nice-padding">
         {% search_other %}
 
         {% if object_list.supports_facet %}
@@ -20,32 +12,40 @@
                     {% if not selected_content_type %}
                         <li class="w-bg-border-furniture">{% trans "All" %} ({{ all_pages.count|intcomma }})</li>
                     {% else %}
-                        <li><a href="{% url 'wagtailadmin_pages:search' %}?q={{ query_string|urlencode }}">{% trans "All" %} ({{ all_pages.count|intcomma }})</a></li>
+                        <li><a href="{{ index_url }}?q={{ query_string|urlencode }}">{% trans "All" %} ({{ all_pages.count|intcomma }})</a></li>
                     {% endif %}
 
                     {% for content_type, count in content_types %}
                         {% if content_type == selected_content_type %}
                             <li class="w-bg-border-furniture">{{ content_type.model_class.get_verbose_name }} ({{ count|intcomma }})</li>
                         {% else %}
-                            <li><a href="{% url 'wagtailadmin_pages:search' %}?q={{ query_string|urlencode }}&amp;content_type={{ content_type.app_label }}.{{ content_type.model|lower }}">{{ content_type.model_class.get_verbose_name }} ({{ count|intcomma }})</a></li>
+                            <li><a href="{{ index_url }}?q={{ query_string|urlencode }}&amp;content_type={{ content_type.app_label }}.{{ content_type.model|lower }}">{{ content_type.model_class.get_verbose_name }} ({{ count|intcomma }})</a></li>
                         {% endif %}
                     {% endfor %}
                 </ul>
             </nav>
         {% endif %}
 
-        {% component table %}
-
-        {% if is_paginated %}
-            {% include "wagtailadmin/shared/pagination_nav.html" with items=page_obj linkurl=index_url %}
-        {% endif %}
-    {% else %}
-        {% if query_string %}
-            <h2 role="alert">{% blocktrans trimmed %}Sorry, no pages match <em>{{ query_string }}</em>{% endblocktrans %}</h2>
-
-            {% search_other %}
+        {% if object_list %}
+            <h2 role="alert">
+                {% with page_count=all_pages.count %}
+                    {% blocktrans trimmed with counter=page_count|intcomma count counter_val=page_count %}
+                        There is one matching page
+                    {% plural %}
+                        There are {{ counter }} matching pages
+                    {% endblocktrans %}
+                {% endwith %}
+            </h2>
+        {% elif query_string %}
+            <h2 role="alert">
+                {% blocktrans trimmed %}Sorry, no pages match <em>{{ query_string }}</em>{% endblocktrans %}
+            </h2>
         {% else %}
             <p>{% trans 'Enter a search term above' %}</p>
         {% endif %}
-    {% endif %}
-</div>
+    </div>
+{% endblock %}
+
+{% block no_results_message %}
+    {# This view has a custom no results message using the alert h2 like the results count message #}
+{% endblock %}
diff --git a/wagtail/admin/urls/pages.py b/wagtail/admin/urls/pages.py
index a2a98b578d..534756ffee 100644
--- a/wagtail/admin/urls/pages.py
+++ b/wagtail/admin/urls/pages.py
@@ -53,7 +53,11 @@ urlpatterns = [
         name="convert_alias",
     ),
     path("search/", search.SearchView.as_view(), name="search"),
-    path("search/results/", search.SearchResultsView.as_view(), name="search_results"),
+    path(
+        "search/results/",
+        search.SearchView.as_view(results_only=True),
+        name="search_results",
+    ),
     path("<int:page_to_move_id>/move/", move.move_choose_destination, name="move"),
     path(
         "<int:page_to_move_id>/move/<int:destination_id>/confirm/",
diff --git a/wagtail/admin/views/pages/search.py b/wagtail/admin/views/pages/search.py
index e4d39a0449..41d80f6308 100644
--- a/wagtail/admin/views/pages/search.py
+++ b/wagtail/admin/views/pages/search.py
@@ -6,7 +6,6 @@ from django.db.models.query import QuerySet
 from django.http import Http404
 from django.utils.translation import gettext_lazy as _
 
-from wagtail.admin.forms.search import SearchForm
 from wagtail.admin.ui.tables import Column, DateColumn
 from wagtail.admin.ui.tables.pages import (
     BulkActionsColumn,
@@ -49,7 +48,7 @@ def page_filter_search(q, pages, all_pages=None, ordering=None):
     return pages, all_pages
 
 
-class BaseSearchView(PermissionCheckedMixin, BaseListingView):
+class SearchView(PermissionCheckedMixin, BaseListingView):
     permission_policy = page_permission_policy
     any_permission_required = {
         "add",
@@ -60,10 +59,16 @@ class BaseSearchView(PermissionCheckedMixin, BaseListingView):
         "unlock",
     }
     paginate_by = 20
-    page_kwarg = "p"
+    page_title = _("Search")
+    header_icon = "search"
     context_object_name = "pages"
     table_class = PageTable
     index_url_name = "wagtailadmin_pages:search"
+    index_results_url_name = "wagtailadmin_pages:search_results"
+    # We override get_queryset here that has a custom search implementation
+    is_searchable = True
+    template_name = "wagtailadmin/pages/search.html"
+    results_template_name = "wagtailadmin/pages/search_results.html"
 
     columns = [
         BulkActionsColumn("bulk_actions"),
@@ -126,8 +131,6 @@ class BaseSearchView(PermissionCheckedMixin, BaseListingView):
         else:
             self.selected_content_type = None
 
-        self.q = self.request.GET.get("q", "")
-
         return super().get(request)
 
     def get_queryset(self) -> QuerySet[Any]:
@@ -145,7 +148,7 @@ class BaseSearchView(PermissionCheckedMixin, BaseListingView):
 
         # Parse query and filter
         pages, self.all_pages = page_filter_search(
-            self.q, pages, self.all_pages, self.ordering
+            self.search_query, pages, self.all_pages, self.ordering
         )
 
         # Facets
@@ -170,23 +173,9 @@ class BaseSearchView(PermissionCheckedMixin, BaseListingView):
         context.update(
             {
                 "all_pages": self.all_pages,
-                "query_string": self.q,
                 "content_types": self.content_types,
                 "selected_content_type": self.selected_content_type,
                 "ordering": self.ordering,
             }
         )
         return context
-
-
-class SearchView(BaseSearchView):
-    template_name = "wagtailadmin/pages/search.html"
-
-    def get_context_data(self, **kwargs: Any) -> dict[str, Any]:
-        context = super().get_context_data(**kwargs)
-        context["search_form"] = SearchForm(self.request.GET)
-        return context
-
-
-class SearchResultsView(BaseSearchView):
-    template_name = "wagtailadmin/pages/search_results.html"