Merge branch 'feature/parent_page_types' of https://github.com/takeflight/wagtail into takeflight-feature/parent_page_types

docs/core_components/pages/theory.rst

@@ -52,7 +52,7 @@ A Parent node could provide its own function returning its descendant objects.
 
             return events
 
-This example makes sure to limit the returned objects to pieces of content which make sense, specifically ones which have been published through Wagtail's admin interface (``live()``) and are children of this node (``descendant_of(self)``). By setting a ``subpage_types`` class property in your model, you can specify which models are allowed to be set as children, but Wagtail will allow any ``Page``-derived model by default. Regardless, it's smart for a parent model to provide an index filtered to make sense.
+This example makes sure to limit the returned objects to pieces of content which make sense, specifically ones which have been published through Wagtail's admin interface (``live()``) and are children of this node (``descendant_of(self)``). By setting a ``subpage_types`` class property in your model, you can specify which models are allowed to be set as children, and by settings a ``parent_page_types`` class property, you can specify which models are allowed to parent certain children. Wagtail will allow any ``Page``-derived model by default. Regardless, it's smart for a parent model to provide an index filtered to make sense.
 
 
 Leaves
@@ -71,7 +71,7 @@ The model for the leaf could provide a function that traverses the tree in the o
             # Find closest ancestor which is an event index
             return self.get_ancestors().type(EventIndexPage).last()
 
-If defined, ``subpage_types`` will also limit the parent models allowed to contain a leaf. If not, Wagtail will allow any combination of parents and leafs to be associated in the Wagtail tree. Like with index pages, it's a good idea to make sure that the index is actually of the expected model to contain the leaf.
+If defined, ``subpage_types`` and ``parent_page_types`` will also limit the parent models allowed to contain a leaf. If not, Wagtail will allow any combination of parents and leafs to be associated in the Wagtail tree. Like with index pages, it's a good idea to make sure that the index is actually of the expected model to contain the leaf.
 
 
 Other Relationships

wagtail/tests/models.py

@@ -376,7 +376,9 @@ class ZuluSnippet(models.Model):
 
 
 class StandardIndex(Page):
-    pass
+    """ Index for the site, not allowed to be placed anywhere """
+    parent_page_types = []
+
 
 StandardIndex.content_panels = [
     FieldPanel('title', classname="full title"),
@@ -387,14 +389,22 @@ StandardIndex.content_panels = [
 class StandardChild(Page):
     pass
 
+
 class BusinessIndex(Page):
+    """ Can be placed anywhere, can only have Business children """
     subpage_types = ['tests.BusinessChild', 'tests.BusinessSubIndex']
 
+
 class BusinessSubIndex(Page):
+    """ Can be placed under BusinessIndex, and have BusinessChild children """
     subpage_types = ['tests.BusinessChild']
+    parent_page_types = ['tests.BusinessIndex']
+
 
 class BusinessChild(Page):
+    """ Can only be placed under Business indexes, no children allowed """
     subpage_types = []
+    parent_page_types = ['tests.BusinessIndex', 'tests.BusinessSubIndex']
 
 
 class SearchTest(models.Model, index.Indexed):

wagtail/wagtailadmin/edit_handlers.py

@@ -19,8 +19,8 @@ from django.core.urlresolvers import reverse
 from django.utils.translation import ugettext_lazy
 
 from wagtail.wagtailcore.models import Page
-from wagtail.wagtailcore.utils import camelcase_to_underscore
 from wagtail.wagtailcore.fields import RichTextArea
+from wagtail.wagtailcore.utils import camelcase_to_underscore, resolve_model_string
 
 
 FORM_FIELD_OVERRIDES = {}
@@ -483,25 +483,16 @@ class BasePageChooserPanel(BaseChooserPanel):
     def target_content_type(cls):
         if cls._target_content_type is None:
             if cls.page_type:
-                if isinstance(cls.page_type, string_types):
-                    # translate the passed model name into an actual model class
-                    from django.db.models import get_model
-                    try:
-                        app_label, model_name = cls.page_type.split('.')
-                    except ValueError:
-                        raise ImproperlyConfigured("The page_type passed to PageChooserPanel must be of the form 'app_label.model_name'")
+                try:
+                    model = resolve_model_string(cls.page_type)
+                except LookupError:
+                    raise ImproperlyConfigured("{0}.page_type must be of the form 'app_label.model_name', given {1!r}".format(
+                        cls.__name__, cls.page_type))
+                except ValueError:
+                    raise ImproperlyConfigured("{0}.page_type refers to model {1!r} that has not been installed".format(
+                        cls.__name__, cls.page_type))
 
-                    try:
-                        page_type = get_model(app_label, model_name)
-                    except LookupError:
-                        page_type = None
-
-                    if page_type is None:
-                        raise ImproperlyConfigured("PageChooserPanel refers to model '%s' that has not been installed" % cls.page_type)
-                else:
-                    page_type = cls.page_type
-
-                cls._target_content_type = ContentType.objects.get_for_model(page_type)
+                cls._target_content_type = ContentType.objects.get_for_model(model)
             else:
                 # TODO: infer the content type by introspection on the foreign key
                 cls._target_content_type = ContentType.objects.get_by_natural_key('wagtailcore', 'page')

wagtail/wagtailadmin/tests/test_pages_views.py

@@ -8,7 +8,11 @@ from django.core import mail
 from django.core.paginator import Paginator
 from django.utils import timezone
 
-from wagtail.tests.models import SimplePage, EventPage, EventPageCarouselItem, StandardIndex, BusinessIndex, BusinessChild, BusinessSubIndex, TaggedPage, Advert, AdvertPlacement
+from wagtail.tests.models import (
+    SimplePage, EventPage, EventPageCarouselItem,
+    StandardIndex, StandardChild,
+    BusinessIndex, BusinessChild, BusinessSubIndex,
+    TaggedPage, Advert, AdvertPlacement)
 from wagtail.tests.utils import unittest, WagtailTestUtils
 from wagtail.wagtailcore.models import Page, PageRevision
 from wagtail.wagtailcore.signals import page_published, page_unpublished
@@ -1483,11 +1487,14 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils):
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, add_subpage_url)
 
