From 547c29faf5d5883ddb19eff351d2121167f88279 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Wed, 4 Feb 2015 15:46:32 +0000 Subject: [PATCH] Deprecate the base_model parameter on InlinePanel - fixes #405 --- docs/core_components/form_builder.rst | 2 +- docs/core_components/pages/editing_api.rst | 12 +++-- docs/core_components/snippets.rst | 2 +- runtests.py | 1 + wagtail/tests/models.py | 10 ++-- wagtail/tests/utils.py | 16 +++++++ wagtail/wagtailadmin/edit_handlers.py | 30 +++++++++--- .../wagtailadmin/tests/test_edit_handlers.py | 46 +++++++++++++++++-- 8 files changed, 97 insertions(+), 22 deletions(-) diff --git a/docs/core_components/form_builder.rst b/docs/core_components/form_builder.rst index 65ed639bf7..a98284d35b 100644 --- a/docs/core_components/form_builder.rst +++ b/docs/core_components/form_builder.rst @@ -36,7 +36,7 @@ Within the models.py of one of your apps, create a model that extends wagtailfor FormPage.content_panels = [ FieldPanel('title', classname="full title"), FieldPanel('intro', classname="full"), - InlinePanel(FormPage, 'form_fields', label="Form fields"), + InlinePanel('form_fields', label="Form fields"), FieldPanel('thank_you_text', classname="full"), MultiFieldPanel([ FieldPanel('to_address', classname="full"), diff --git a/docs/core_components/pages/editing_api.rst b/docs/core_components/pages/editing_api.rst index 2087f8c879..7cf4a7054c 100644 --- a/docs/core_components/pages/editing_api.rst +++ b/docs/core_components/pages/editing_api.rst @@ -31,7 +31,7 @@ There are four basic types of panels: ``MultiFieldPanel( children, heading="", classname=None )`` This panel condenses several ``FieldPanel`` s or choosers, from a list or tuple, under a single ``heading`` string. - ``InlinePanel( base_model, relation_name, panels=None, classname=None, label='', help_text='' )`` + ``InlinePanel( relation_name, panels=None, classname=None, label='', help_text='' )`` This panel allows for the creation of a "cluster" of related objects over a join to a separate model, such as a list of related links or slides to an image carousel. This is a very powerful, but tricky feature which will take some space to cover, so we'll skip over it for now. For a full explanation on the usage of ``InlinePanel``, see :ref:`inline_panels`. ``FieldRowPanel( children, classname=None)`` @@ -354,16 +354,20 @@ Let's look at the example of adding related links to a ``Page``-derived model. W BookPage.content_panels = [ # ... - InlinePanel( BookPage, 'related_links', label="Related Links" ), + InlinePanel( 'related_links', label="Related Links" ), ] The ``RelatedLink`` class is a vanilla Django abstract model. The ``BookPageRelatedLinks`` model extends it with capability for being ordered in the Wagtail interface via the ``Orderable`` class as well as adding a ``page`` property which links the model to the ``BookPage`` model we're adding the related links objects to. Finally, in the panel definitions for ``BookPage``, we'll add an ``InlinePanel`` to provide an interface for it all. Let's look again at the parameters that ``InlinePanel`` accepts: .. code-block:: python - InlinePanel( base_model, relation_name, panels=None, label='', help_text='' ) + InlinePanel( relation_name, panels=None, label='', help_text='' ) -``base_model`` is the model you're extending with the cluster. The ``relation_name`` is the ``related_name`` label given to the cluster's ``ParentalKey`` relation. You can add the ``panels`` manually or make them part of the cluster model. Finally, ``label`` and ``help_text`` provide a heading and caption, respectively, for the Wagtail editor. +The ``relation_name`` is the ``related_name`` label given to the cluster's ``ParentalKey`` relation. You can add the ``panels`` manually or make them part of the cluster model. Finally, ``label`` and ``help_text`` provide a heading and caption, respectively, for the Wagtail editor. + +.. versionchanged:: 0.9 + + In previous versions, it was necessary to pass the base model as the first parameter to ``InlinePanel``; this is no longer required. For another example of using model clusters, see :ref:`tagging` diff --git a/docs/core_components/snippets.rst b/docs/core_components/snippets.rst index 33d20bdc4f..e323bdb689 100644 --- a/docs/core_components/snippets.rst +++ b/docs/core_components/snippets.rst @@ -151,7 +151,7 @@ To attach multiple adverts to a page, the ``SnippetChooserPanel`` can be placed ... BookPage.content_panels = [ - InlinePanel(BookPage, 'advert_placements', label="Adverts"), + InlinePanel('advert_placements', label="Adverts"), # ... ] diff --git a/runtests.py b/runtests.py index 7961d2e192..a1925f907d 100755 --- a/runtests.py +++ b/runtests.py @@ -15,6 +15,7 @@ os.environ['DJANGO_SETTINGS_MODULE'] = 'wagtail.tests.settings' def runtests(): # Don't ignore DeprecationWarnings warnings.simplefilter('default', DeprecationWarning) + warnings.simplefilter('default', PendingDeprecationWarning) argv = sys.argv[:1] + ['test'] + sys.argv[1:] try: diff --git a/wagtail/tests/models.py b/wagtail/tests/models.py index 2b20d7a7cb..c419de7c74 100644 --- a/wagtail/tests/models.py +++ b/wagtail/tests/models.py @@ -247,10 +247,10 @@ EventPage.content_panels = [ FieldPanel('audience'), FieldPanel('cost'), FieldPanel('signup_link'), - InlinePanel(EventPage, 'carousel_items', label="Carousel items"), + InlinePanel('carousel_items', label="Carousel items"), FieldPanel('body', classname="full"), - InlinePanel(EventPage, 'speakers', label="Speakers"), - InlinePanel(EventPage, 'related_links', label="Related links"), + InlinePanel('speakers', label="Speakers"), + InlinePanel('related_links', label="Related links"), ] EventPage.promote_panels = [ @@ -329,7 +329,7 @@ class FormPage(AbstractEmailForm): FormPage.content_panels = [ FieldPanel('title', classname="full title"), - InlinePanel(FormPage, 'form_fields', label="Form fields"), + InlinePanel('form_fields', label="Form fields"), MultiFieldPanel([ FieldPanel('to_address', classname="full"), FieldPanel('from_address', classname="full"), @@ -392,7 +392,7 @@ class StandardIndex(Page): StandardIndex.content_panels = [ FieldPanel('title', classname="full title"), - InlinePanel(StandardIndex, 'advert_placements', label="Adverts"), + InlinePanel('advert_placements', label="Adverts"), ] diff --git a/wagtail/tests/utils.py b/wagtail/tests/utils.py index fd155e6b45..f2993df6f3 100644 --- a/wagtail/tests/utils.py +++ b/wagtail/tests/utils.py @@ -1,5 +1,6 @@ from contextlib import contextmanager import warnings +import sys from django.contrib.auth import get_user_model from django.utils import six @@ -28,3 +29,18 @@ class WagtailTestUtils(object): for w in warning_list: if not issubclass(w.category, DeprecationWarning): warnings.showwarning(message=w.message, category=w.category, filename=w.filename, lineno=w.lineno, file=w.file, line=w.line) + + # borrowed from https://github.com/django/django/commit/9f427617e4559012e1c2fd8fce46cbe225d8515d + @staticmethod + def reset_warning_registry(): + """ + Clear warning registry for all modules. This is required in some tests + because of a bug in Python that prevents warnings.simplefilter("always") + from always making warnings appear: http://bugs.python.org/issue4180 + + The bug was fixed in Python 3.4.2. + """ + key = "__warningregistry__" + for mod in sys.modules.values(): + if hasattr(mod, key): + getattr(mod, key).clear() diff --git a/wagtail/wagtailadmin/edit_handlers.py b/wagtail/wagtailadmin/edit_handlers.py index 93c1e09f9f..47a6b666db 100644 --- a/wagtail/wagtailadmin/edit_handlers.py +++ b/wagtail/wagtailadmin/edit_handlers.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals import copy +import warnings from six import text_type @@ -21,6 +22,7 @@ from taggit.managers import TaggableManager from wagtail.wagtailadmin import widgets from wagtail.wagtailcore.models import Page from wagtail.wagtailcore.utils import camelcase_to_underscore, resolve_model_string +from wagtail.utils.deprecation import RemovedInWagtail11Warning FORM_FIELD_OVERRIDES = { @@ -640,13 +642,29 @@ class BaseInlinePanel(EditHandler): class InlinePanel(object): - def __init__(self, base_model, relation_name, panels=None, label='', help_text=''): - # the base_model param is now redundant; we set up relations based on the model passed to + def __init__(self, *args, **kwargs): + # prior to Wagtail 0.9, InlinePanel required two params, base_model and relation_name. + # base_model is no longer required; we set up relations based on the model passed to # bind_to_model instead - self.relation_name = relation_name - self.panels = panels - self.label = label - self.help_text = help_text + if len(args) == 1: # new-style: InlinePanel(relation_name) + self.relation_name = args[0] + elif len(args) == 2: # old-style: InlinePanel(base_model, relation_name) + self.relation_name = args[1] + + warnings.warn( + "InlinePanel no longer needs to be passed a model parameter. " + "InlinePanel({classname}, '{relname}') should be changed to InlinePanel('{relname}')".format( + classname=args[0].__name__, relname=self.relation_name + ), RemovedInWagtail11Warning) + else: + raise TypeError("InlinePanel() takes exactly 1 argument (%d given)" % len(args)) + + self.panels = kwargs.pop('panels', None) + self.label = kwargs.pop('label', '') + self.help_text = kwargs.pop('help_text', '') + + if kwargs: + raise TypeError("InlinePanel got an unexpected keyword argument '%s'" % kwargs.keys()[0]) def bind_to_model(self, model): return type(str('_InlinePanel'), (BaseInlinePanel,), { diff --git a/wagtail/wagtailadmin/tests/test_edit_handlers.py b/wagtail/wagtailadmin/tests/test_edit_handlers.py index 6745841b54..9b76c7f2a4 100644 --- a/wagtail/wagtailadmin/tests/test_edit_handlers.py +++ b/wagtail/wagtailadmin/tests/test_edit_handlers.py @@ -1,4 +1,5 @@ from datetime import date +import warnings from django.core.exceptions import ImproperlyConfigured from django.test import TestCase @@ -19,6 +20,8 @@ from wagtail.wagtailadmin.widgets import AdminPageChooser, AdminDateInput from wagtail.wagtailimages.edit_handlers import ImageChooserPanel from wagtail.wagtailcore.models import Page, Site from wagtail.tests.models import PageChooserModel, EventPage, EventPageSpeaker +from wagtail.tests.utils import WagtailTestUtils +from wagtail.utils.deprecation import RemovedInWagtail11Warning class TestGetFormForModel(TestCase): @@ -129,7 +132,7 @@ class TestTabbedInterface(TestCase): FieldPanel('date_to'), ], heading='Event details', classname="shiny"), ObjectList([ - InlinePanel(EventPage, 'speakers', label="Speakers"), + InlinePanel('speakers', label="Speakers"), ], heading='Speakers'), ]).bind_to_model(EventPage) @@ -200,7 +203,7 @@ class TestObjectList(TestCase): FieldPanel('title', widget=forms.Textarea), FieldPanel('date_from'), FieldPanel('date_to'), - InlinePanel(EventPage, 'speakers', label="Speakers"), + InlinePanel('speakers', label="Speakers"), ], heading='Event details', classname="shiny").bind_to_model(EventPage) def test_get_form_class(self): @@ -391,7 +394,7 @@ class TestPageChooserPanel(TestCase): result.target_content_type) -class TestInlinePanel(TestCase): +class TestInlinePanel(TestCase, WagtailTestUtils): fixtures = ['test.json'] def test_render(self): @@ -399,7 +402,7 @@ class TestInlinePanel(TestCase): Check that the inline panel renders the panels set on the model when no 'panels' parameter is passed in the InlinePanel definition """ - SpeakerInlinePanel = InlinePanel(EventPage, 'speakers', label="Speakers").bind_to_model(EventPage) + SpeakerInlinePanel = InlinePanel('speakers', label="Speakers").bind_to_model(EventPage) EventPageForm = SpeakerInlinePanel.get_form_class(EventPage) # SpeakerInlinePanel should instruct the form class to include a 'speakers' formset @@ -434,7 +437,7 @@ class TestInlinePanel(TestCase): Check that inline panel renders the panels listed in the InlinePanel definition where one is specified """ - SpeakerInlinePanel = InlinePanel(EventPage, 'speakers', label="Speakers", panels=[ + SpeakerInlinePanel = InlinePanel('speakers', label="Speakers", panels=[ FieldPanel('first_name', widget=forms.Textarea), ImageChooserPanel('image'), ]).bind_to_model(EventPage) @@ -471,3 +474,36 @@ class TestInlinePanel(TestCase): # render_js_init must provide the JS initializer self.assertIn('var panel = InlinePanel({', panel.render_js_init()) + + def test_old_style_inlinepanel_declaration(self): + """ + Check that the deprecated form of InlinePanel declaration (where the base model is passed + as the first arg) still works + """ + self.reset_warning_registry() + with warnings.catch_warnings(record=True) as w: + SpeakerInlinePanelDef = InlinePanel(EventPage, 'speakers', label="Speakers") + + # Check that a RemovedInWagtail11Warning has been triggered + self.assertEqual(len(w), 1) + self.assertTrue(issubclass(w[-1].category, RemovedInWagtail11Warning)) + self.assertTrue("InlinePanel(EventPage, 'speakers') should be changed to InlinePanel('speakers')" in str(w[-1].message)) + + SpeakerInlinePanel = SpeakerInlinePanelDef.bind_to_model(EventPage) + EventPageForm = SpeakerInlinePanel.get_form_class(EventPage) + + # SpeakerInlinePanel should instruct the form class to include a 'speakers' formset + self.assertEqual(['speakers'], list(EventPageForm.formsets.keys())) + + event_page = EventPage.objects.get(slug='christmas') + form = EventPageForm(instance=event_page) + panel = SpeakerInlinePanel(instance=event_page, form=form) + + result = panel.render_as_field() + self.assertIn('', result) + self.assertIn('value="Father"', result) + + def test_invalid_inlinepanel_declaration(self): + self.assertRaises(TypeError, lambda: InlinePanel(label="Speakers")) + self.assertRaises(TypeError, lambda: InlinePanel(EventPage, 'speakers', 'bacon', label="Speakers")) + self.assertRaises(TypeError, lambda: InlinePanel(EventPage, 'speakers', label="Speakers", bacon="chunky"))