Allow custom Page Managers

Previously, if a developer wanted to use a custom Manager on their Page
subclass, some fairly hacky hacks were required. Now, the `objects`
attribute is only overridden if it is a plain `Manager`. If it is
anything else, it is left alone. A system check has been added to ensure
that all `Page` managers inherit from `PageManager`

CHANGELOG.txt

@@ -17,6 +17,7 @@ Changelog
  * `page_unpublish` signal is now fired for each page that was unpublished by a call to `PageQuerySet.unpublish()`
  * Add `get_upload_to` method to `AbstractImage`, to allow overriding the default image upload path (Ben Emery)
  * Notification emails are now sent per user (Matthew Downey)
+ * Added the ability to override the default manager on Page models
  * New translations for Arabic and Latvian
  * Fix: HTTP cache purge now works again on Python 2 (Mitchel Cabuloy)
  * Fix: Locked pages can no longer be unpublished (Alex Bridge)

docs/releases/1.3.rst

@@ -62,6 +62,7 @@ Minor features
  * ``page_unpublish`` signal is now fired for each page that was unpublished by a call to ``PageQuerySet.unpublish()``
  * Add `get_upload_to` method to `AbstractImage`, to allow overriding the default image upload path (Ben Emery)
  * Notification emails are now sent per user (Matthew Downey)
+ * Added the ability to override the default manager on Page models
  * New translations for Arabic and Latvian
 
 

docs/topics/pages.rst

@@ -440,45 +440,38 @@ This is because ``Page`` enforces ordering QuerySets by path. Instead you must a
 
     news_items = NewsItemPage.objects.live().order_by('-publication_date')
 
-Page custom managers
+Custom Page managers
 --------------------
 
-``Page`` enforces its own 'objects' manager in its ``__init__`` method, so you cannot add a custom manager at the 'objects' attribute.
+You can add a custom Manager to your ``Page`` class. Any custom ``Manager``\s should inherit from :class:`wagtail.wagtailcore.models.PageManager`:
 
 .. code-block:: python
 
-    class EventPageQuerySet(PageQuerySet):
+    from django.db import models
+    from wagtail.wagtailcore.models import Page, PageManager
 
-        def future(self):
-            return self.filter(
-                start_date__gte=timezone.localtime(timezone.now()).date()
-            )
+    class EventPageManager(PageManager):
+        """ Custom manager for Event pages """
 
     class EventPage(Page):
         start_date = models.DateField()
 
-        objects = EventPageQuerySet.as_manager()  # will not work
+        objects = EventPageManager()
 
-To use a custom manager you must choose a different attribute name. Make sure to subclass ``wagtail.wagtailcore.models.PageManager``.
+Alternately, if you only need to add extra ``QuerySet`` methods, you can inherit from :class:`wagtail.wagtailcore.models.PageQuerySet`, and call :func:`~django.db.models.managers.Manager.from_queryset` to build a custom ``Manager``:
 
 .. code-block:: python
 
     from django.db import models
     from django.utils import timezone
-    from wagtail.wagtailcore.models import Page, PageManager
-
-
-    class FutureEventPageManager(PageManager):
-
-        def get_queryset(self):
-            return super().get_queryset().filter(
-                start_date__gte=timezone.localtime(timezone.now()).date()
-            )
+    from wagtail.wagtailcore.models import Page, PageManager, PageQuerySet
 
+    class EventPageQuerySet(PageQuerySet):
+        def future(self):
+            today = timezone.localtime(timezone.now()).date()
+            return self.filter(start_date__gte=today)
 
     class EventPage(Page):
         start_date = models.DateField()
 
-        future_events = FutureEventPageManager()
-
-Then you can use ``EventPage.future_events`` in the manner you might expect.
+        objects = PageManager.from_queryset(EventQuerySet)

wagtail/tests/testapp/migrations/0021_custommanagerpage.py

@@ -0,0 +1,25 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('wagtailcore', '0021_capitalizeverbose'),
+        ('tests', '0020_capitalizeverbose'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='CustomManagerPage',
+            fields=[
+                ('page_ptr', models.OneToOneField(parent_link=True, auto_created=True, primary_key=True, serialize=False, to='wagtailcore.Page')),
+            ],
+            options={
+                'abstract': False,
+            },
+            bases=('wagtailcore.page',),
+        ),
+    ]

wagtail/tests/testapp/models.py

