From c823ace6a1c48aab9e93b4387adc30c0fd1d749a Mon Sep 17 00:00:00 2001 From: Tim Heap Date: Mon, 28 Mar 2016 01:52:19 +0200 Subject: [PATCH] Check form class from Page edit handler for correct type Forms for Page classes must subclass WagtailAdminPageForm. If they do not, an error will be thrown for invalid arguments when the Page editor is opened. Partial fix for #2267. --- .../wagtailadmin/tests/test_edit_handlers.py | 53 ++++++++++++++----- wagtail/wagtailcore/models.py | 16 ++++-- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/wagtail/wagtailadmin/tests/test_edit_handlers.py b/wagtail/wagtailadmin/tests/test_edit_handlers.py index 95847291d1..7ff38d3061 100644 --- a/wagtail/wagtailadmin/tests/test_edit_handlers.py +++ b/wagtail/wagtailadmin/tests/test_edit_handlers.py @@ -108,7 +108,21 @@ class TestGetFormForModel(TestCase): self.assertEqual(type(form.fields['date_from'].widget), forms.PasswordInput) +def clear_edit_handler(page_cls): + def decorator(fn): + def decorated(self): + # Clear any old EditHandlers generated + page_cls.get_edit_handler.cache_clear() + fn(self) + # Clear the bad EditHandler generated just now + page_cls.get_edit_handler.cache_clear() + return decorated + return decorator + + class TestPageEditHandlers(TestCase): + + @clear_edit_handler(EventPage) def test_get_edit_handler(self): """ Forms for pages should have a base class of WagtailAdminPageForm. @@ -119,6 +133,7 @@ class TestPageEditHandlers(TestCase): # The generated form should inherit from WagtailAdminPageForm self.assertTrue(issubclass(EventPageForm, WagtailAdminPageForm)) + @clear_edit_handler(ValidatedPage) def test_get_form_for_page_with_custom_base(self): """ ValidatedPage sets a custom base_form_class. This should be used as the @@ -131,26 +146,40 @@ class TestPageEditHandlers(TestCase): # ValidatedPage.base_form_class == ValidatedPageForm self.assertTrue(issubclass(GeneratedValidatedPageForm, ValidatedPageForm)) + @clear_edit_handler(ValidatedPage) def test_check_invalid_base_form_class(self): class BadFormClass(object): pass - # Clear any old EditHandlers generated - ValidatedPage.get_edit_handler.cache_clear() + invalid_base_form = checks.Error( + "ValidatedPage.base_form_class does not extend WagtailAdminPageForm", + hint="Ensure that wagtail.wagtailadmin.tests.test_edit_handlers.BadFormClass extends WagtailAdminPageForm", + obj=ValidatedPage, + id='wagtailcore.E002') + + invalid_edit_handler = checks.Error( + "ValidatedPage.get_edit_handler().get_form_class(ValidatedPage) does not extend WagtailAdminPageForm", + hint="Ensure that the EditHandler for ValidatedPage creates a subclass of WagtailAdminPageForm", + obj=ValidatedPage, + id='wagtailcore.E003') with mock.patch.object(ValidatedPage, 'base_form_class', new=BadFormClass): errors = ValidatedPage.check() - self.assertEqual(len(errors), 1) + self.assertEqual(errors, [invalid_base_form, invalid_edit_handler]) - error = errors[0] - self.assertEqual(error, checks.Error( - "base_form_class does not extend WagtailAdminPageForm", - hint="Ensure that wagtail.wagtailadmin.tests.test_edit_handlers.BadFormClass extends WagtailAdminPageForm", - obj=ValidatedPage, - id='wagtailcore.E002')) - - # Clear the bad EditHandler generated just now - ValidatedPage.get_edit_handler.cache_clear() + @clear_edit_handler(ValidatedPage) + def test_custom_edit_handler_form_class(self): + """ + Set a custom edit handler on a Page class, but dont customise + ValidatedPage.base_form_class, or provide a custom form class for the + edit handler. Check the generated form class is of the correct type. + """ + ValidatedPage.edit_handler = TabbedInterface([]) + with mock.patch.object(ValidatedPage, 'edit_handler', new=TabbedInterface([]), create=True): + form_class = ValidatedPage.get_edit_handler().get_form_class(ValidatedPage) + self.assertTrue(issubclass(form_class, WagtailAdminPageForm)) + errors = ValidatedPage.check() + self.assertEqual(errors, []) class TestExtractPanelDefinitionsFromModelClass(TestCase): diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index d4f27a346b..74a841851d 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -537,15 +537,23 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed from wagtail.wagtailadmin.forms import WagtailAdminPageForm if not issubclass(cls.base_form_class, WagtailAdminPageForm): errors.append(checks.Error( - "base_form_class does not extend WagtailAdminPageForm", + "{}.base_form_class does not extend WagtailAdminPageForm".format( + cls.__name__), hint="Ensure that {}.{} extends WagtailAdminPageForm".format( cls.base_form_class.__module__, cls.base_form_class.__name__), obj=cls, id='wagtailcore.E002')) - # Sadly, there is no way of checking the form class returned from - # cls.get_edit_handler().get_form_class(cls), as these calls can hit - # the DB in order to fetch content types. + + edit_handler = cls.get_edit_handler() + if not issubclass(edit_handler.get_form_class(cls), WagtailAdminPageForm): + errors.append(checks.Error( + "{cls}.get_edit_handler().get_form_class({cls}) does not extend WagtailAdminPageForm".format( + cls=cls.__name__), + hint="Ensure that the EditHandler for {cls} creates a subclass of WagtailAdminPageForm".format( + cls=cls.__name__), + obj=cls, + id='wagtailcore.E003')) return errors