From 9e5ba4bd3ebab6a1fc409894ab9d9113b93e087c Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Wed, 9 Dec 2015 16:53:49 +0000 Subject: [PATCH] Add system check to validate parent_page_types / subpage_types - fixes #1128 --- CHANGELOG.txt | 1 + docs/releases/1.3.rst | 1 + wagtail/wagtailcore/models.py | 68 +++++++++++++++++++++++------------ 3 files changed, 47 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 0843ec10b..303fff485 100644 --- a/CHANGELOG.txt +++ b/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 diff --git a/docs/releases/1.3.rst b/docs/releases/1.3.rst index 030154255..6690acf84 100644 --- a/docs/releases/1.3.rst +++ b/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 diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 11a709500..7392393fb 100644 --- a/wagtail/wagtailcore/models.py +++ b/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 = [ - 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]) - ) + cls._clean_subpage_models = [ + resolve_model_string(model_string, cls._meta.app_label) + for model_string in subpage_types + ] + + for model in cls._clean_subpage_models: + if not issubclass(model, Page): + raise LookupError("%s is not a Page subclass" % model) 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 = [ - 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]) - ) + cls._clean_parent_page_models = [ + resolve_model_string(model_string, cls._meta.app_label) + for model_string in parent_page_types + ] + + for model in cls._clean_parent_page_models: + if not issubclass(model, Page): + raise LookupError("%s is not a Page subclass" % model) return cls._clean_parent_page_models