add check for correct search_fields on pages

- fixes #4940

CHANGELOG.txt

@@ -15,6 +15,7 @@ Changelog
  * Improved styling of workflow timeline modal view (Tidjani Dia)
  * Add secondary actions menu in edit page headers (Tidjani Dia)
  * Removed WOFF fonts
+ * Add system check for missing core Page fields in `search_fields` (LB (Ben Johnston))
  * Fix: Accessibility fixes for Windows high contrast mode; Dashboard icons colour and contrast (Sakshi Uppoor)
  * Fix: Rename additional 'spin' CSS animations to avoid clashes with other libraries (Kevin Gutiérrez)
  * Fix: `default_app_config` deprecations for Django >= 3.2 (Tibor Leupold)

docs/releases/2.16.md

@@ -22,6 +22,7 @@
  * Removed WOFF fonts
  * Improved styling of workflow timeline modal view (Tidjani Dia)
  * Add secondary actions menu in edit page headers (Tidjani Dia)
+ * Add system check for missing core Page fields in `search_fields` (LB (Ben Johnston))
 
 ### Bug fixes
 

wagtail/search/apps.py

@@ -5,6 +5,8 @@ from django.utils.translation import gettext_lazy as _
 
 from wagtail.search.signal_handlers import register_signal_handlers
 
+from . import checks  # NOQA
+
 
 class WagtailSearchAppConfig(AppConfig):
     name = 'wagtail.search'

wagtail/search/checks.py

@@ -0,0 +1,26 @@
+from django.core.checks import Warning, register
+
+
+@register('search')
+def page_search_fields_check(app_configs, **kwargs):
+    """Checks each page model with search_fields to core fields are included"""
+    from wagtail.core.models import Page, get_page_models
+
+    page_models = get_page_models()
+    errors = []
+
+    for cls in page_models:
+        # Only checks an initial subset of fields as only need to check some are missing to show the warning
+        if not all(field in cls.search_fields for field in Page.search_fields[:10]):
+            errors.append(
+                Warning(
+                    'Core Page fields missing in `search_fields`',
+                    hint=' '.join([
+                        'Ensure that {} extends the Page model search fields',
+                        '`search_fields = Page.search_fields + [...]`'
+                    ]).format(cls.__name__),
+                    obj=cls,
+                    id='wagtailsearch.W001'
+                ))
+
+    return errors

wagtail/search/tests/test_indexed_class.py

@@ -3,8 +3,10 @@ from contextlib import contextmanager
 from django.core import checks
 from django.test import TestCase
 
+from wagtail.core.models import Page
 from wagtail.search import index
 from wagtail.tests.search import models
+from wagtail.tests.testapp.models import EventPage, SingleEventPage
 
 
 @contextmanager
@@ -33,6 +35,15 @@ class TestSearchFields(TestCase):
     def make_dummy_type(self, search_fields):
         return type(str('DummyType'), (index.Indexed, ), dict(search_fields=search_fields))
 
+    def get_checks_result(warning_id=None):
+        """Run Django checks on any with the 'search' tag used when registering the check"""
+        checks_result = checks.run_checks()
+        if warning_id:
+            return [
+                warning for warning in
+                checks_result if warning.id == warning_id]
+        return checks_result
+
     def test_basic(self):
         cls = self.make_dummy_type([
             index.SearchField('test', boost=100, partial_match=False),
@@ -92,3 +103,23 @@ class TestSearchFields(TestCase):
             ]
             errors = models.Book.check()
             self.assertEqual(errors, expected_errors)
+
+    def test_checking_core_page_fields_are_indexed(self):
+        """Run checks to ensure that when core page fields are missing we get a warning"""
+
+        # first confirm that errors show as EventPage (in test models) has no Page.search_fields
+        errors = [error for error in checks.run_checks() if error.id == 'wagtailsearch.W001']
+
+        # should only ever get this warning on the sub-classes of the page model
+        self.assertEqual([EventPage, SingleEventPage], [error.obj for error in errors])
+
+        for error in errors:
+            self.assertEqual(error.msg, 'Core Page fields missing in `search_fields`', )
+            self.assertIn(
+                'Page model search fields `search_fields = Page.search_fields + [...]`',
+                error.hint)
+
+        # second check that we get no errors when setting up the models correctly
+        with patch_search_fields(EventPage, Page.search_fields + EventPage.search_fields):
+            errors = [error for error in checks.run_checks() if error.id == 'wagtailsearch.W001']
+            self.assertEqual([], errors)