Add support for custom search handler classes to ModelAdmin's IndexView

Author:    Seb <seb@takeflight.com.au>
Date:      Sun Apr 7 12:34:00 2019 +1000

docs/reference/contrib/modeladmin/indexview.rst

@@ -280,17 +280,98 @@ for your model. For example:
 ``ModelAdmin.search_fields``
 ----------------------------
 
-**Expected value**: A list or tuple, where each item is the name of a model field
-of type ``CharField``, ``TextField``, ``RichTextField`` or ``StreamField``.
+**Expected value**: A list or tuple, where each item is the name of a model
+field of type ``CharField``, ``TextField``, ``RichTextField`` or
+``StreamField``.
 
 Set ``search_fields`` to enable a search box at the top of the index page
 for your model. You should add names of any fields on the model that should
 be searched whenever somebody submits a search query using the search box.
 
-Searching is all handled via Django's QuerySet API, rather than using Wagtail's
-search backend. This means it will work for all models, whatever search backend
+Searching is handled via Django's QuerySet API by default,
+see `ModelAdmin.search_handler_class`_ about changing this behaviour.
+This means by default it will work for all models, whatever search backend
 your project is using, and without any additional setup or configuration.
 
+
+.. _modeladmin_search_handler_class:
+
+-----------------------------------
+``ModelAdmin.search_handler_class``
+-----------------------------------
+
+**Expected value**: A subclass of
+``wagtail.contrib.modeladmin.helpers.search.BaseSearchHandler``
+
+The default value is ``DjangoORMSearchHandler``, which uses the Django ORM to
+perform lookups on the fields specified by ``search_fields``.
+
+If you would prefer to use the built-in Wagtail search backend to search your
+models, you can use the ``WagtailBackendSearchHandler`` class instead. For
+example:
+
+.. code-block:: python
+    from wagtail.contrib.modeladmin.helpers import WagtailBackendSearchHandler
+
+    from .models import Person
+
+    class PersonAdmin(ModelAdmin):
+        model = Person
+        search_handler_class = WagtailBackendSearchHandler
+
+
+Extra considerations when using ``WagtailBackendSearchHandler``
+===============================================================
+
+
+``ModelAdmin.search_fields`` is used differently
+------------------------------------------------
+
+The value of ``search_fields`` is passed to the underlying search backend to
+limit the fields used when matching. Each item in the list must be indexed
+on your model using :ref:`wagtailsearch_index_searchfield`.
+
+To allow matching on **any** indexed field, set the ``search_fields`` attribute
+on your ``ModelAdmin`` class to ``None``, or remove it completely.
+
+
+Indexing extra fields using ``index.FilterField``
+-------------------------------------------------
+
+The underlying search backend must be able to interpret all of the fields and
+relationships used in the queryset created by ``IndexView``, including those
+used in ``prefetch()`` or ``select_related()`` queryset methods, or used in
+``list_display``, ``list_filter`` or ``ordering``.
+
+Be sure to test things thoroughly in a development environment (ideally
+using the same search backend as you use in production). Wagtail will raise
+an ``IndexError`` if the backend encounters something it does not understand,
+and will tell you what you need to change.
+
+
+.. _modeladmin_extra_search_kwargs:
+
+----------------------------------
+``ModelAdmin.extra_search_kwargs``
+----------------------------------
+
+**Expected value**: A dictionary of keyword arguments that will be passed on to the ``search()`` method of
+``search_handler_class``.
+
+For example, to override the ``WagtailBackendSearchHandler`` default operator you could do the following:
+
+.. code-block:: python
+    from wagtail.contrib.modeladmin.helpers import WagtailBackendSearchHandler
+    from wagtail.search.utils import OR
+
+    from .models import IndexedModel
+
+    class DemoAdmin(ModelAdmin):
+        model = IndexedModel
+        search_handler_class = WagtailBackendSearchHandler
+        extra_search_kwargs = {'operator': OR}
+
+
 .. _modeladmin_ordering:
 
 ---------------------------