-        # add_subpage should give us the full set of page types to choose
+        # add_subpage should give us choices of StandardChild, and BusinessIndex.
+        # BusinessSubIndex and BusinessChild are not allowed
         response = self.client.get(add_subpage_url)
         self.assertEqual(response.status_code, 200)
-        self.assertContains(response, 'Standard Child')
-        self.assertContains(response, 'Business Child')
+        self.assertContains(response, StandardChild.get_verbose_name())
+        self.assertContains(response, BusinessIndex.get_verbose_name())
+        self.assertNotContains(response, BusinessSubIndex.get_verbose_name())
+        self.assertNotContains(response, BusinessChild.get_verbose_name())
 
     def test_business_subpage(self):
         add_subpage_url = reverse('wagtailadmin_pages_add_subpage', args=(self.business_index.id, ))
@@ -1500,8 +1507,10 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils):
         # add_subpage should give us a cut-down set of page types to choose
         response = self.client.get(add_subpage_url)
         self.assertEqual(response.status_code, 200)
-        self.assertNotContains(response, 'Standard Child')
-        self.assertContains(response, 'Business Child')
+        self.assertNotContains(response, StandardIndex.get_verbose_name())
+        self.assertNotContains(response, StandardChild.get_verbose_name())
+        self.assertContains(response, BusinessSubIndex.get_verbose_name())
+        self.assertContains(response, BusinessChild.get_verbose_name())
 
     def test_business_child_subpage(self):
         add_subpage_url = reverse('wagtailadmin_pages_add_subpage', args=(self.business_child.id, ))
@@ -1516,12 +1525,16 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils):
         self.assertEqual(response.status_code, 403)
 
     def test_cannot_add_invalid_subpage_type(self):
-        # cannot add SimplePage as a child of BusinessIndex, as SimplePage is not present in subpage_types
-        response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'simplepage', self.business_index.id)))
+        # cannot add StandardChild as a child of BusinessIndex, as StandardChild is not present in subpage_types
+        response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'standardchild', self.business_index.id)))
         self.assertEqual(response.status_code, 403)
 
         # likewise for BusinessChild which has an empty subpage_types list
-        response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'simplepage', self.business_child.id)))
+        response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'standardchild', self.business_child.id)))
+        self.assertEqual(response.status_code, 403)
+
+        # cannot add BusinessChild to StandardIndex, as BusinessChild restricts is parent page types
+        response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'businesschild', self.standard_index.id)))
         self.assertEqual(response.status_code, 403)
 
         # but we can add a BusinessChild to BusinessIndex

wagtail/wagtailadmin/views/pages.py

@@ -68,7 +68,7 @@ def add_subpage(request, parent_page_id):
     if not parent_page.permissions_for_user(request.user).can_add_subpage():
         raise PermissionDenied
 
-    page_types = sorted(parent_page.clean_subpage_types(), key=lambda pagetype: pagetype.name.lower())
+    page_types = sorted(parent_page.allowed_subpage_types(), key=lambda pagetype: pagetype.name.lower())
 
     if len(page_types) == 1:
         # Only one page type is available - redirect straight to the create form rather than
