Change Indexed.search_fields to be a listish thing

Indexed.search_fields used to be a tuple. This is incorrect, and it
should have been a list.  Changing it to be a list now would be a
backwards incompatible change, as people do

    search_fields = Page.search_fields + (
        SearchField('body')
    )

Adding a tuple to the end of a list causes an error, so this would
cause all old code that used tuples to throw an error. This is not
great.

A new ThisShouldBeAList class, which subclasses list, has been added.
It additionally allows tuples to be added to it, as in the above
behaviour, but will raise a deprecation warning if someone does this.
Old code that uses tuples will continue to work, but raise a deprecation
warning.

See #2310

docs/getting_started/tutorial.rst

@@ -162,10 +162,10 @@ The following example defines a basic blog post model in ``blog/models.py``:
         intro = models.CharField(max_length=250)
         body = RichTextField(blank=True)
 
-        search_fields = Page.search_fields + (
+        search_fields = Page.search_fields + [
             index.SearchField('intro'),
             index.SearchField('body'),
-        )
+        ]
 
         content_panels = Page.content_panels + [
             FieldPanel('date'),
@@ -229,10 +229,10 @@ model:
         intro = models.CharField(max_length=250)
         body = RichTextField(blank=True)
 
-        search_fields = Page.search_fields + (
+        search_fields = Page.search_fields + [
             index.SearchField('intro'),
             index.SearchField('body'),
-        )
+        ]
 
         content_panels = Page.content_panels + [
             FieldPanel('date'),

docs/topics/pages.rst

@@ -50,10 +50,10 @@ This example represents a typical blog post:
 
         # Search index configuraiton
 
-        search_fields = Page.search_fields + (
+        search_fields = Page.search_fields + [
             index.SearchField('body'),
             index.FilterField('date'),
-        )
+        ]
 
 
         # Editor panels configuration
@@ -119,11 +119,11 @@ Search
 
 The ``search_fields`` attribute defines which fields are added to the search index and how they are indexed.
 
-This should be a tuple of ``SearchField`` and ``FilterField`` objects. ``SearchField`` adds a field for full-text search. ``FilterField`` adds a field for filtering the results. A field can be indexed with both ``SearchField`` and ``FilterField`` at the same time (but only one instance of each).
+This should be a list of ``SearchField`` and ``FilterField`` objects. ``SearchField`` adds a field for full-text search. ``FilterField`` adds a field for filtering the results. A field can be indexed with both ``SearchField`` and ``FilterField`` at the same time (but only one instance of each).
 
 In the above example, we've indexed ``body`` for full-text search and ``date`` for filtering.
 
-The arguments that these field types accept are documented here: :ref:`wagtailsearch_indexing_fields`.
+The arguments that these field types accept are documented in :ref:`wagtailsearch_indexing_fields`.
 
 
 Editor panels

docs/topics/search/indexing.rst

@@ -74,10 +74,10 @@ This creates an ``EventPage`` model with two fields: ``description`` and ``date`
         description = models.TextField()
         date = models.DateField()
 
-        search_fields = Page.search_fields + ( # Inherit search_fields from Page
+        search_fields = Page.search_fields + [ # Inherit search_fields from Page
             index.SearchField('description'),
             index.FilterField('date'),
-        )
+        ]
 
 
     # Get future events which contain the string "Christmas" in the title or description
@@ -180,13 +180,13 @@ One use for this is indexing the ``get_*_display`` methods Django creates automa
 
         is_private = models.BooleanField(choices=IS_PRIVATE_CHOICES)
 
-        search_fields = Page.search_fields + (
+        search_fields = Page.search_fields + [
             # Index the human-readable string for searching.
             index.SearchField('get_is_private_display'),
 
             # Index the boolean value for filtering.
             index.FilterField('is_private'),
-        )
+        ]
 
 Callables also provide a way to index fields from related models. In the example from :ref:`inline_panels`, to index each BookPage by the titles of its related_links:
 
@@ -222,14 +222,14 @@ To do this, inherit from ``index.Indexed`` and add some ``search_fields`` to the
         author = models.ForeignKey(Author)
         published_date = models.DateTimeField()
 
-        search_fields = (
+        search_fields = [
             index.SearchField('title', partial_match=True, boost=10),
             index.SearchField('get_genre_display'),
 
             index.FilterField('genre'),
             index.FilterField('author'),
             index.FilterField('published_date'),
-        )
+        ]
 
     # As this model doesn't have a search method in its QuerySet, we have to call search directly on the backend
     >>> from wagtail.wagtailsearch.backends import get_search_backend

wagtail/tests/demosite/models.py

@@ -144,9 +144,9 @@ class HomePage(Page):
         'related_links',
     )
 
-    search_fields = Page.search_fields + (
+    search_fields = Page.search_fields + [
         index.SearchField('body'),
-    )
+    ]
 
     class Meta:
         verbose_name = "homepage"
@@ -190,10 +190,10 @@ class StandardPage(Page):
         'related_links',
     )
 
-    search_fields = Page.search_fields + (
+    search_fields = Page.search_fields + [
         index.SearchField('intro'),
         index.SearchField('body'),
-    )
+    ]
 
 
 class StandardPageCarouselItem(Orderable, AbstractCarouselItem):
@@ -235,9 +235,9 @@ class StandardIndexPage(Page):
         'related_links',
     )
 
-    search_fields = Page.search_fields + (
+    search_fields = Page.search_fields + [
         index.SearchField('intro'),
-    )
+    ]
 
 
 class StandardIndexPageRelatedLink(Orderable, AbstractRelatedLink):
@@ -280,9 +280,9 @@ class BlogEntryPage(Page):
         'related_links',
     )
 
-    search_fields = Page.search_fields + (
+    search_fields = Page.search_fields + [
         index.SearchField('body'),
-    )
+    ]
 
     def get_blog_index(self):
         # Find closest ancestor which is a blog index
@@ -325,9 +325,9 @@ class BlogIndexPage(Page):
         'related_links',
     )
 
-    search_fields = Page.search_fields + (
+    search_fields = Page.search_fields + [
         index.SearchField('intro'),
-    )
+    ]
 
     def get_blog_entries(self):
         # Get list of live blog pages that are descendants of this page
@@ -412,11 +412,11 @@ class EventPage(Page):
         'speakers',
     )
 
-    search_fields = Page.search_fields + (
+    search_fields = Page.search_fields + [
         index.SearchField('get_audience_display'),
         index.SearchField('location'),
         index.SearchField('body'),
-    )
+    ]
 
     def get_event_index(self):
         # Find closest ancestor which is an event index
@@ -487,9 +487,9 @@ class EventIndexPage(Page):
         'related_links',
     )
 
-    search_fields = Page.search_fields + (
+    search_fields = Page.search_fields + [
         index.SearchField('intro'),
-    )
+    ]
 
     def get_events(self):
         # Get list of live event pages that are descendants of this page
@@ -548,12 +548,12 @@ class PersonPage(Page, ContactFieldsMixin):
         'related_links',
     ) + ContactFieldsMixin.api_fields
 
-    search_fields = Page.search_fields + (
+    search_fields = Page.search_fields + [
         index.SearchField('first_name'),
         index.SearchField('last_name'),
         index.SearchField('intro'),
         index.SearchField('biography'),
-    )
+    ]
 
 
 class PersonPageRelatedLink(Orderable, AbstractRelatedLink):
@@ -595,9 +595,9 @@ class ContactPage(Page, ContactFieldsMixin):
         'feed_image',
     ) + ContactFieldsMixin.api_fields
 
-    search_fields = Page.search_fields + (
+    search_fields = Page.search_fields + [
         index.SearchField('body'),
-    )
+    ]
 
 
 ContactPage.content_panels = Page.content_panels + [

wagtail/tests/snippets/models.py

@@ -46,9 +46,9 @@ class RegisterDecorator(models.Model):
 class SearchableSnippet(models.Model, index.Indexed):
     text = models.CharField(max_length=255)
 
-    search_fields = (
+    search_fields = [
         index.SearchField('text'),
-    )
+    ]
 
     def __str__(self):
         return self.text

wagtail/tests/test_utils.py

@@ -0,0 +1,36 @@
+# -*- coding: utf-8 -*
+from __future__ import unicode_literals
+
+import warnings
+
+from django.test import SimpleTestCase
+
+from wagtail.utils.deprecation import RemovedInWagtail17Warning, SearchFieldsShouldBeAList
+
+
+class TestThisShouldBeAList(SimpleTestCase):
+    def test_add_a_list(self):
+        with warnings.catch_warnings(record=True) as w:
+            base = SearchFieldsShouldBeAList(['hello'])
+            result = base + ['world']
+
+            # Ensure that adding things together works
+            self.assertEqual(result, ['hello', 'world'])
+            # Ensure that a new SearchFieldsShouldBeAList was returned
+            self.assertIsInstance(result, SearchFieldsShouldBeAList)
+            # Check that no deprecation warnings were raised
+            self.assertEqual(len(w), 0)
+
+    def test_add_a_tuple(self):
+        with warnings.catch_warnings(record=True) as w:
+            base = SearchFieldsShouldBeAList(['hello'])
+            result = base + ('world',)
+
+            # Ensure that adding things together works
+            self.assertEqual(result, ['hello', 'world'])
+            # Ensure that a new SearchFieldsShouldBeAList was returned
+            self.assertIsInstance(result, SearchFieldsShouldBeAList)
+            # Check that a deprecation warning was raised
+            self.assertEqual(len(w), 1)
+            warning = w[0]
+            self.assertIs(warning.category, RemovedInWagtail17Warning)

wagtail/tests/testapp/models.py

@@ -211,11 +211,11 @@ class EventPage(Page):
         related_name='+'
     )
 
-    search_fields = (
+    search_fields = [
         index.SearchField('get_audience_display'),
         index.SearchField('location'),
         index.SearchField('body'),
-    )
+    ]
 
     password_required_template = 'tests/event_page_password_required.html'
 

wagtail/utils/deprecation.py

@@ -1,3 +1,6 @@
+import warnings
+
+
 class RemovedInWagtail16Warning(DeprecationWarning):
     pass
 
@@ -7,3 +10,50 @@ removed_in_next_version_warning = RemovedInWagtail16Warning
 
 class RemovedInWagtail17Warning(PendingDeprecationWarning):
     pass
+
+
+class ThisShouldBeAList(list):
+    """
+    Some properties - such as Indexed.search_fields - used to be tuples. This
+    is incorrect, and they should have been lists.  Changing these to be a list
+    now would be backwards incompatible, as people do
+
+    .. code-block:: python
+
+        search_fields = Page.search_fields + (
+            SearchField('body')
+        )
+
+    Adding a tuple to the end of a list causes an error.
+
+    This class will allow tuples to be added to it, as in the above behaviour,
+    but will raise a deprecation warning if someone does this.
+    """
+    message = 'Using a {type} for {name} is deprecated, use a list instead'
+
+    def __init__(self, items, name, category):
+        super(ThisShouldBeAList, self).__init__(items)
+        self.name = name
+        self.category = category
+
+    def _format_message(self, rhs):
+        return self.message.format(name=self.name, type=type(rhs).__name__)
+
+    def __add__(self, rhs):
+        cls = type(self)
+        if isinstance(rhs, tuple):
+            # Seems that a tuple was passed in. Raise a deprecation
+            # warning, but then keep going anyway.
+            message = self._format_message(rhs)
+            warnings.warn(message, category=self.category, stacklevel=2)
+            rhs = list(rhs)
+        return cls(super(ThisShouldBeAList, self).__add__(list(rhs)),
+                   name=self.name, category=self.category)
+
+
+class SearchFieldsShouldBeAList(ThisShouldBeAList):
+    """
+    Indexed.search_fields was a tuple, but it should have been a list
+    """
+    def __init__(self, items, name='search_fields', category=RemovedInWagtail17Warning):
+        super(SearchFieldsShouldBeAList, self).__init__(items, name, category)

wagtail/wagtailadmin/taggable.py

@@ -2,6 +2,7 @@ from django.contrib.contenttypes.models import ContentType
 from django.db.models import Count
 from taggit.models import Tag
 
+from wagtail.utils.deprecation import SearchFieldsShouldBeAList
 from wagtail.wagtailsearch import index
 
 
@@ -11,12 +12,12 @@ class TagSearchable(index.Indexed):
     for models that provide those things.
     """
 
-    search_fields = (
+    search_fields = SearchFieldsShouldBeAList([
         index.SearchField('title', partial_match=True, boost=10),
         index.RelatedFields('tags', [
             index.SearchField('name', partial_match=True, boost=10),
         ]),
-    )
+    ], name='search_fields on TagSearchable subclasses')
 
     @classmethod
     def get_indexed_objects(cls):

wagtail/wagtailadmin/tests/test_pages_views.py

@@ -1511,7 +1511,7 @@ class TestPageSearch(TestCase, WagtailTestUtils):
         search_fields = Page.search_fields
 
         # Add slug to the search_fields
-        Page.search_fields = Page.search_fields + (SearchField('slug', partial_match=True),)
+        Page.search_fields = Page.search_fields + [SearchField('slug', partial_match=True)]
 
         # Confirm the slug is being searched
         response = self.get({'q': "hello"})

wagtail/wagtailcore/models.py

@@ -30,6 +30,7 @@ from django.utils.translation import ugettext_lazy as _
 from modelcluster.models import ClusterableModel, get_all_child_relations
 from treebeard.mp_tree import MP_Node
 
+from wagtail.utils.deprecation import SearchFieldsShouldBeAList
 from wagtail.wagtailcore.query import PageQuerySet, TreeQuerySet
 from wagtail.wagtailcore.signals import page_published, page_unpublished
 from wagtail.wagtailcore.url_routing import RouteResult
@@ -324,7 +325,7 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed
         editable=False
     )
 
-    search_fields = (
+    search_fields = SearchFieldsShouldBeAList([
         index.SearchField('title', partial_match=True, boost=2),
         index.FilterField('id'),
         index.FilterField('live'),
@@ -336,7 +337,7 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed
         index.FilterField('show_in_menus'),
         index.FilterField('first_published_at'),
         index.FilterField('latest_revision_created_at'),
-    )
+    ], name='search_fields on Page subclasses')
 
     # Do not allow plain Page instances to be created through the Wagtail admin
     is_creatable = False
@@ -1786,9 +1787,9 @@ class CollectionMember(models.Model):
         on_delete=models.CASCADE
     )
 
-    search_fields = (
+    search_fields = SearchFieldsShouldBeAList([
         index.FilterField('collection'),
-    )
+    ], name='search_fields on CollectionMember subclasses')
 
     class Meta:
         abstract = True

wagtail/wagtaildocs/models.py

@@ -13,6 +13,7 @@ from django.utils.encoding import python_2_unicode_compatible
 from django.utils.translation import ugettext_lazy as _
 from taggit.managers import TaggableManager
 
+from wagtail.utils.deprecation import SearchFieldsShouldBeAList
 from wagtail.wagtailadmin.taggable import TagSearchable
 from wagtail.wagtailadmin.utils import get_object_usage
 from wagtail.wagtailcore.models import CollectionMember
@@ -42,9 +43,9 @@ class AbstractDocument(CollectionMember, TagSearchable):
 
     objects = DocumentQuerySet.as_manager()
 
-    search_fields = TagSearchable.search_fields + CollectionMember.search_fields + (
+    search_fields = SearchFieldsShouldBeAList(TagSearchable.search_fields + CollectionMember.search_fields + [
         index.FilterField('uploaded_by_user'),
-    )
+    ], name='search_fields on Document subclasses')
 
     def __str__(self):
         return self.title

wagtail/wagtailimages/models.py

@@ -120,9 +120,9 @@ class AbstractImage(CollectionMember, TagSearchable):
         return reverse('wagtailimages:image_usage',
                        args=(self.id,))
 
-    search_fields = TagSearchable.search_fields + CollectionMember.search_fields + (
+    search_fields = TagSearchable.search_fields + CollectionMember.search_fields + [
         index.FilterField('uploaded_by_user'),
-    )
+    ]
 
     def __str__(self):
         return self.title

wagtail/wagtailsearch/index.py

@@ -3,6 +3,8 @@ from django.db import models
 from django.db.models.fields import FieldDoesNotExist
 from django.db.models.fields.related import ForeignObjectRel, OneToOneRel, RelatedField
 
+from wagtail.utils.deprecation import SearchFieldsShouldBeAList
+
 
 class Indexed(object):
     @classmethod
@@ -75,7 +77,7 @@ class Indexed(object):
         """
         return self
 
-    search_fields = ()
+    search_fields = SearchFieldsShouldBeAList([], name='search_fields on Indexed subclasses')
 
 
 def get_indexed_models():

wagtail/wagtailsearch/tests/test_indexed_class.py

@@ -34,7 +34,7 @@ class TestSearchFields(TestCase):
         # standard convention of:
         #
         #     class SpecificPageType(Page):
-        #         search_fields = Page.search_fields + (some_other_definitions)
+        #         search_fields = Page.search_fields + [some_other_definitions]
         #
         # ...causes the definitions in some_other_definitions to override Page.search_fields
         # as intended.