From 000d417ec92ba8218653b4a1456e35666fc49254 Mon Sep 17 00:00:00 2001 From: Sage Abdullah Date: Fri, 16 Aug 2024 09:45:40 +0100 Subject: [PATCH] Move search implementation from IndexView to BaseListingView --- .../wagtailadmin/generic/index_results.html | 44 ++----- .../wagtailadmin/generic/listing_results.html | 35 +++++- .../tests/viewsets/test_model_viewset.py | 11 ++ wagtail/admin/views/generic/base.py | 79 ++++++++++++ wagtail/admin/views/generic/history.py | 6 +- wagtail/admin/views/generic/models.py | 113 ++++++------------ wagtail/snippets/tests/test_snippets.py | 5 +- wagtail/snippets/tests/test_viewset.py | 3 +- wagtail/users/views/users.py | 2 + 9 files changed, 178 insertions(+), 120 deletions(-) diff --git a/wagtail/admin/templates/wagtailadmin/generic/index_results.html b/wagtail/admin/templates/wagtailadmin/generic/index_results.html index c64cc67eab..893fc60cfe 100644 --- a/wagtail/admin/templates/wagtailadmin/generic/index_results.html +++ b/wagtail/admin/templates/wagtailadmin/generic/index_results.html @@ -1,47 +1,21 @@ {% extends "wagtailadmin/generic/listing_results.html" %} {% load i18n wagtailadmin_tags %} -{% block before_results %} - {{ block.super }} - +{% block other_searches %} {% if is_searching and view.show_other_searches %}
{% search_other %}
{% endif %} - - {% if not object_list %} - {# pass, to allow the same logic as `if object_list and (is_searching or is_filtering)` #} - {% elif is_searching or is_filtering %} -
-

- {% blocktrans trimmed with counter=items_count|intcomma count counter_val=items_count %} - There is {{ counter }} match - {% plural %} - There are {{ counter }} matches - {% endblocktrans %} -

-
- {% endif %} {% endblock %} {% block no_results_message %} - {% fragment as no_results_message %} - {% if is_searching or is_filtering %} - {% blocktrans trimmed with model_name=model_opts.verbose_name_plural asvar no_results_text %} - No {{ model_name }} match your query. - {% endblocktrans %} - {% elif add_url %} - {% blocktrans trimmed with model_name=model_opts.verbose_name_plural asvar no_results_text %} - There are no {{ model_name }} to display. Why not add one? - {% endblocktrans %} - {% else %} - {% blocktrans trimmed with model_name=model_opts.verbose_name_plural asvar no_results_text %} - There are no {{ model_name }} to display. - {% endblocktrans %} - {% endif %} - {{ no_results_text|capfirst }} - {% endfragment %} - -

{{ no_results_message }}

+ {% if add_url and not is_searching and not is_filtering %} + {% blocktrans trimmed with model_name=verbose_name_plural asvar no_results_text %} + There are no {{ model_name }} to display. Why not add one? + {% endblocktrans %} +

{{ no_results_text|capfirst }}

+ {% else %} + {{ block.super }} + {% endif %} {% endblock %} diff --git a/wagtail/admin/templates/wagtailadmin/generic/listing_results.html b/wagtail/admin/templates/wagtailadmin/generic/listing_results.html index b9b9fcffa8..0d41cf3982 100644 --- a/wagtail/admin/templates/wagtailadmin/generic/listing_results.html +++ b/wagtail/admin/templates/wagtailadmin/generic/listing_results.html @@ -18,6 +18,22 @@ {% endfor %} {% endif %} + + {% block other_searches %}{% endblock %} + + {% if not object_list %} + {# pass, to allow the same logic as `if object_list and (is_searching or is_filtering)` #} + {% elif is_searching or is_filtering %} +
+

+ {% blocktrans trimmed with counter=items_count|intcomma count counter_val=items_count %} + There is {{ counter }} match + {% plural %} + There are {{ counter }} matches + {% endblocktrans %} +

+
+ {% endif %} {% endblock %} {% if object_list %} @@ -33,6 +49,23 @@ {% endblock %} {% else %}
- {% block no_results_message %}

{% trans "There are no results." %}

{% endblock %} + {% fragment as no_results_message %} + {% if not verbose_name_plural %} + {% trans "There are no results." as no_results_text %} + {% elif is_searching or is_filtering %} + {% blocktrans trimmed with model_name=verbose_name_plural asvar no_results_text %} + No {{ model_name }} match your query. + {% endblocktrans %} + {% else %} + {% blocktrans trimmed with model_name=verbose_name_plural asvar no_results_text %} + There are no {{ model_name }} to display. + {% endblocktrans %} + {% endif %} + {{ no_results_text|capfirst }} + {% endfragment %} + + {% block no_results_message %} +

{{ no_results_message }}

+ {% endblock %}
{% endif %} diff --git a/wagtail/admin/tests/viewsets/test_model_viewset.py b/wagtail/admin/tests/viewsets/test_model_viewset.py index f1c8df0de1..e7e7651a5f 100644 --- a/wagtail/admin/tests/viewsets/test_model_viewset.py +++ b/wagtail/admin/tests/viewsets/test_model_viewset.py @@ -451,6 +451,7 @@ class TestSearchIndexView(WagtailTestUtils, TestCase): def test_search_disabled(self): response = self.get("fctoy_alt1", {"q": "ork"}) + self.assertFalse(response.context.get("search_form")) self.assertContains(response, "Forky") self.assertContains(response, "Buzz Lightyear") self.assertNotContains(response, "There are 2 matches") @@ -1043,6 +1044,11 @@ class TestHistoryView(WagtailTestUtils, TestCase): for rendered_row, expected_row in zip(rendered_rows, expected): self.assertSequenceEqual(rendered_row, expected_row) + # History view is not searchable + input = soup.select_one("input#id_q") + self.assertIsNone(input) + self.assertFalse(response.context.get("search_form")) + def test_action_filter(self): response = self.client.get(self.url) self.assertEqual(response.status_code, 200) @@ -1241,6 +1247,11 @@ class TestUsageView(WagtailTestUtils, TestCase): self.assertIsNotNone(link) self.assertIn(tbx_edit_url, link.attrs.get("href")) + # Usage view is not searchable + input = soup.select_one("input#id_q") + self.assertIsNone(input) + self.assertFalse(response.context.get("search_form")) + def test_usage_without_permission(self): self.user.is_superuser = False self.user.save() diff --git a/wagtail/admin/views/generic/base.py b/wagtail/admin/views/generic/base.py index 539a8040a9..73a0458cb5 100644 --- a/wagtail/admin/views/generic/base.py +++ b/wagtail/admin/views/generic/base.py @@ -1,8 +1,10 @@ +import warnings from collections import namedtuple from django.contrib.admin.utils import quote, unquote from django.core.exceptions import ImproperlyConfigured from django.db import models +from django.db.models import Q from django.shortcuts import get_object_or_404, redirect from django.urls import reverse, reverse_lazy from django.utils.formats import date_format @@ -20,9 +22,12 @@ from django_filters.filters import ( ) from wagtail.admin import messages +from wagtail.admin.forms.search import SearchForm from wagtail.admin.ui.tables import Column, Table from wagtail.admin.utils import get_valid_next_url_from_request from wagtail.admin.widgets.button import ButtonWithDropdown +from wagtail.search.backends import get_search_backend +from wagtail.search.index import class_is_indexed from wagtail.utils.utils import flatten_choices @@ -191,8 +196,13 @@ class BaseListingView(WagtailAdminTemplateMixin, BaseListView): index_url_name = None index_results_url_name = None page_kwarg = "p" + is_searchable = None # Subclasses must explicitly set this to True to enable search + search_kwarg = "q" + search_fields = None + search_backend_name = "default" default_ordering = None filterset_class = None + verbose_name_plural = None _show_breadcrumbs = True def get_template_names(self): @@ -212,6 +222,65 @@ class BaseListingView(WagtailAdminTemplateMixin, BaseListView): }, ] + def get_search_form(self): + if not self.is_searchable: + return None + + if self.search_kwarg in self.request.GET: + return SearchForm(self.request.GET) + + return SearchForm() + + @cached_property + def search_form(self): + return self.get_search_form() + + @cached_property + def search_query(self): + if self.search_form and self.search_form.is_valid(): + return self.search_form.cleaned_data[self.search_kwarg] + return "" + + @cached_property + def is_searching(self): + return bool(self.search_query) + + def search_queryset(self, queryset): + if not self.is_searching: + return queryset + + # Use Wagtail Search if the model is indexed and a search backend is defined. + # Django ORM can still be used on an indexed model by unsetting + # search_backend_name and defining search_fields on the view. + if class_is_indexed(queryset.model) and self.search_backend_name: + search_backend = get_search_backend(self.search_backend_name) + if queryset.model.get_autocomplete_search_fields(): + return search_backend.autocomplete( + self.search_query, + queryset, + fields=self.search_fields, + order_by_relevance=(not self.is_explicitly_ordered), + ) + else: + # fall back on non-autocompleting search + warnings.warn( + f"{queryset.model} is defined as Indexable but does not specify " + "any AutocompleteFields. Searches within the admin will only " + "respond to complete words.", + category=RuntimeWarning, + ) + return search_backend.search( + self.search_query, + queryset, + fields=self.search_fields, + order_by_relevance=(not self.is_explicitly_ordered), + ) + + query = Q() + for field in self.search_fields or []: + query |= Q(**{field + "__icontains": self.search_query}) + return queryset.filter(query) + @cached_property def filters(self): if self.filterset_class: @@ -415,6 +484,7 @@ class BaseListingView(WagtailAdminTemplateMixin, BaseListView): queryset = self.get_base_queryset() queryset = self.order_queryset(queryset) queryset = self.filter_queryset(queryset) + queryset = self.search_queryset(queryset) return queryset def paginate_queryset(self, queryset, page_size): @@ -466,6 +536,7 @@ class BaseListingView(WagtailAdminTemplateMixin, BaseListView): context["index_url"] = self.index_url context["index_results_url"] = self.index_results_url + context["verbose_name_plural"] = self.verbose_name_plural context["table"] = table context["media"] = table.media # On Django's BaseListView, a listing where pagination is applied, but the results @@ -484,6 +555,12 @@ class BaseListingView(WagtailAdminTemplateMixin, BaseListView): context["is_filtering"] = self.is_filtering context["media"] += self.filters.form.media + if self.search_form: + context["search_form"] = self.search_form + context["is_searching"] = self.is_searching + context["query_string"] = self.search_query + context["media"] += self.search_form.media + # If we're rendering the results as an HTML fragment, the caller can pass a _w_filter_fragment=1 # URL parameter to indicate that the filters should be rendered as a