@@ -136,7 +136,7 @@ def create(request, content_type_app_name, content_type_model_name, parent_page_
         raise Http404
 
     # page must be in the list of allowed subpage types for this parent ID
-    if content_type not in parent_page.clean_subpage_types():
+    if content_type not in parent_page.allowed_subpage_types():
         raise PermissionDenied
 
     page = page_class(owner=request.user)

wagtail/wagtailcore/models.py

@@ -8,7 +8,7 @@ from six.moves.urllib.parse import urlparse
 from modelcluster.models import ClusterableModel, get_all_child_relations
 
 from django.db import models, connection, transaction
-from django.db.models import get_model, Q
+from django.db.models import Q
 from django.http import Http404
 from django.core.cache import cache
 from django.core.handlers.wsgi import WSGIRequest
@@ -20,13 +20,13 @@ from django.conf import settings
 from django.template.response import TemplateResponse
 from django.utils import timezone
 from django.utils.translation import ugettext_lazy as _
-from django.core.exceptions import ValidationError
+from django.core.exceptions import ValidationError, ImproperlyConfigured
 from django.utils.functional import cached_property
 from django.utils.encoding import python_2_unicode_compatible
 
 from treebeard.mp_tree import MP_Node
 
-from wagtail.wagtailcore.utils import camelcase_to_underscore
+from wagtail.wagtailcore.utils import camelcase_to_underscore, resolve_model_string
 from wagtail.wagtailcore.query import PageQuerySet
 from wagtail.wagtailcore.url_routing import RouteResult
 
@@ -58,15 +58,15 @@ class Site(models.Model):
     @staticmethod
     def find_for_request(request):
         """
-            Find the site object responsible for responding to this HTTP
-            request object. Try:
-             - unique hostname first
-             - then hostname and port
-             - if there is no matching hostname at all, or no matching
-               hostname:port combination, fall back to the unique default site,
-               or raise an exception
-            NB this means that high-numbered ports on an extant hostname may
-            still be routed to a different hostname which is set as the default
+        Find the site object responsible for responding to this HTTP
+        request object. Try:
+         - unique hostname first
+         - then hostname and port
+         - if there is no matching hostname at all, or no matching
+           hostname:port combination, fall back to the unique default site,
+           or raise an exception
+        NB this means that high-numbered ports on an extant hostname may
+        still be routed to a different hostname which is set as the default
         """
         try:
             hostname = request.META['HTTP_HOST'].split(':')[0] # KeyError here goes to the final except clause
@@ -236,6 +236,7 @@ class PageBase(models.base.ModelBase):
             cls.ajax_template = None
 
         cls._clean_subpage_types = None  # to be filled in on first call to cls.clean_subpage_types
+        cls._clean_parent_page_types = None  # to be filled in on first call to cls.clean_parent_page_types
 
         if not dct.get('is_abstract'):
             # subclasses are only abstract if the subclass itself defines itself so
@@ -504,8 +505,8 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed
     @classmethod
     def clean_subpage_types(cls):
         """
-            Returns the list of subpage types, with strings converted to class objects
-            where required
+        Returns the list of subpage types, with strings converted to class objects
+        where required
         """
         if cls._clean_subpage_types is None:
             subpage_types = getattr(cls, 'subpage_types', None)
@@ -513,44 +514,79 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed
                 # if subpage_types is not specified on the Page class, allow all page types as subpages
                 res = get_page_types()
             else:
-                res = []
-                for page_type in cls.subpage_types:
-                    if isinstance(page_type, string_types):
-                        try:
-                            app_label, model_name = page_type.split(".")
-                        except ValueError:
-                            # If we can't split, assume a model in current app
-                            app_label = cls._meta.app_label
-                            model_name = page_type
-
-                        model = get_model(app_label, model_name)
-                        if model:
-                            res.append(ContentType.objects.get_for_model(model))
-                        else:
-                            raise NameError(_("name '{0}' (used in subpage_types list) is not defined.").format(page_type))
-
-                    else:
-                        # assume it's already a model class
-                        res.append(ContentType.objects.get_for_model(page_type))
+                try:
+                    models = [resolve_model_string(model_string, cls._meta.app_label)
+                              for model_string in subpage_types]
+                except LookupError as err:
+                    raise ImproperlyConfigured("{0}.subpage_types must be a list of 'app_label.model_name' strings, given {1!r}".format(
+                        cls.__name__, err.args[1]))
+                res = list(map(ContentType.objects.get_for_model, models))
 
             cls._clean_subpage_types = res
 
         return cls._clean_subpage_types
 
+    @classmethod
+    def clean_parent_page_types(cls):
+        """
+        Returns the list of parent page types, with strings converted to class
+        objects where required
+        """
+        if cls._clean_parent_page_types is None:
+            parent_page_types = getattr(cls, 'parent_page_types', None)
+            if parent_page_types is None:
+                # if parent_page_types is not specified on the Page class, allow all page types as subpages
+                res = get_page_types()
+            else:
+                try:
+                    models = [resolve_model_string(model_string, cls._meta.app_label)
+                              for model_string in parent_page_types]
+                except LookupError as err:
+                    raise ImproperlyConfigured("{0}.parent_page_types must be a list of 'app_label.model_name' strings, given {1!r}".format(
+                        cls.__name__, err.args[1]))
+                res = list(map(ContentType.objects.get_for_model, models))
+
+            cls._clean_parent_page_types = res
+
+        return cls._clean_parent_page_types
+
     @classmethod
     def allowed_parent_page_types(cls):
         """
-            Returns the list of page types that this page type can be a subpage of
+        Returns the list of page types that this page type can be a subpage of
         """
-        return [ct for ct in get_page_types() if cls in ct.model_class().clean_subpage_types()]
+        cls_ct = ContentType.objects.get_for_model(cls)
+        return [ct for ct in cls.clean_parent_page_types()
+                if cls_ct in ct.model_class().clean_subpage_types()]
+
+    @classmethod
+    def allowed_subpage_types(cls):
+        """
+        Returns the list of page types that this page type can be a subpage of
+        """
+        # Special case the 'Page' class, such as the Root page or Home page -
+        # otherwise you can not add initial pages when setting up a site
+        if cls == Page:
+            return get_page_types()
+
+        cls_ct = ContentType.objects.get_for_model(cls)
+        return [ct for ct in cls.clean_subpage_types()
+                if cls_ct in ct.model_class().clean_parent_page_types()]
 
     @classmethod
     def allowed_parent_pages(cls):
         """
-            Returns the list of pages that this page type can be a subpage of
+        Returns the list of pages that this page type can be a subpage of
         """
         return Page.objects.filter(content_type__in=cls.allowed_parent_page_types())
 
+    @classmethod
+    def allowed_subpages(cls):
+        """
+        Returns the list of pages that this page type can be a parent page of
+        """
+        return Page.objects.filter(content_type__in=cls.allowed_subpage_types())
+
     @classmethod
     def get_verbose_name(cls):
         """
@@ -1032,7 +1068,7 @@ class PagePermissionTester(object):
     def can_add_subpage(self):
         if not self.user.is_active:
             return False
-        if not self.page.specific_class.clean_subpage_types():  # this page model has an empty subpage_types list, so no subpages are allowed
+        if not self.page.specific_class.allowed_subpage_types():  # this page model has an empty subpage_types list, so no subpages are allowed
             return False
         return self.user.is_superuser or ('add' in self.permissions)
 
@@ -1096,7 +1132,7 @@ class PagePermissionTester(object):
         """
         if not self.user.is_active:
             return False
-        if not self.page.specific_class.clean_subpage_types():  # this page model has an empty subpage_types list, so no subpages are allowed
+        if not self.page.specific_class.allowed_subpage_types():  # this page model has an empty subpage_types list, so no subpages are allowed
             return False
 
         return self.user.is_superuser or ('publish' in self.permissions)

wagtail/wagtailcore/utils.py

@@ -1,6 +1,38 @@
 import re
+from django.db.models import Model, get_model
+from six import string_types
 
 
 def camelcase_to_underscore(str):
     # http://djangosnippets.org/snippets/585/
     return re.sub('(((?<=[a-z])[A-Z])|([A-Z](?![A-Z]|$)))', '_\\1', str).lower().strip('_')
+
+
+def resolve_model_string(model_string, default_app=None):
+    """
+    Resolve an 'app_label.model_name' string in to an actual model class.
+    If a model class is passed in, just return that.
+    """
+    if isinstance(model_string, string_types):
+        try:
+            app_label, model_name = model_string.split(".")
+        except ValueError:
+            if default_app is not None:
+                # If we can't split, assume a model in current app
+                app_label = default_app
+                model_name = model_string
+            else:
+                raise ValueError("Can not resolve {0!r} in to a model. Model names "
+                                 "should be in the form app_label.model_name".format(
+                                     model_string), model_string)
+
+        model = get_model(app_label, model_name)
+        if not model:
+            raise LookupError("Can not resolve {0!r} in to a model".format(model_string), model_string)
+        return model
+
+    elif model_string is not None and issubclass(model_string, Model):
+        return model
+
+    else:
+        raise LookupError("Can not resolve {0!r} in to a model".format(model_string), model_string)