From 121c01c7f7db6087a985fa8dc9957bc78b9f6a6a Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 3 Feb 2015 23:00:08 +0000 Subject: [PATCH] Refactor edit handler classes so that they can have a persistent reference to their corresponding model. Previously, TabbedInterface, ObjectList, FieldPanel et al were factory functions that yield a class that has no knowledge of their corresponding models. These are now classes in their own right, meaning that when we place them in a content_panels definition, we end up with a list of 'panel definition' objects rather than a list of EditHandler classes. These panel definition objects can be 'traded in' for a proper EditHandler class by calling bind_to_model(model) - this means that our EditHandler classes are now aware of the model they are attached to. Since bind_to_model(model) returns a fresh class each time it is called, the panel definition objects can be shared between models with no issues - so we can still make use of common definitions like wagtaildemo's LinkFields.panels. --- wagtail/wagtailadmin/edit_handlers.py | 146 ++++++++++++------ .../wagtailadmin/tests/test_edit_handlers.py | 31 ++-- wagtail/wagtailadmin/views/pages.py | 2 +- wagtail/wagtaildocs/edit_handlers.py | 13 +- wagtail/wagtailimages/edit_handlers.py | 13 +- wagtail/wagtailredirects/views.py | 2 +- wagtail/wagtailsnippets/edit_handlers.py | 18 ++- wagtail/wagtailsnippets/views/snippets.py | 2 +- 8 files changed, 148 insertions(+), 79 deletions(-) diff --git a/wagtail/wagtailadmin/edit_handlers.py b/wagtail/wagtailadmin/edit_handlers.py index 8e2e5a8d12..8b7ef31db9 100644 --- a/wagtail/wagtailadmin/edit_handlers.py +++ b/wagtail/wagtailadmin/edit_handlers.py @@ -301,31 +301,51 @@ class BaseTabbedInterface(BaseCompositeEditHandler): template = "wagtailadmin/edit_handlers/tabbed_interface.html" -def TabbedInterface(children): - return type(str('_TabbedInterface'), (BaseTabbedInterface,), {'children': children}) +class TabbedInterface(object): + def __init__(self, children): + self.children = children + + def bind_to_model(self, model): + return type(str('_TabbedInterface'), (BaseTabbedInterface,), { + 'model': model, + 'children': [child.bind_to_model(model) for child in self.children], + }) class BaseObjectList(BaseCompositeEditHandler): template = "wagtailadmin/edit_handlers/object_list.html" -def ObjectList(children, heading="", classname=""): - return type(str('_ObjectList'), (BaseObjectList,), { - 'children': children, - 'heading': heading, - 'classname': classname - }) +class ObjectList(object): + def __init__(self, children, heading="", classname=""): + self.children = children + self.heading = heading + self.classname = classname + + def bind_to_model(self, model): + return type(str('_ObjectList'), (BaseObjectList,), { + 'model': model, + 'children': [child.bind_to_model(model) for child in self.children], + 'heading': self.heading, + 'classname': self.classname, + }) class BaseFieldRowPanel(BaseCompositeEditHandler): template = "wagtailadmin/edit_handlers/field_row_panel.html" -def FieldRowPanel(children, classname=""): - return type(str('_FieldRowPanel'), (BaseFieldRowPanel,), { - 'children': children, - 'classname': classname, - }) +class FieldRowPanel(object): + def __init__(self, children, classname=""): + self.children = children + self.classname = classname + + def bind_to_model(self, model): + return type(str('_FieldRowPanel'), (BaseFieldRowPanel,), { + 'model': model, + 'children': [child.bind_to_model(model) for child in self.children], + 'classname': self.classname, + }) class BaseMultiFieldPanel(BaseCompositeEditHandler): @@ -338,12 +358,19 @@ class BaseMultiFieldPanel(BaseCompositeEditHandler): return classes -def MultiFieldPanel(children, heading="", classname=""): - return type(str('_MultiFieldPanel'), (BaseMultiFieldPanel,), { - 'children': children, - 'heading': heading, - 'classname': classname, - }) +class MultiFieldPanel(object): + def __init__(self, children, heading="", classname=""): + self.children = children + self.heading = heading + self.classname = classname + + def bind_to_model(self, model): + return type(str('_MultiFieldPanel'), (BaseMultiFieldPanel,), { + 'model': model, + 'children': [child.bind_to_model(model) for child in self.children], + 'heading': self.heading, + 'classname': self.classname, + }) class BaseFieldPanel(EditHandler): @@ -402,26 +429,38 @@ class BaseFieldPanel(EditHandler): return [self.field_name] -def FieldPanel(field_name, classname="", widget=None): - base = { - 'field_name': field_name, - 'classname': classname, - } +class FieldPanel(object): + def __init__(self, field_name, classname="", widget=None): + self.field_name = field_name + self.classname = classname + self.widget = widget - if widget: - base['widget'] = widget + def bind_to_model(self, model): + base = { + 'model': model, + 'field_name': self.field_name, + 'classname': self.classname, + } - return type(str('_FieldPanel'), (BaseFieldPanel,), base) + if self.widget: + base['widget'] = self.widget + + return type(str('_FieldPanel'), (BaseFieldPanel,), base) class BaseRichTextFieldPanel(BaseFieldPanel): pass -def RichTextFieldPanel(field_name): - return type(str('_RichTextFieldPanel'), (BaseRichTextFieldPanel,), { - 'field_name': field_name, - }) +class RichTextFieldPanel(object): + def __init__(self, field_name): + self.field_name = field_name + + def bind_to_model(self, model): + return type(str('_RichTextFieldPanel'), (BaseRichTextFieldPanel,), { + 'model': model, + 'field_name': self.field_name, + }) class BaseChooserPanel(BaseFieldPanel): @@ -497,11 +536,17 @@ class BasePageChooserPanel(BaseChooserPanel): return super(BasePageChooserPanel, self).render_as_field(show_help_text, context) -def PageChooserPanel(field_name, page_type=None): - return type(str('_PageChooserPanel'), (BasePageChooserPanel,), { - 'field_name': field_name, - 'page_type': page_type, - }) +class PageChooserPanel(object): + def __init__(self, field_name, page_type=None): + self.field_name = field_name + self.page_type = page_type + + def bind_to_model(self, model): + return type(str('_PageChooserPanel'), (BasePageChooserPanel,), { + 'model': model, + 'field_name': self.field_name, + 'page_type': self.page_type, + }) class BaseInlinePanel(EditHandler): @@ -520,7 +565,7 @@ class BaseInlinePanel(EditHandler): def get_child_edit_handler_class(cls): if cls._child_edit_handler_class is None: panels = cls.get_panel_definitions() - cls._child_edit_handler_class = MultiFieldPanel(panels, heading=cls.heading) + cls._child_edit_handler_class = MultiFieldPanel(panels, heading=cls.heading).bind_to_model(cls.related.model) return cls._child_edit_handler_class @@ -586,15 +631,24 @@ class BaseInlinePanel(EditHandler): })) -def InlinePanel(base_model, relation_name, panels=None, label='', help_text=''): - rel = getattr(base_model, relation_name).related - return type(str('_InlinePanel'), (BaseInlinePanel,), { - 'relation_name': relation_name, - 'related': rel, - 'panels': panels, - 'heading': label, - 'help_text': help_text, # TODO: can we pick this out of the foreign key definition as an alternative? (with a bit of help from the inlineformset object, as we do for label/heading) - }) +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 + # bind_to_model instead + self.relation_name = relation_name + self.panels = panels + self.label = label + self.help_text = help_text + + def bind_to_model(self, model): + return type(str('_InlinePanel'), (BaseInlinePanel,), { + 'model': model, + 'relation_name': self.relation_name, + 'related': getattr(model, self.relation_name).related, + 'panels': self.panels, + 'heading': self.label, + 'help_text': self.help_text, # TODO: can we pick this out of the foreign key definition as an alternative? (with a bit of help from the inlineformset object, as we do for label/heading) + }) # This allows users to include the publishing panel in their own per-model override diff --git a/wagtail/wagtailadmin/tests/test_edit_handlers.py b/wagtail/wagtailadmin/tests/test_edit_handlers.py index 1b01065a5a..fe3d1734be 100644 --- a/wagtail/wagtailadmin/tests/test_edit_handlers.py +++ b/wagtail/wagtailadmin/tests/test_edit_handlers.py @@ -7,9 +7,8 @@ from django import forms from wagtail.wagtailadmin.edit_handlers import ( get_form_for_model, extract_panel_definitions_from_model_class, - BaseFieldPanel, FieldPanel, - BaseRichTextFieldPanel, + RichTextFieldPanel, TabbedInterface, ObjectList, PageChooserPanel, @@ -17,7 +16,7 @@ from wagtail.wagtailadmin.edit_handlers import ( ) from wagtail.wagtailadmin.widgets import AdminPageChooser, AdminDateInput -from wagtail.wagtailimages.edit_handlers import BaseImageChooserPanel, ImageChooserPanel +from wagtail.wagtailimages.edit_handlers import ImageChooserPanel from wagtail.wagtailcore.models import Page, Site from wagtail.tests.models import PageChooserModel, EventPage, EventPageSpeaker @@ -91,7 +90,7 @@ class TestExtractPanelDefinitionsFromModelClass(TestCase): result = extract_panel_definitions_from_model_class(EventPageSpeaker) self.assertEqual(len(result), 4) #print repr(result) - self.assertTrue(any([issubclass(panel, BaseImageChooserPanel) for panel in result])) + self.assertTrue(any([isinstance(panel, ImageChooserPanel) for panel in result])) def test_exclude(self): panels = extract_panel_definitions_from_model_class(Site, exclude=['hostname']) @@ -103,19 +102,19 @@ class TestExtractPanelDefinitionsFromModelClass(TestCase): panels = extract_panel_definitions_from_model_class(EventPage) self.assertTrue(any([ - issubclass(panel, BaseFieldPanel) and panel.field_name == 'date_from' + isinstance(panel, FieldPanel) and panel.field_name == 'date_from' for panel in panels ])) # returned panel types should respect modelfield.get_panel() - used on RichTextField self.assertTrue(any([ - issubclass(panel, BaseRichTextFieldPanel) and panel.field_name == 'body' + isinstance(panel, RichTextFieldPanel) and panel.field_name == 'body' for panel in panels ])) # treebeard fields should be excluded self.assertFalse(any([ - issubclass(panel, BaseFieldPanel) and panel.field_name == 'path' + panel.field_name == 'path' for panel in panels ])) @@ -132,7 +131,7 @@ class TestTabbedInterface(TestCase): ObjectList([ InlinePanel(EventPage, 'speakers', label="Speakers"), ], heading='Speakers'), - ]) + ]).bind_to_model(EventPage) def test_get_form_class(self): EventPageForm = self.EventPageTabbedInterface.get_form_class(EventPage) @@ -211,7 +210,7 @@ class TestObjectList(TestCase): FieldPanel('date_from'), FieldPanel('date_to'), InlinePanel(EventPage, 'speakers', label="Speakers"), - ], heading='Event details', classname="shiny") + ], heading='Event details', classname="shiny").bind_to_model(EventPage) def test_get_form_class(self): EventPageForm = self.EventPageObjectList.get_form_class(EventPage) @@ -254,7 +253,7 @@ class TestFieldPanel(TestCase): self.event = EventPage(title='Abergavenny sheepdog trials', date_from=date(2014, 7, 20), date_to=date(2014, 7, 21)) - self.EndDatePanel = FieldPanel('date_to', classname='full-width') + self.EndDatePanel = FieldPanel('date_to', classname='full-width').bind_to_model(EventPage) def test_render_as_object(self): form = self.EventPageForm( @@ -343,7 +342,7 @@ class TestPageChooserPanel(TestCase): model = PageChooserModel # a model with a foreign key to Page which we want to render as a page chooser # a PageChooserPanel class that works on PageChooserModel's 'page' field - self.MyPageChooserPanel = PageChooserPanel('page', 'tests.EventPage') + self.MyPageChooserPanel = PageChooserPanel('page', 'tests.EventPage').bind_to_model(PageChooserModel) # build a form class containing the fields that MyPageChooserPanel wants self.PageChooserForm = self.MyPageChooserPanel.get_form_class(PageChooserModel) @@ -388,14 +387,14 @@ class TestPageChooserPanel(TestCase): result = PageChooserPanel( 'barbecue', 'wagtailcore.site' - ).target_content_type() + ).bind_to_model(PageChooserModel).target_content_type() self.assertEqual(result.name, 'site') def test_target_content_type_malformed_type(self): result = PageChooserPanel( 'barbecue', 'snowman' - ) + ).bind_to_model(PageChooserModel) self.assertRaises(ImproperlyConfigured, result.target_content_type) @@ -403,7 +402,7 @@ class TestPageChooserPanel(TestCase): result = PageChooserPanel( 'barbecue', 'snowman.lorry' - ) + ).bind_to_model(PageChooserModel) self.assertRaises(ImproperlyConfigured, result.target_content_type) @@ -416,7 +415,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") + SpeakerInlinePanel = InlinePanel(EventPage, 'speakers', label="Speakers").bind_to_model(EventPage) EventPageForm = SpeakerInlinePanel.get_form_class(EventPage) # SpeakerInlinePanel should instruct the form class to include a 'speakers' formset @@ -453,7 +452,7 @@ class TestInlinePanel(TestCase): SpeakerInlinePanel = InlinePanel(EventPage, 'speakers', label="Speakers", panels=[ FieldPanel('first_name'), ImageChooserPanel('image'), - ]) + ]).bind_to_model(EventPage) EventPageForm = SpeakerInlinePanel.get_form_class(EventPage) # SpeakerInlinePanel should instruct the form class to include a 'speakers' formset diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index 591684031a..b5e05ac4f7 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -712,7 +712,7 @@ def get_page_edit_handler(page_class): ObjectList(page_class.content_panels, heading='Content'), ObjectList(page_class.promote_panels, heading='Promote'), ObjectList(page_class.settings_panels, heading='Settings', classname="settings") - ]) + ]).bind_to_model(page_class) return PAGE_EDIT_HANDLERS[page_class] diff --git a/wagtail/wagtaildocs/edit_handlers.py b/wagtail/wagtaildocs/edit_handlers.py index 03c48f412a..70f5628476 100644 --- a/wagtail/wagtaildocs/edit_handlers.py +++ b/wagtail/wagtaildocs/edit_handlers.py @@ -13,7 +13,12 @@ class BaseDocumentChooserPanel(BaseChooserPanel): return {cls.field_name: AdminDocumentChooser} -def DocumentChooserPanel(field_name): - return type(str('_DocumentChooserPanel'), (BaseDocumentChooserPanel,), { - 'field_name': field_name, - }) +class DocumentChooserPanel(object): + def __init__(self, field_name): + self.field_name = field_name + + def bind_to_model(self, model): + return type(str('_DocumentChooserPanel'), (BaseDocumentChooserPanel,), { + 'model': model, + 'field_name': self.field_name, + }) diff --git a/wagtail/wagtailimages/edit_handlers.py b/wagtail/wagtailimages/edit_handlers.py index 059355c082..687d3b64ad 100644 --- a/wagtail/wagtailimages/edit_handlers.py +++ b/wagtail/wagtailimages/edit_handlers.py @@ -13,7 +13,12 @@ class BaseImageChooserPanel(BaseChooserPanel): return {cls.field_name: AdminImageChooser} -def ImageChooserPanel(field_name): - return type(str('_ImageChooserPanel'), (BaseImageChooserPanel,), { - 'field_name': field_name, - }) +class ImageChooserPanel(object): + def __init__(self, field_name): + self.field_name = field_name + + def bind_to_model(self, model): + return type(str('_ImageChooserPanel'), (BaseImageChooserPanel,), { + 'model': model, + 'field_name': self.field_name, + }) diff --git a/wagtail/wagtailredirects/views.py b/wagtail/wagtailredirects/views.py index 98e2668db8..f065bb83eb 100644 --- a/wagtail/wagtailredirects/views.py +++ b/wagtail/wagtailredirects/views.py @@ -12,7 +12,7 @@ from wagtail.wagtailadmin import messages from wagtail.wagtailredirects import models -REDIRECT_EDIT_HANDLER = ObjectList(models.Redirect.content_panels) +REDIRECT_EDIT_HANDLER = ObjectList(models.Redirect.content_panels).bind_to_model(models.Redirect) @permission_required('wagtailredirects.change_redirect') diff --git a/wagtail/wagtailsnippets/edit_handlers.py b/wagtail/wagtailsnippets/edit_handlers.py index 3aa3845b5e..2e230715b4 100644 --- a/wagtail/wagtailsnippets/edit_handlers.py +++ b/wagtail/wagtailsnippets/edit_handlers.py @@ -39,9 +39,15 @@ class BaseSnippetChooserPanel(BaseChooserPanel): })) -def SnippetChooserPanel(field_name, snippet_type): - return type(str('_SnippetChooserPanel'), (BaseSnippetChooserPanel,), { - 'field_name': field_name, - 'snippet_type_name': force_text(snippet_type._meta.verbose_name), - 'snippet_type': snippet_type, - }) +class SnippetChooserPanel(object): + def __init__(self, field_name, snippet_type): + self.field_name = field_name + self.snippet_type = snippet_type + + def bind_to_model(self, model): + return type(str('_SnippetChooserPanel'), (BaseSnippetChooserPanel,), { + 'model': model, + 'field_name': self.field_name, + 'snippet_type_name': force_text(self.snippet_type._meta.verbose_name), + 'snippet_type': self.snippet_type, + }) diff --git a/wagtail/wagtailsnippets/views/snippets.py b/wagtail/wagtailsnippets/views/snippets.py index 565e1f40f2..971aacc7e0 100644 --- a/wagtail/wagtailsnippets/views/snippets.py +++ b/wagtail/wagtailsnippets/views/snippets.py @@ -59,7 +59,7 @@ SNIPPET_EDIT_HANDLERS = {} def get_snippet_edit_handler(model): if model not in SNIPPET_EDIT_HANDLERS: panels = extract_panel_definitions_from_model_class(model) - edit_handler = ObjectList(panels) + edit_handler = ObjectList(panels).bind_to_model(model) SNIPPET_EDIT_HANDLERS[model] = edit_handler