From 1dad100695060435b597545cd61c3ee420afee9f Mon Sep 17 00:00:00 2001
From: Josh Schneier <josh.schneier@gmail.com>
Date: Thu, 31 Mar 2016 22:02:46 -0400
Subject: [PATCH] Add a check that the declared search_fields exist
---
CHANGELOG.txt | 1 +
docs/releases/1.6.rst | 1 +
wagtail/tests/search/models.py | 2 +-
wagtail/wagtailsearch/index.py | 29 +++++++++++++++++++
.../wagtailsearch/tests/test_indexed_class.py | 12 ++++++++
5 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/CHANGELOG.txt b/CHANGELOG.txt
index 14a71a6c79..22e7d51951 100644
--- a/CHANGELOG.txt
+++ b/CHANGELOG.txt
@@ -17,6 +17,7 @@ Changelog
* Add new EmailBlock and IntegerBlock (Oktay Altay)
* Added a new FloatBlock, DecimalBlock and a RegexBlock (Oktay Altay, Andy Babic)
* Wagtail version number is now shown on the settings menu (Chris Rogers)
+ * Added a system check to validate that fields listed in `search_fields` are defined on the model (Josh Schneier)
* Fix: Email templates and document uploader now support custom `STATICFILES_STORAGE` (Jonny Scholes)
* Fix: Removed alignment options (deprecated in HTML and not rendered by Wagtail) from `TableBlock` context menu (Moritz Pfeiffer)
* Fix: Fixed incorrect CSS path on ModelAdmin's "choose a parent page" view
diff --git a/docs/releases/1.6.rst b/docs/releases/1.6.rst
index 84e75d6467..2aa129a3ad 100644
--- a/docs/releases/1.6.rst
+++ b/docs/releases/1.6.rst
@@ -32,6 +32,7 @@ Minor features
* Added new EmailBlock and IntegerBlock (Oktay Altay)
* Added a new FloatBlock, DecimalBlock and a RegexBlock (Oktay Altay, Andy Babic)
* Wagtail version number is now shown on the settings menu (Chris Rogers)
+ * Added a system check to validate that fields listed in ``search_fields`` are defined on the model (Josh Schneier)
Bug fixes
diff --git a/wagtail/tests/search/models.py b/wagtail/tests/search/models.py
index f04e93dfe0..90ca4526a0 100644
--- a/wagtail/tests/search/models.py
+++ b/wagtail/tests/search/models.py
@@ -6,7 +6,7 @@ from taggit.managers import TaggableManager
from wagtail.wagtailsearch import index
-class SearchTest(models.Model, index.Indexed):
+class SearchTest(index.Indexed, models.Model):
title = models.CharField(max_length=255)
content = models.TextField()
live = models.BooleanField(default=False)
diff --git a/wagtail/wagtailsearch/index.py b/wagtail/wagtailsearch/index.py
index bc0ae3d170..98110f5665 100644
--- a/wagtail/wagtailsearch/index.py
+++ b/wagtail/wagtailsearch/index.py
@@ -3,6 +3,7 @@ from __future__ import absolute_import, unicode_literals
import logging
from django.apps import apps
+from django.core import checks
from django.db import models
from django.db.models.fields import FieldDoesNotExist
from django.db.models.fields.related import ForeignObjectRel, OneToOneRel, RelatedField
@@ -85,6 +86,34 @@ class Indexed(object):
"""
return self
+ @classmethod
+ def _has_field(cls, name):
+ try:
+ cls._meta.get_field(name)
+ return True
+ except models.fields.FieldDoesNotExist:
+ return hasattr(cls, name)
+
+ @classmethod
+ def check(cls, **kwargs):
+ errors = super(Indexed, cls).check(**kwargs)
+ errors.extend(cls._check_search_fields(**kwargs))
+ return errors
+
+ @classmethod
+ def _check_search_fields(cls, **kwargs):
+ errors = []
+ for field in cls.get_search_fields():
+ message = "{model}.search_fields contains field '{name}' but it doesn't exist"
+ if not cls._has_field(field.field_name):
+ errors.append(
+ checks.Warning(
+ message.format(model=cls.__name__, name=field.field_name),
+ obj=cls,
+ )
+ )
+ return errors
+
search_fields = SearchFieldsShouldBeAList([], name='search_fields on Indexed subclasses')
diff --git a/wagtail/wagtailsearch/tests/test_indexed_class.py b/wagtail/wagtailsearch/tests/test_indexed_class.py
index e7c9dd0684..5e518add7f 100644
--- a/wagtail/wagtailsearch/tests/test_indexed_class.py
+++ b/wagtail/wagtailsearch/tests/test_indexed_class.py
@@ -1,5 +1,6 @@
from __future__ import absolute_import, unicode_literals
+from django.core import checks
from django.test import TestCase
from wagtail.tests.search import models
@@ -68,3 +69,14 @@ class TestSearchFields(TestCase):
self.assertEqual(len(cls.get_search_fields()), 2)
self.assertEqual(len(cls.get_searchable_search_fields()), 1)
self.assertEqual(len(cls.get_filterable_search_fields()), 1)
+
+ def test_checking_search_fields(self):
+ models.SearchTest.search_fields += [index.SearchField('foo')]
+ expected_errors = [
+ checks.Warning(
+ "SearchTest.search_fields contains field 'foo' but it doesn't exist",
+ obj=models.SearchTest
+ )
+ ]
+ errors = models.SearchTest.check()
+ self.assertEqual(errors, expected_errors)