diff --git a/wagtail/wagtailadmin/edit_handlers.py b/wagtail/wagtailadmin/edit_handlers.py index 9e64024216..eb80205234 100644 --- a/wagtail/wagtailadmin/edit_handlers.py +++ b/wagtail/wagtailadmin/edit_handlers.py @@ -1,5 +1,7 @@ from __future__ import absolute_import, unicode_literals +import warnings + import django from django import forms from django.contrib.contenttypes.models import ContentType @@ -11,6 +13,7 @@ from django.utils.six import text_type from django.utils.translation import ugettext_lazy from wagtail.utils.decorators import cached_classmethod +from wagtail.utils.deprecation import RemovedInWagtail17Warning from wagtail.wagtailadmin import widgets from wagtail.wagtailcore.models import Page from wagtail.wagtailcore.utils import camelcase_to_underscore, resolve_model_string @@ -506,41 +509,43 @@ class BaseChooserPanel(BaseFieldPanel): class BasePageChooserPanel(BaseChooserPanel): object_type_name = "page" - _target_content_type = None - @classmethod def widget_overrides(cls): return {cls.field_name: widgets.AdminPageChooser( - content_type=cls.target_content_type(), can_choose_root=cls.can_choose_root)} + target_models=cls.target_models(), + can_choose_root=cls.can_choose_root)} - @classmethod + @cached_classmethod + def target_models(cls): + if cls.page_type: + target_models = [] + + for page_type in cls.page_type: + try: + target_models.append(resolve_model_string(page_type)) + except LookupError: + raise ImproperlyConfigured( + "{0}.page_type must be of the form 'app_label.model_name', given {1!r}".format( + cls.__name__, page_type + ) + ) + except ValueError: + raise ImproperlyConfigured( + "{0}.page_type refers to model {1!r} that has not been installed".format( + cls.__name__, page_type + ) + ) + + return target_models + else: + return [cls.model._meta.get_field(cls.field_name).rel.to] + + @cached_classmethod def target_content_type(cls): - if cls._target_content_type is None: - if cls.page_type: - target_models = [] - - for page_type in cls.page_type: - try: - target_models.append(resolve_model_string(page_type)) - except LookupError: - raise ImproperlyConfigured( - "{0}.page_type must be of the form 'app_label.model_name', given {1!r}".format( - cls.__name__, page_type - ) - ) - except ValueError: - raise ImproperlyConfigured( - "{0}.page_type refers to model {1!r} that has not been installed".format( - cls.__name__, page_type - ) - ) - - cls._target_content_type = list(ContentType.objects.get_for_models(*target_models).values()) - else: - target_model = cls.model._meta.get_field(cls.field_name).rel.to - cls._target_content_type = [ContentType.objects.get_for_model(target_model)] - - return cls._target_content_type + warnings.warn( + '{cls}.target_content_type() is deprecated. Use {cls}.target_models() instead'.format(cls=cls.__name__), + category=RemovedInWagtail17Warning) + return list(ContentType.objects.get_for_models(*cls.target_models()).values()) class PageChooserPanel(object): diff --git a/wagtail/wagtailadmin/tests/test_edit_handlers.py b/wagtail/wagtailadmin/tests/test_edit_handlers.py index e5de7f3b4a..a2b59fdc28 100644 --- a/wagtail/wagtailadmin/tests/test_edit_handlers.py +++ b/wagtail/wagtailadmin/tests/test_edit_handlers.py @@ -1,5 +1,6 @@ from __future__ import absolute_import, unicode_literals +import warnings from datetime import date import mock @@ -12,6 +13,7 @@ from wagtail.tests.testapp.forms import ValidatedPageForm from wagtail.tests.testapp.models import ( EventPage, EventPageChooserModel, EventPageSpeaker, PageChooserModel, SimplePage, ValidatedPage) from wagtail.tests.utils import WagtailTestUtils +from wagtail.utils.deprecation import RemovedInWagtail17Warning from wagtail.wagtailadmin.edit_handlers import ( FieldPanel, InlinePanel, ObjectList, PageChooserPanel, RichTextFieldPanel, TabbedInterface, extract_panel_definitions_from_model_class, get_form_for_model) @@ -528,28 +530,68 @@ class TestPageChooserPanel(TestCase): self.assertIn(expected_js, result) - def test_target_content_type(self): + def test_target_models(self): result = PageChooserPanel( 'barbecue', 'wagtailcore.site' - ).bind_to_model(PageChooserModel).target_content_type()[0] - self.assertEqual(result.name, 'site') + ).bind_to_model(PageChooserModel).target_models() + self.assertEqual(result, [Site]) - def test_target_content_type_malformed_type(self): + def test_target_models_malformed_type(self): result = PageChooserPanel( 'barbecue', 'snowman' ).bind_to_model(PageChooserModel) self.assertRaises(ImproperlyConfigured, - result.target_content_type) + result.target_models) - def test_target_content_type_nonexistent_type(self): + def test_target_models_nonexistent_type(self): result = PageChooserPanel( 'barbecue', 'snowman.lorry' ).bind_to_model(PageChooserModel) self.assertRaises(ImproperlyConfigured, - result.target_content_type) + result.target_models) + + def test_target_content_type(self): + with warnings.catch_warnings(record=True) as ws: + warnings.simplefilter('always') + + result = PageChooserPanel( + 'barbecue', + 'wagtailcore.site' + ).bind_to_model(PageChooserModel).target_content_type()[0] + self.assertEqual(result.name, 'site') + + self.assertEqual(len(ws), 1) + self.assertIs(ws[0].category, RemovedInWagtail17Warning) + + def test_target_content_type_malformed_type(self): + with warnings.catch_warnings(record=True) as ws: + warnings.simplefilter('always') + + result = PageChooserPanel( + 'barbecue', + 'snowman' + ).bind_to_model(PageChooserModel) + self.assertRaises(ImproperlyConfigured, + result.target_content_type) + + self.assertEqual(len(ws), 1) + self.assertIs(ws[0].category, RemovedInWagtail17Warning) + + def test_target_content_type_nonexistent_type(self): + with warnings.catch_warnings(record=True) as ws: + warnings.simplefilter('always') + + result = PageChooserPanel( + 'barbecue', + 'snowman.lorry' + ).bind_to_model(PageChooserModel) + self.assertRaises(ImproperlyConfigured, + result.target_content_type) + self.assertEqual(len(ws), 1) + self.assertIs(ws[0].category, RemovedInWagtail17Warning) class TestInlinePanel(TestCase, WagtailTestUtils): diff --git a/wagtail/wagtailadmin/tests/test_widgets.py b/wagtail/wagtailadmin/tests/test_widgets.py index 1fcba82bb0..7aee1ab0f7 100644 --- a/wagtail/wagtailadmin/tests/test_widgets.py +++ b/wagtail/wagtailadmin/tests/test_widgets.py @@ -1,9 +1,12 @@ from __future__ import absolute_import, unicode_literals +import warnings + from django.contrib.contenttypes.models import ContentType from django.test import TestCase from wagtail.tests.testapp.models import EventPage, SimplePage +from wagtail.utils.deprecation import RemovedInWagtail17Warning from wagtail.wagtailadmin import widgets from wagtail.wagtailcore.models import Page @@ -52,26 +55,45 @@ class TestAdminPageChooserWidget(TestCase): # def test_render_html_init_with_content_type omitted as HTML does not # change when selecting a content type - def test_render_js_init_with_content_type(self): - content_type = ContentType.objects.get_for_model(SimplePage) - widget = widgets.AdminPageChooser(content_type=content_type) + def test_render_js_init_with_target_model(self): + widget = widgets.AdminPageChooser(target_models=[SimplePage]) js_init = widget.render_js_init('test-id', 'test', None) self.assertEqual(js_init, "createPageChooser(\"test-id\", [\"tests.simplepage\"], null, false);") - def test_render_js_init_with_multiple_content_types(self): - content_types = [ - # Not using get_for_models as we need deterministic ordering - ContentType.objects.get_for_model(SimplePage), - ContentType.objects.get_for_model(EventPage), - ] - widget = widgets.AdminPageChooser(content_type=content_types) + def test_render_js_init_with_multiple_target_models(self): + target_models = [SimplePage, EventPage] + widget = widgets.AdminPageChooser(target_models=target_models) js_init = widget.render_js_init('test-id', 'test', None) self.assertEqual( js_init, "createPageChooser(\"test-id\", [\"tests.simplepage\", \"tests.eventpage\"], null, false);" ) + def test_render_js_init_with_content_type(self): + with warnings.catch_warnings(record=True) as ws: + warnings.simplefilter('always') + content_type = ContentType.objects.get_for_model(SimplePage) + widget = widgets.AdminPageChooser(content_type=content_type) + + self.assertEqual(len(ws), 1) + self.assertIs(ws[0].category, RemovedInWagtail17Warning) + self.assertEqual(widget.target_models, [SimplePage]) + + def test_render_js_init_with_multiple_content_types(self): + with warnings.catch_warnings(record=True) as ws: + warnings.simplefilter('always') + content_types = [ + # Not using get_for_models as we need deterministic ordering + ContentType.objects.get_for_model(SimplePage), + ContentType.objects.get_for_model(EventPage), + ] + widget = widgets.AdminPageChooser(content_type=content_types) + + self.assertEqual(len(ws), 1) + self.assertIs(ws[0].category, RemovedInWagtail17Warning) + self.assertEqual(widget.target_models, [SimplePage, EventPage]) + def test_render_js_init_with_can_choose_root(self): widget = widgets.AdminPageChooser(can_choose_root=True) diff --git a/wagtail/wagtailadmin/widgets.py b/wagtail/wagtailadmin/widgets.py index 469ae917b0..e80f6b820d 100644 --- a/wagtail/wagtailadmin/widgets.py +++ b/wagtail/wagtailadmin/widgets.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals import itertools import json +import warnings from functools import total_ordering from django.contrib.contenttypes.models import ContentType @@ -16,6 +17,7 @@ from django.utils.html import format_html from django.utils.translation import ugettext_lazy as _ from taggit.forms import TagWidget +from wagtail.utils.deprecation import RemovedInWagtail17Warning from wagtail.utils.widgets import WidgetWithScript from wagtail.wagtailcore import hooks from wagtail.wagtailcore.models import Page @@ -135,24 +137,48 @@ class AdminPageChooser(AdminChooser): choose_another_text = _('Choose another page') link_to_chosen_text = _('Edit this page') - def __init__(self, content_type=None, can_choose_root=False, **kwargs): + def __init__(self, target_models=None, content_type=None, can_choose_root=False, **kwargs): super(AdminPageChooser, self).__init__(**kwargs) - self._content_type = content_type + + self.target_models = list(target_models or [Page]) + + if content_type is not None: + if target_models is not None: + raise ValueError("Can not set both target_models and content_type") + warnings.warn( + 'The content_type argument for AdminPageChooser() is deprecated. Use the target_models argument instead', + category=RemovedInWagtail17Warning) + if isinstance(content_type, ContentType): + self.target_models = [content_type.model_class()] + else: + self.target_models = [ct.model_class() for ct in content_type] + self.can_choose_root = can_choose_root @cached_property def target_content_types(self): - target_content_types = self._content_type or ContentType.objects.get_for_model(Page) - # Make sure target_content_types is a list or tuple - if not isinstance(target_content_types, (list, tuple)): - target_content_types = [target_content_types] - return target_content_types + warnings.warn( + 'AdminPageChooser.target_content_types is deprecated. Use AdminPageChooser.target_models instead', + category=RemovedInWagtail17Warning) + return list(ContentType.objects.get_for_models(*self.target_models).values()) + + def _get_lowest_common_page_class(self): + """ + Return a Page class that is an ancestor for all Page classes in + ``target_models``, and is also a concrete Page class itself. + """ + if len(self.target_models) == 1: + # Shortcut for a single page type + return self.target_models[0] + else: + base = self.target_models[0] + others = self.target_models[1:] + for cls in base.__mro__: + if all(cls in other.__mro__ for other in others): + return cls def render_html(self, name, value, attrs): - if len(self.target_content_types) == 1: - model_class = self.target_content_types[0].model_class() - else: - model_class = Page + model_class = self._get_lowest_common_page_class() instance, value = self.get_instance_and_id(model_class, value) @@ -171,22 +197,18 @@ class AdminPageChooser(AdminChooser): page = value else: # Value is an ID look up object - if len(self.target_content_types) == 1: - model_class = self.target_content_types[0].model_class() - else: - model_class = Page - + model_class = self._get_lowest_common_page_class() page = self.get_instance(model_class, value) parent = page.get_parent() if page else None - return "createPageChooser({id}, {content_type}, {parent}, {can_choose_root});".format( + return "createPageChooser({id}, {model_names}, {parent}, {can_choose_root});".format( id=json.dumps(id_), - content_type=json.dumps([ + model_names=json.dumps([ '{app}.{model}'.format( - app=content_type.app_label, - model=content_type.model) - for content_type in self.target_content_types + app=model._meta.app_label, + model=model._meta.model_name) + for model in self.target_models ]), parent=json.dumps(parent.id if parent else None), can_choose_root=('true' if self.can_choose_root else 'false')