Add system check to validate parent_page_types / subpage_types - fixes #1128

CHANGELOG.txt

@@ -7,6 +7,7 @@ Changelog
  * Django 1.9 support
  * Support for indexing fields across relations in Elasticsearch
  * Added toolbar to cross-link between different search areas, and `register_admin_search_area` hook to register new areas (Ben Kerle)
+ * Added system checks to check the `subpage_types` and `parent_page_types` attriutes of page models
  * Added `WAGTAIL_PASSWORD_RESET_ENABLED` setting to allow password resets to be disabled independently of the password management interface (John Draper)
  * Updated fonts for more comprehensive Unicode support
  * Added `.alt` attribute to image renditions

docs/releases/1.3.rst

@@ -52,6 +52,7 @@ The search interface in the Wagtail admin now includes a toolbar to quickly swit
 Minor features
 ~~~~~~~~~~~~~~
 
+ * Added system checks to check the `subpage_types` and `parent_page_types` attriutes of page models
  * Added ``WAGTAIL_PASSWORD_RESET_ENABLED`` setting to allow password resets to be disabled independently of the password management interface (John Draper)
  * Updated fonts for more comprehensive Unicode support
  * Added ``.alt`` attribute to image renditions

wagtail/wagtailcore/models.py

@@ -25,7 +25,7 @@ from django.utils import timezone
 from django.utils.six import StringIO
 from django.utils.six.moves.urllib.parse import urlparse
 from django.utils.translation import ugettext_lazy as _
-from django.core.exceptions import ValidationError, ImproperlyConfigured
+from django.core.exceptions import ValidationError
 from django.utils.functional import cached_property
 from django.utils.encoding import python_2_unicode_compatible
 from django.core import checks
@@ -479,6 +479,28 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed
                 )
             )
 
+        try:
+            cls.clean_subpage_models()
+        except (ValueError, LookupError) as e:
+            errors.append(
+                checks.Error(
+                    "Invalid subpage_types setting for %s" % cls,
+                    hint=str(e),
+                    id='wagtailcore.E002'
+                )
+            )
+
+        try:
+            cls.clean_parent_page_models()
+        except (ValueError, LookupError) as e:
+            errors.append(
+                checks.Error(
+                    "Invalid parent_page_types setting for %s" % cls,
+                    hint=str(e),
+                    id='wagtailcore.E002'
+                )
+            )
+
         return errors
 
     def _update_descendant_url_paths(self, old_url_path, new_url_path):
@@ -739,7 +761,9 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed
     @classmethod
     def clean_subpage_models(cls):
         """
-        Returns the list of subpage types, normalised as model classes
+        Returns the list of subpage types, normalised as model classes.
+        Throws ValueError if any entry in subpage_types cannot be recognised as a model name,
+        or LookupError if a model does not exist (or is not a Page subclass).
         """
         if cls._clean_subpage_models is None:
             subpage_types = getattr(cls, 'subpage_types', None)
@@ -747,16 +771,14 @@ 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
                 cls._clean_subpage_models = get_page_models()
             else:
-                try:
+                cls._clean_subpage_models = [
-                    cls._clean_subpage_models = [
+                    resolve_model_string(model_string, cls._meta.app_label)
-                        resolve_model_string(model_string, cls._meta.app_label)
+                    for model_string in subpage_types
-                        for model_string in subpage_types
+                ]
-                    ]
+
-                except LookupError as err:
+                for model in cls._clean_subpage_models:
-                    raise ImproperlyConfigured(
+                    if not issubclass(model, Page):
-                        "{0}.subpage_types must be a list of 'app_label.model_name' strings, given {1!r}"
+                        raise LookupError("%s is not a Page subclass" % model)
-                        .format(cls.__name__, err.args[1])
-                    )
 
         return cls._clean_subpage_models
 
@@ -775,7 +797,9 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed
     @classmethod
     def clean_parent_page_models(cls):
         """
-        Returns the list of parent page types, normalised as model classes
+        Returns the list of parent page types, normalised as model classes.
+        Throws ValueError if any entry in parent_page_types cannot be recognised as a model name,
+        or LookupError if a model does not exist (or is not a Page subclass).
         """
 
         if cls._clean_parent_page_models is None:
@@ -784,16 +808,14 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed
                 # if parent_page_types is not specified on the Page class, allow all page types as subpages
                 cls._clean_parent_page_models = get_page_models()
             else:
-                try:
+                cls._clean_parent_page_models = [
-                    cls._clean_parent_page_models = [
+                    resolve_model_string(model_string, cls._meta.app_label)
-                        resolve_model_string(model_string, cls._meta.app_label)
+                    for model_string in parent_page_types
-                        for model_string in parent_page_types
+                ]
-                    ]
+
-                except LookupError as err:
+                for model in cls._clean_parent_page_models:
-                    raise ImproperlyConfigured(
+                    if not issubclass(model, Page):
-                        "{0}.parent_page_types must be a list of 'app_label.model_name' strings, given {1!r}"
+                        raise LookupError("%s is not a Page subclass" % model)
-                        .format(cls.__name__, err.args[1])
-                    )
 
         return cls._clean_parent_page_models