@@ -318,6 +399,7 @@ language) you can override the ``get_ordering()`` method instead.
 Set ``list_per_page`` to control how many items appear on each paginated page
 of the index view. By default, this is set to ``100``.
 
+
 .. _modeladmin_get_queryset:
 
 -----------------------------
@@ -646,4 +728,3 @@ See the following part of the docs to find out more:
 
 See the following part of the docs to find out more:
 :ref:`modeladmin_overriding_views`
-

docs/topics/search/indexing.rst

@@ -99,6 +99,8 @@ This creates an ``EventPage`` model with two fields: ``description`` and ``date`
     >>> EventPage.objects.filter(date__gt=timezone.now()).search("Christmas")
 
 
+.. _wagtailsearch_index_searchfield:
+
 ``index.SearchField``
 ---------------------
 
@@ -113,6 +115,8 @@ Options
  - **es_extra** (``dict``) - This field is to allow the developer to set or override any setting on the field in the ElasticSearch mapping. Use this if you want to make use of any ElasticSearch features that are not yet supported in Wagtail.
 
 
+.. _wagtailsearch_index_filterfield:
+
 ``index.FilterField``
 ---------------------
 

wagtail/contrib/modeladmin/helpers/__init__.py

@@ -1,3 +1,4 @@
-from .button import ButtonHelper, PageButtonHelper # NOQA
-from .permission import PagePermissionHelper, PermissionHelper # NOQA
-from .url import AdminURLHelper, PageAdminURLHelper # NOQA
+from .button import ButtonHelper, PageButtonHelper  # NOQA
+from .permission import PagePermissionHelper, PermissionHelper  # NOQA
+from .search import DjangoORMSearchHandler, WagtailBackendSearchHandler  # NOQA
+from .url import AdminURLHelper, PageAdminURLHelper  # NOQA

wagtail/contrib/modeladmin/helpers/search.py

@@ -0,0 +1,72 @@
+import operator
+from functools import reduce
+
+from django.contrib.admin.utils import lookup_needs_distinct
+from django.db.models import Q
+
+from wagtail.search.backends import get_search_backend
+
+
+class BaseSearchHandler:
+    def __init__(self, search_fields):
+        self.search_fields = search_fields
+
+    def search_queryset(self, queryset, search_term, **kwargs):
+        """
+        Returns an iterable of objects from ``queryset`` matching the
+        provided ``search_term``.
+        """
+        raise NotImplementedError()
+
+    @property
+    def show_search_form(self):
+        """
+        Returns a boolean that determines whether a search form should be
+        displayed in the IndexView UI.
+        """
+        return True
+
+
+class DjangoORMSearchHandler(BaseSearchHandler):
+    def search_queryset(self, queryset, search_term, **kwargs):
+        if not search_term or not self.search_fields:
+            return queryset
+
+        orm_lookups = ['%s__icontains' % str(search_field)
+                       for search_field in self.search_fields]
+        for bit in search_term.split():
+            or_queries = [Q(**{orm_lookup: bit})
+                          for orm_lookup in orm_lookups]
+            queryset = queryset.filter(reduce(operator.or_, or_queries))
+        opts = queryset.model._meta
+        for search_spec in orm_lookups:
+            if lookup_needs_distinct(opts, search_spec):
+                return queryset.distinct()
+        return queryset
+
+
+    @property
+    def show_search_form(self):
+        return bool(self.search_fields)
+
+
+class WagtailBackendSearchHandler(BaseSearchHandler):
+
+    default_search_backend = 'default'
+
+    def search_queryset(
+        self, queryset, search_term, preserve_order=False, operator=None,
+        partial_match=True, backend=None, **kwargs
+    ):
+        if not search_term:
+            return queryset
+
+        backend = get_search_backend(backend or self.default_search_backend)
+        return backend.search(
+            search_term,
+            queryset,
+            fields=self.search_fields or None,
+            operator=operator,
+            partial_match=partial_match,
+            order_by_relevance=not preserve_order,
+        )

wagtail/contrib/modeladmin/options.py

@@ -12,8 +12,8 @@ from wagtail.core import hooks
 from wagtail.core.models import Page
 
 from .helpers import (
-    AdminURLHelper, ButtonHelper, PageAdminURLHelper, PageButtonHelper, PagePermissionHelper,
-    PermissionHelper)
+    AdminURLHelper, ButtonHelper, DjangoORMSearchHandler, PageAdminURLHelper, PageButtonHelper,
+    PagePermissionHelper, PermissionHelper)
 from .menus import GroupMenuItem, ModelAdminMenuItem, SubMenu
 from .mixins import ThumbnailMixin  # NOQA
 from .views import ChooseParentView, CreateView, DeleteView, EditView, IndexView, InspectView
@@ -96,6 +96,8 @@ class ModelAdmin(WagtailRegisterable):
     inspect_template_name = ''
     delete_template_name = ''
     choose_parent_template_name = ''
+    search_handler_class = DjangoORMSearchHandler
+    extra_search_kwargs = {}
     permission_helper_class = None
     url_helper_class = None
     button_helper_class = None
@@ -238,6 +240,22 @@ class ModelAdmin(WagtailRegisterable):
         """
         return self.search_fields or ()
 
+    def get_search_handler(self, request, search_fields=None):
+        """
+        Returns an instance of ``self.search_handler_class`` that can be used by
+        ``IndexView``.
+        """
+        return self.search_handler_class(
+            search_fields or self.get_search_fields(request)
+        )
+
+    def get_extra_search_kwargs(self, request, search_term):
+        """
+        Returns a dictionary of additional kwargs to be sent to
+        ``SearchHandler.search_queryset()``.
+        """
+        return self.extra_search_kwargs
+
     def get_extra_attrs_for_row(self, obj, context):
         """
         Return a dictionary of HTML attributes to be added to the `<tr>`

wagtail/contrib/modeladmin/templates/modeladmin/includes/search_form.html

@@ -1,5 +1,5 @@
 {% load i18n static %}
-{% if view.search_fields %}
+{% if show_search %}
 <form id="changelist-search" class="col search-form" action="{{ view.index_url }}" method="get">
     <ul class="fields">
         <li class="required">

wagtail/contrib/modeladmin/tests/test_page_modeladmin.py

@@ -44,7 +44,7 @@ class TestIndexView(TestCase, WagtailTestUtils):
 
         self.assertEqual(response.status_code, 200)
 
-        # There are two eventpage's where the title contains 'Someone'
+        # There is one eventpage where the title contains 'Someone'
         self.assertEqual(response.context['result_count'], 1)
 
     def test_ordering(self):

wagtail/contrib/modeladmin/tests/test_search_handlers.py

@@ -0,0 +1,138 @@
+from unittest.mock import patch
+
+from django.test import TestCase
+
+from wagtail.contrib.modeladmin.helpers import DjangoORMSearchHandler, WagtailBackendSearchHandler
+from wagtail.tests.modeladmintest.models import Book
+
+
+class FakeSearchBackend:
+
+    search_last_called_with = None
+
+    def search(
+        self, query, model_or_queryset, fields=None, operator=None,
+        order_by_relevance=True, partial_match=True
+    ):
+        return {
+            'query': query,
+            'model_or_queryset': model_or_queryset,
+            'fields': fields,
+            'operator': operator,
+            'order_by_relevance': order_by_relevance,
+            'partial_match': partial_match
+        }
+
+
+class TestORMSearchHandler(TestCase):
+    fixtures = ['modeladmintest_test.json']
+
+    def get_search_handler(self, search_fields=None):
+        return DjangoORMSearchHandler(search_fields)
+
+    def get_queryset(self):
+        return Book.objects.all()
+
+    def test_search_queryset_no_search_query(self):
+        # When no search fields are specified, DjangoORMSearchHandler
+        # returns the queryset that was passed to it
+        search_handler = self.get_search_handler(search_fields=('title',))
+        queryset = self.get_queryset()
+        result = search_handler.search_queryset(queryset, '')
+        self.assertIs(result, queryset)
+
+    def test_search_queryset_no_search_fields(self):
+        # When the search query is blank, DjangoORMSearchHandler
+        # returns the queryset that was passed to it
+        search_handler = self.get_search_handler()
+        queryset = self.get_queryset()
+        result = search_handler.search_queryset(queryset, 'Lord')
+        self.assertIs(result, queryset)
+
+    def test_search_queryset(self):
+        search_handler = self.get_search_handler(search_fields=('title',))
+        queryset = self.get_queryset()
+        expected_result = queryset.filter(pk=1)
+        result = search_handler.search_queryset(queryset, 'Lord of the rings')
+        self.assertEqual(list(expected_result), list(result))
+
+    def test_show_search_form(self):
+        search_handler = self.get_search_handler(search_fields=None)
+        self.assertFalse(search_handler.show_search_form)
+
+        search_handler = self.get_search_handler(search_fields=('content',))
+        self.assertTrue(search_handler.show_search_form)
+
+
+class TestSearchBackendHandler(TestCase):
+    def get_search_handler(self, search_fields=None):
+        return WagtailBackendSearchHandler(search_fields)
+
+    def get_queryset(self):
+        return Book.objects.all()
+
+    @patch('wagtail.contrib.modeladmin.helpers.search.get_search_backend', return_value=FakeSearchBackend())
+    def test_search_queryset_no_search_query(self, mocked_method):
+        # When the search query is blank, WagtailBackendSearchHandler
+        # returns the queryset that was passed to it
+        search_handler = self.get_search_handler(search_fields=('title',))
+        queryset = self.get_queryset()
+        result = search_handler.search_queryset(queryset, '')
+        self.assertIs(result, queryset)
+
+    @patch('wagtail.contrib.modeladmin.helpers.search.get_search_backend', return_value=FakeSearchBackend())
+    def test_search_queryset_no_search_fields(self, mocked_method):
+        # When no search fields are specified, WagtailBackendSearchHandler
+        # searches on all indexed fields
+        search_handler = self.get_search_handler()
+        queryset = self.get_queryset()
+        search_kwargs = search_handler.search_queryset(queryset, 'test')
+        self.assertTrue(mocked_method.called)
+        self.assertEqual(search_kwargs, dict(
+            query='test',
+            model_or_queryset=queryset,
+            fields=None,
+            operator=None,
+            order_by_relevance=True,
+            partial_match=True,
+        ))
+
+    @patch('wagtail.contrib.modeladmin.helpers.search.get_search_backend', return_value=FakeSearchBackend())
+    def test_search_queryset_with_search_fields(self, mocked_method):
+        # When no search fields are specified, WagtailBackendSearchHandler
+        # searches on all indexed fields
+        search_fields = ('field1', 'field2')
+        search_handler = self.get_search_handler(search_fields)
+        queryset = self.get_queryset()
+        search_kwargs = search_handler.search_queryset(queryset, 'test')
+        self.assertTrue(mocked_method.called)
+        self.assertEqual(search_kwargs, dict(
+            query='test',
+            model_or_queryset=queryset,
+            fields=search_fields,
+            operator=None,
+            order_by_relevance=True,
+            partial_match=True,
+        ))
+
+    @patch('wagtail.contrib.modeladmin.helpers.search.get_search_backend', return_value=FakeSearchBackend())
+    def test_search_queryset_preserve_order(self, get_search_backend):
+        search_handler = self.get_search_handler()
+        queryset = self.get_queryset()
+
+        search_kwargs = search_handler.search_queryset(queryset, 'Lord', preserve_order=True)
+        self.assertEqual(search_kwargs, dict(
+            query='Lord',
+            model_or_queryset=queryset,
+            fields=None,
+            operator=None,
+            order_by_relevance=False,
+            partial_match=True,
+        ))
+
+    def test_show_search_form(self):
+        search_handler = self.get_search_handler(search_fields=None)
+        self.assertTrue(search_handler.show_search_form)
+
+        search_handler = self.get_search_handler(search_fields=('content',))
+        self.assertTrue(search_handler.show_search_form)

wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py

@@ -6,9 +6,11 @@ from django.core import checks
 from django.test import TestCase
 
 from wagtail.admin.edit_handlers import FieldPanel, TabbedInterface
+from wagtail.contrib.modeladmin.helpers.search import DjangoORMSearchHandler
 from wagtail.images.models import Image
 from wagtail.images.tests.utils import get_test_image_file
 from wagtail.tests.modeladmintest.models import Author, Book, Publisher, Token
+from wagtail.tests.modeladmintest.wagtail_hooks import BookModelAdmin
 from wagtail.tests.utils import WagtailTestUtils
 
 
@@ -65,7 +67,7 @@ class TestBookIndexView(TestCase, WagtailTestUtils):
         for book in response.context['object_list']:
             self.assertEqual(book.author_id, 1)
 
-    def test_search(self):
+    def test_search_indexed(self):
         response = self.get(q='of')
 
         self.assertEqual(response.status_code, 200)
@@ -73,6 +75,20 @@ class TestBookIndexView(TestCase, WagtailTestUtils):
         # There are two books where the title contains 'of'
         self.assertEqual(response.context['result_count'], 2)
 
+    def test_search_form_present(self):
+        # Test the backend search handler allows the search form to render
+        response = self.get()
+
+        self.assertContains(response, '<input id="id_q"')
+
+
+    def test_search_form_absent(self):
+        # DjangoORMSearchHandler + no search_fields, search form should be absent
+        with mock.patch.object(BookModelAdmin, 'search_handler_class', DjangoORMSearchHandler):
+            response = self.get()
+
+            self.assertNotContains(response, '<input id="id_q"')
+
     def test_ordering(self):
         response = self.get(o='0.1')
 
@@ -109,6 +125,13 @@ class TestAuthorIndexView(TestCase, WagtailTestUtils):
     def get(self, **params):
         return self.client.get('/admin/modeladmintest/author/', params)
 
+    def test_search(self):
+        response = self.get(q='Roald Dahl')
+
+        self.assertEqual(response.status_code, 200)
+
+        self.assertEqual(response.context['result_count'], 2)
+
     def test_col_extra_class_names(self):
         response = self.get()
         self.assertEqual(response.status_code, 200)

wagtail/contrib/modeladmin/views.py

@@ -1,6 +1,4 @@
-import operator
 from collections import OrderedDict
-from functools import reduce
 
 from django import forms
 from django.contrib.admin import FieldListFilter
@@ -29,7 +27,6 @@ from wagtail.admin import messages
 
 from .forms import ParentChooserForm
 
-
 try:
     from django.db.models.sql.constants import QUERY_TERMS
 except ImportError:
@@ -238,6 +235,7 @@ class IndexView(WMABaseView):
         self.search_fields = self.model_admin.get_search_fields(request)
         self.items_per_page = self.model_admin.list_per_page
         self.select_related = self.model_admin.list_select_related
+        self.search_handler = self.model_admin.get_search_handler(request, self.search_fields)
 
         # Get search parameters from the query string.
         try:
@@ -268,25 +266,9 @@ class IndexView(WMABaseView):
             obj, classnames_add=['button-small', 'button-secondary'])
 
     def get_search_results(self, request, queryset, search_term):
-        """
-        Returns a tuple containing a queryset to implement the search,
-        and a boolean indicating if the results may contain duplicates.
-        """
-        use_distinct = False
-        if self.search_fields and search_term:
-            orm_lookups = ['%s__icontains' % str(search_field)
-                           for search_field in self.search_fields]
-            for bit in search_term.split():
-                or_queries = [models.Q(**{orm_lookup: bit})
-                              for orm_lookup in orm_lookups]
-                queryset = queryset.filter(reduce(operator.or_, or_queries))
-            if not use_distinct:
-                for search_spec in orm_lookups:
-                    if lookup_needs_distinct(self.opts, search_spec):
-                        use_distinct = True
-                        break
-
-        return queryset, use_distinct
+        kwargs = self.model_admin.get_extra_search_kwargs(request, search_term)
+        kwargs['preserve_order'] = self.ORDER_VAR in request.GET
+        return self.search_handler.search_queryset(queryset, search_term, **kwargs)
 
     def get_filters_params(self, params=None):
         """
@@ -456,10 +438,20 @@ class IndexView(WMABaseView):
         # ordering fields so we can guarantee a deterministic order across all
         # database backends.
         pk_name = self.opts.pk.name
+
+        if hasattr(self.model, 'get_filterable_search_fields'):
+            # The model is indexed, so let's be careful to only add
+            # indexed fields to ordering where possible
+            filterable_fields = self.model.get_filterable_search_fields()
+        else:
+            filterable_fields = None
+
         if not (set(ordering) & {'pk', '-pk', pk_name, '-' + pk_name}):
-            # The two sets do not intersect, meaning the pk isn't present. So
-            # we add it.
-            ordering.append('-pk')
+            # ordering isn't already being applied to pk
+            if filterable_fields is None or 'pk' in filterable_fields:
+                ordering.append('-pk')
+            else:
+                ordering.append('-' + pk_name)
 
         return ordering
 
@@ -534,15 +526,13 @@ class IndexView(WMABaseView):
         ordering = self.get_ordering(request, qs)
         qs = qs.order_by(*ordering)
 
+        # Remove duplicates from results, if necessary
+        if filters_use_distinct:
+            qs = qs.distinct()
+
         # Apply search results
-        qs, search_use_distinct = self.get_search_results(
-            request, qs, self.query)
+        return self.get_search_results(request, qs, self.query)
 
-        # Remove duplicates from results, if necessary
-        if filters_use_distinct | search_use_distinct:
-            return qs.distinct()
-        else:
-            return qs
 
     def apply_select_related(self, qs):
         if self.select_related is True:
@@ -586,7 +576,8 @@ class IndexView(WMABaseView):
             'paginator': paginator,
             'page_obj': page_obj,
             'object_list': page_obj.object_list,
-            'user_can_create': self.permission_helper.user_can_create(user)
+            'user_can_create': self.permission_helper.user_can_create(user),
+            'show_search': self.search_handler.show_search_form,
         }
 
         if self.is_pagemodel:

wagtail/tests/modeladmintest/models.py

@@ -30,6 +30,12 @@ class Book(models.Model, index.Indexed):
     title = models.CharField(max_length=255)
     cover_image = models.ForeignKey('wagtailimages.Image', on_delete=models.SET_NULL, null=True, blank=True)
 
+    search_fields = [
+        index.SearchField('title'),
+        index.FilterField('title'),
+        index.FilterField('pk'),
+    ]
+
     def __str__(self):
         return self.title
 

wagtail/tests/modeladmintest/wagtail_hooks.py

@@ -1,4 +1,5 @@
 from wagtail.admin.edit_handlers import FieldPanel, ObjectList, TabbedInterface
+from wagtail.contrib.modeladmin.helpers import WagtailBackendSearchHandler
 from wagtail.contrib.modeladmin.options import (
     ModelAdmin, ModelAdminGroup, ThumbnailMixin, modeladmin_register)
 from wagtail.contrib.modeladmin.views import CreateView
@@ -47,10 +48,10 @@ class BookModelAdmin(ThumbnailMixin, ModelAdmin):
     list_display = ('title', 'author', 'admin_thumb')
     list_filter = ('author', )
     ordering = ('title', )
-    search_fields = ('title', )
     inspect_view_enabled = True
     inspect_view_fields_exclude = ('title', )
     thumb_image_field_name = 'cover_image'
+    search_handler_class = WagtailBackendSearchHandler
 
     def get_extra_attrs_for_row(self, obj, context):
         return {