@@ -17,7 +17,7 @@ from modelcluster.models import ClusterableModel
 from modelcluster.contrib.taggit import ClusterTaggableManager
 
 from wagtail.contrib.settings.models import BaseSetting, register_setting
-from wagtail.wagtailcore.models import Page, Orderable
+from wagtail.wagtailcore.models import Page, Orderable, PageManager
 from wagtail.wagtailcore.fields import RichTextField, StreamField
 from wagtail.wagtailcore.blocks import CharBlock, RichTextBlock
 from wagtail.wagtailadmin.edit_handlers import (
@@ -577,3 +577,11 @@ class CustomImageFilePath(AbstractImage):
 
         self.file.seek(original_position)
         return os.path.join(folder_name, checksum[:3], filename)
+
+
+class CustomManager(PageManager):
+    pass
+
+
+class CustomManagerPage(Page):
+    objects = CustomManager()

wagtail/wagtailcore/models.py

@@ -1,5 +1,6 @@
 from __future__ import unicode_literals
 
+
 import logging
 import json
 import warnings
@@ -229,8 +230,13 @@ class PageBase(models.base.ModelBase):
             # don't proceed with all this page type registration stuff
             return
 
-        # Add page manager
-        PageManager().contribute_to_class(cls, 'objects')
+        # Override the default `objects` attribute with a `PageManager`.
+        # Managers are not inherited by MTI child models, so `Page` subclasses
+        # will get a plain `Manager` instead of a `PageManager`.
+        # If the developer has set their own custom `Manager` subclass, do not
+        # clobber it.
+        if type(cls.objects) is models.Manager:
+            PageManager().contribute_to_class(cls, 'objects')
 
         if 'template' not in dct:
             # Define a default template path derived from the app name and model name
@@ -342,6 +348,8 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed
     # Do not allow plain Page instances to be created through the Wagtail admin
     is_creatable = False
 
+    objects = PageManager()
+
     def __init__(self, *args, **kwargs):
         super(Page, self).__init__(*args, **kwargs)
         if not self.id and not self.content_type_id:
@@ -453,6 +461,17 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed
                         )
                     )
 
+        if not isinstance(cls.objects, PageManager):
+            errors.append(
+                checks.Error(
+                    "Manager does not inherit from PageManager",
+                    hint="Ensure that custom Page managers inherit from {}.{}".format(
+                        PageManager.__module__, PageManager.__name__),
+                    obj=cls,
+                    id='wagtailcore.E002',
+                )
+            )
+
         return errors
 
     def _update_descendant_url_paths(self, old_url_path, new_url_path):

wagtail/wagtailcore/tests/test_page_model.py

@@ -10,13 +10,14 @@ from django.contrib.contenttypes.models import ContentType
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import AnonymousUser
 
-from wagtail.wagtailcore.models import Page, Site, get_page_models
+from wagtail.wagtailcore.models import Page, Site, get_page_models, PageManager
 from wagtail.tests.testapp.models import (
     SingleEventPage, EventPage, EventIndex, SimplePage,
     BusinessIndex, BusinessSubIndex, BusinessChild, StandardIndex,
     MTIBasePage, MTIChildPage, AbstractPage, TaggedPage,
     BlogCategory, BlogCategoryBlogPage, Advert, ManyToManyBlogPage,
-    GenericSnippetPage, BusinessNowherePage, SingletonPage)
+    GenericSnippetPage, BusinessNowherePage, SingletonPage,
+    CustomManager, CustomManagerPage)
 from wagtail.tests.utils import WagtailTestUtils
 
 
@@ -1003,3 +1004,27 @@ class TestIsCreatable(TestCase):
         """
         self.assertFalse(AbstractPage.is_creatable)
         self.assertNotIn(AbstractPage, get_page_models())
+
+
+class TestPageManager(TestCase):
+    def test_page_manager(self):
+        """
+        Assert that the Page class uses PageManager
+        """
+        self.assertIs(type(Page.objects), PageManager)
+
+    def test_page_subclass_manager(self):
+        """
+        Assert that Page subclasses get a PageManager without having to do
+        anything special. MTI subclasses do *not* inherit their parents Manager
+        by default.
+        """
+        self.assertIs(type(SimplePage.objects), PageManager)
+
+    def test_custom_page_manager(self):
+        """
+        Subclasses should be able to override their default Manager, and
+        Wagtail should respect this. It is up to the developer to ensure their
+        custom Manager inherits from PageManager.
+        """
+        self.assertIs(type(CustomManagerPage.objects), CustomManager)