From d964675ee8fcb7ea58681ac8869733a86d58e4ec Mon Sep 17 00:00:00 2001 From: LB Johnston Date: Mon, 25 Mar 2019 21:02:46 +1000 Subject: [PATCH] add check for correct search_fields on pages - fixes #4940 --- CHANGELOG.txt | 1 + docs/releases/2.16.md | 1 + wagtail/search/apps.py | 2 ++ wagtail/search/checks.py | 26 ++++++++++++++++++ wagtail/search/tests/test_indexed_class.py | 31 ++++++++++++++++++++++ 5 files changed, 61 insertions(+) create mode 100644 wagtail/search/checks.py diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 3964551b7f..8fb18c78af 100644 --- a/CHANGELOG.txt +++ b/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) diff --git a/docs/releases/2.16.md b/docs/releases/2.16.md index 5ebcb8c97d..c33944fe1c 100644 --- a/docs/releases/2.16.md +++ b/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 diff --git a/wagtail/search/apps.py b/wagtail/search/apps.py index 597c0686c4..b3a439c055 100644 --- a/wagtail/search/apps.py +++ b/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' diff --git a/wagtail/search/checks.py b/wagtail/search/checks.py new file mode 100644 index 0000000000..4814ce5832 --- /dev/null +++ b/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 diff --git a/wagtail/search/tests/test_indexed_class.py b/wagtail/search/tests/test_indexed_class.py index c6eac5622d..882e8dd4a7 100644 --- a/wagtail/search/tests/test_indexed_class.py +++ b/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)