mirrors/wagtail
mirrors/wagtail
0
0
Fork 0
mirror of https://github.com/wagtail/wagtail.git synced 2024-12-01 11:41:20 +01:00
Code Issues Releases Wiki Activity

Do not use ContentTypes in PageChooserPanel

Browse Source 
Using ContentTypes can lead to the awkward case where they are used
before the database is migrated, leading to errors on start up. Removing
ContentTypes and using plain Model classes instead makes this
impossible, and also makes the code clearer.

Fixes #2433
This commit is contained in:
Tim Heap 2016-04-04 12:26:15 +10:00 committed by Karl Hobley
parent a48ae09ce9
commit 09878fcfd8
4 changed files with 159 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
wagtail/wagtailadmin/edit_handlers.py
View File

@ -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):

56
wagtail/wagtailadmin/tests/test_edit_handlers.py
View File

@ -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):

42
wagtail/wagtailadmin/tests/test_widgets.py
View File

@ -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)

64
wagtail/wagtailadmin/widgets.py
View File

@ -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')