From 6632feedb36a7139cab11333709559cbfa205f14 Mon Sep 17 00:00:00 2001 From: Joss Ingram Date: Thu, 4 Jun 2015 12:40:18 +0100 Subject: [PATCH 1/9] Snippet choose panel now takes model string fixes #1362 --- wagtail/wagtailsnippets/edit_handlers.py | 33 ++++++++++++++++++------ wagtail/wagtailsnippets/tests.py | 27 +++++++++++++++++++ 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/wagtail/wagtailsnippets/edit_handlers.py b/wagtail/wagtailsnippets/edit_handlers.py index 0c3f26564..22992e39a 100644 --- a/wagtail/wagtailsnippets/edit_handlers.py +++ b/wagtail/wagtailsnippets/edit_handlers.py @@ -4,28 +4,42 @@ from django.template.loader import render_to_string from django.contrib.contenttypes.models import ContentType from django.utils.safestring import mark_safe from django.utils.encoding import force_text +from django.core.exceptions import ImproperlyConfigured from wagtail.wagtailadmin.edit_handlers import BaseChooserPanel +from wagtail.wagtailcore.utils import resolve_model_string from .widgets import AdminSnippetChooser class BaseSnippetChooserPanel(BaseChooserPanel): object_type_name = 'item' - _content_type = None + _target_content_type = None @classmethod def widget_overrides(cls): return {cls.field_name: AdminSnippetChooser( - content_type=cls.content_type(), snippet_type_name=cls.snippet_type_name)} + content_type=cls.target_content_type(), snippet_type_name=cls.snippet_type_name)} @classmethod - def content_type(cls): - if cls._content_type is None: - # TODO: infer the content type by introspection on the foreign key rather than having to pass it explicitly - cls._content_type = ContentType.objects.get_for_model(cls.snippet_type) + def target_content_type(cls): + if cls._target_content_type is None: + if cls.snippet_type: + try: + model = resolve_model_string(cls.snippet_type) + except LookupError: + raise ImproperlyConfigured("{0}.snippet_type must be of the form 'app_label.model_name', given {1!r}".format( + cls.__name__, cls.snippet_type)) + except ValueError: + raise ImproperlyConfigured("{0}.snippet_type refers to model {1!r} that has not been installed".format( + cls.__name__, cls.snippet_type)) - return cls._content_type + cls._target_content_type = ContentType.objects.get_for_model(model) + 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 def render_as_field(self): instance_obj = self.get_chosen_item() @@ -35,6 +49,10 @@ class BaseSnippetChooserPanel(BaseChooserPanel): 'snippet_type_name': self.snippet_type_name, })) + @property + def snippet_type_name(self): + return force_text(self.target_content_type()._meta.verbose_name) + class SnippetChooserPanel(object): def __init__(self, field_name, snippet_type): @@ -45,6 +63,5 @@ class SnippetChooserPanel(object): 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/tests.py b/wagtail/wagtailsnippets/tests.py index 0ea534fed..2da7ba017 100644 --- a/wagtail/wagtailsnippets/tests.py +++ b/wagtail/wagtailsnippets/tests.py @@ -1,11 +1,13 @@ from django.test import TestCase from django.core.urlresolvers import reverse from django.test.utils import override_settings +from django.core.exceptions import ImproperlyConfigured from wagtail.tests.utils import WagtailTestUtils from wagtail.tests.testapp.models import Advert, SnippetChooserModel from wagtail.tests.snippets.models import AlphaSnippet, ZuluSnippet, RegisterDecorator, RegisterFunction from wagtail.wagtailsnippets.models import register_snippet, SNIPPET_MODELS +from wagtail.wagtailsnippets.edit_handlers import SnippetChooserPanel from wagtail.wagtailsnippets.views.snippets import ( get_snippet_edit_handler @@ -185,6 +187,31 @@ class TestSnippetChooserPanel(TestCase): self.assertIn('createSnippetChooser("id_advert", "tests/advert");', self.snippet_chooser_panel.render_as_field()) + def test_target_content_type(self): + result = SnippetChooserPanel( + 'barbecue', + 'wagtailcore.site' + ).bind_to_model(SnippetChooserModel).target_content_type() + self.assertEqual(result.name, 'Site') + + def test_target_content_type_malformed_type(self): + result = SnippetChooserPanel( + 'barbecue', + 'snowman' + ).bind_to_model(SnippetChooserModel) + self.assertRaises(ImproperlyConfigured, + result.target_content_type) + + def test_target_content_type_nonexistent_type(self): + result = SnippetChooserPanel( + 'barbecue', + 'snowman.lorry' + ).bind_to_model(SnippetChooserModel) + self.assertRaises(ImproperlyConfigured, + result.target_content_type) + + + class TestSnippetRegistering(TestCase): def test_register_function(self): From 028632eabebe4988b32e5635e2bed8fdede36d32 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 27 Aug 2015 17:24:06 +0100 Subject: [PATCH 2/9] PEP8 fixes --- wagtail/wagtailsnippets/tests.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/wagtail/wagtailsnippets/tests.py b/wagtail/wagtailsnippets/tests.py index ba1512f1e..92e187d86 100644 --- a/wagtail/wagtailsnippets/tests.py +++ b/wagtail/wagtailsnippets/tests.py @@ -200,7 +200,7 @@ class TestSnippetDelete(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) def test_delete_post(self): - post_data = {'foo': 'bar'} # For some reason, this test doesn't work without a bit of POST data + post_data = {'foo': 'bar'} # For some reason, this test doesn't work without a bit of POST data response = self.client.post(reverse('wagtailsnippets:delete', args=('tests', 'advert', self.test_snippet.id, )), post_data) # Should be redirected to explorer page @@ -263,8 +263,6 @@ class TestSnippetChooserPanel(TestCase): result.target_content_type) - - class TestSnippetRegistering(TestCase): def test_register_function(self): self.assertIn(RegisterFunction, SNIPPET_MODELS) From cd51efea6aa1d060754a9a4859642ba67f19d895 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 27 Aug 2015 17:31:10 +0100 Subject: [PATCH 3/9] Use a real field name and a valid snippet model in SnippetChooserPanel tests --- wagtail/wagtailsnippets/tests.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/wagtail/wagtailsnippets/tests.py b/wagtail/wagtailsnippets/tests.py index 92e187d86..5e723a5fe 100644 --- a/wagtail/wagtailsnippets/tests.py +++ b/wagtail/wagtailsnippets/tests.py @@ -241,14 +241,14 @@ class TestSnippetChooserPanel(TestCase): def test_target_content_type(self): result = SnippetChooserPanel( - 'barbecue', - 'wagtailcore.site' + 'advert', + 'tests.advert' ).bind_to_model(SnippetChooserModel).target_content_type() - self.assertEqual(result.name, 'Site') + self.assertEqual(result.name, 'advert') def test_target_content_type_malformed_type(self): result = SnippetChooserPanel( - 'barbecue', + 'advert', 'snowman' ).bind_to_model(SnippetChooserModel) self.assertRaises(ImproperlyConfigured, @@ -256,7 +256,7 @@ class TestSnippetChooserPanel(TestCase): def test_target_content_type_nonexistent_type(self): result = SnippetChooserPanel( - 'barbecue', + 'advert', 'snowman.lorry' ).bind_to_model(SnippetChooserModel) self.assertRaises(ImproperlyConfigured, From 64a58be072dbdc8697dc9b49a90d00465a933997 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 27 Aug 2015 17:34:51 +0100 Subject: [PATCH 4/9] Allow the snippet_type argument to be omitted from SnippetChooserPanel; add tests --- wagtail/wagtailsnippets/edit_handlers.py | 2 +- wagtail/wagtailsnippets/tests.py | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/wagtail/wagtailsnippets/edit_handlers.py b/wagtail/wagtailsnippets/edit_handlers.py index 22992e39a..8a7370401 100644 --- a/wagtail/wagtailsnippets/edit_handlers.py +++ b/wagtail/wagtailsnippets/edit_handlers.py @@ -55,7 +55,7 @@ class BaseSnippetChooserPanel(BaseChooserPanel): class SnippetChooserPanel(object): - def __init__(self, field_name, snippet_type): + def __init__(self, field_name, snippet_type=None): self.field_name = field_name self.snippet_type = snippet_type diff --git a/wagtail/wagtailsnippets/tests.py b/wagtail/wagtailsnippets/tests.py index 5e723a5fe..256c599f3 100644 --- a/wagtail/wagtailsnippets/tests.py +++ b/wagtail/wagtailsnippets/tests.py @@ -239,13 +239,26 @@ class TestSnippetChooserPanel(TestCase): self.assertIn('createSnippetChooser("id_advert", "tests/advert");', self.snippet_chooser_panel.render_as_field()) - def test_target_content_type(self): + def test_target_content_type_from_string(self): result = SnippetChooserPanel( 'advert', 'tests.advert' ).bind_to_model(SnippetChooserModel).target_content_type() self.assertEqual(result.name, 'advert') + def test_target_content_type_from_model(self): + result = SnippetChooserPanel( + 'advert', + Advert + ).bind_to_model(SnippetChooserModel).target_content_type() + self.assertEqual(result.name, 'advert') + + def test_target_content_type_autodetected(self): + result = SnippetChooserPanel( + 'advert' + ).bind_to_model(SnippetChooserModel).target_content_type() + self.assertEqual(result.name, 'advert') + def test_target_content_type_malformed_type(self): result = SnippetChooserPanel( 'advert', From 0ea5eee45ec284602dc1f699c67ba2a7690e8ad7 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 27 Aug 2015 19:57:26 +0100 Subject: [PATCH 5/9] Failing test for snippet chooser button text --- wagtail/wagtailsnippets/tests.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/wagtail/wagtailsnippets/tests.py b/wagtail/wagtailsnippets/tests.py index 256c599f3..6625ce295 100644 --- a/wagtail/wagtailsnippets/tests.py +++ b/wagtail/wagtailsnippets/tests.py @@ -233,7 +233,10 @@ class TestSnippetChooserPanel(TestCase): '_SnippetChooserPanel') def test_render_as_field(self): - self.assertTrue(self.advert_text in self.snippet_chooser_panel.render_as_field()) + field_html = self.snippet_chooser_panel.render_as_field() + self.assertIn(self.advert_text, field_html) + self.assertIn("Choose advert", field_html) + self.assertIn("Choose another advert", field_html) def test_render_js(self): self.assertIn('createSnippetChooser("id_advert", "tests/advert");', From 939ff0827b3fd2ea044cccefe3e0061f3f059c15 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 28 Aug 2015 10:13:25 +0100 Subject: [PATCH 6/9] Turn snippet_type_name into a class method --- wagtail/wagtailsnippets/edit_handlers.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/wagtail/wagtailsnippets/edit_handlers.py b/wagtail/wagtailsnippets/edit_handlers.py index 8a7370401..f1c5e2886 100644 --- a/wagtail/wagtailsnippets/edit_handlers.py +++ b/wagtail/wagtailsnippets/edit_handlers.py @@ -19,7 +19,7 @@ class BaseSnippetChooserPanel(BaseChooserPanel): @classmethod def widget_overrides(cls): return {cls.field_name: AdminSnippetChooser( - content_type=cls.target_content_type(), snippet_type_name=cls.snippet_type_name)} + content_type=cls.target_content_type(), snippet_type_name=cls.get_snippet_type_name())} @classmethod def target_content_type(cls): @@ -46,12 +46,12 @@ class BaseSnippetChooserPanel(BaseChooserPanel): return mark_safe(render_to_string(self.field_template, { 'field': self.bound_field, self.object_type_name: instance_obj, - 'snippet_type_name': self.snippet_type_name, + 'snippet_type_name': self.get_snippet_type_name(), })) - @property - def snippet_type_name(self): - return force_text(self.target_content_type()._meta.verbose_name) + @classmethod + def get_snippet_type_name(cls): + return force_text(cls.target_content_type()._meta.verbose_name) class SnippetChooserPanel(object): From 6a7a23364bbf5cd486ad508b5776cac6c1050bdc Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 28 Aug 2015 10:39:27 +0100 Subject: [PATCH 7/9] Get snippet type name from model, not content type (otherwise it returns 'Content type') --- wagtail/wagtailsnippets/edit_handlers.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/wagtail/wagtailsnippets/edit_handlers.py b/wagtail/wagtailsnippets/edit_handlers.py index f1c5e2886..7ceadd1f0 100644 --- a/wagtail/wagtailsnippets/edit_handlers.py +++ b/wagtail/wagtailsnippets/edit_handlers.py @@ -14,6 +14,7 @@ from .widgets import AdminSnippetChooser class BaseSnippetChooserPanel(BaseChooserPanel): object_type_name = 'item' + _target_model = None _target_content_type = None @classmethod @@ -22,23 +23,26 @@ class BaseSnippetChooserPanel(BaseChooserPanel): content_type=cls.target_content_type(), snippet_type_name=cls.get_snippet_type_name())} @classmethod - def target_content_type(cls): - if cls._target_content_type is None: + def target_model(cls): + if cls._target_model is None: if cls.snippet_type: try: - model = resolve_model_string(cls.snippet_type) + cls._target_model = resolve_model_string(cls.snippet_type) except LookupError: raise ImproperlyConfigured("{0}.snippet_type must be of the form 'app_label.model_name', given {1!r}".format( cls.__name__, cls.snippet_type)) except ValueError: raise ImproperlyConfigured("{0}.snippet_type refers to model {1!r} that has not been installed".format( cls.__name__, cls.snippet_type)) - - cls._target_content_type = ContentType.objects.get_for_model(model) else: - target_model = cls.model._meta.get_field(cls.field_name).rel.to - cls._target_content_type = ContentType.objects.get_for_model(target_model) + cls._target_model = cls.model._meta.get_field(cls.field_name).rel.to + return cls._target_model + + @classmethod + def target_content_type(cls): + if cls._target_content_type is None: + cls._target_content_type = ContentType.objects.get_for_model(cls.target_model()) return cls._target_content_type def render_as_field(self): @@ -51,7 +55,7 @@ class BaseSnippetChooserPanel(BaseChooserPanel): @classmethod def get_snippet_type_name(cls): - return force_text(cls.target_content_type()._meta.verbose_name) + return force_text(cls.target_model()._meta.verbose_name) class SnippetChooserPanel(object): From 2ed16e3ee2a46b6dfb05232edb547b4db63c7691 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 28 Aug 2015 10:50:32 +0100 Subject: [PATCH 8/9] Update documentation to indicate that SnippetChooserPanel no longer requires a snippet_type argument --- .../customisation/page_editing_interface.rst | 2 +- docs/reference/pages/editing_api.rst | 10 +++++++--- docs/topics/snippets.rst | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/docs/advanced_topics/customisation/page_editing_interface.rst b/docs/advanced_topics/customisation/page_editing_interface.rst index 51311d3c2..9e4161ce0 100644 --- a/docs/advanced_topics/customisation/page_editing_interface.rst +++ b/docs/advanced_topics/customisation/page_editing_interface.rst @@ -23,7 +23,7 @@ As standard, Wagtail organises panels into three tabs: 'Content', 'Promote' and FieldPanel('body', classname="full"), ] sidebar_content_panels = [ - SnippetChooserPanel('advert', Advert), + SnippetChooserPanel('advert'), InlinePanel('related_links', label="Related links"), ] diff --git a/docs/reference/pages/editing_api.rst b/docs/reference/pages/editing_api.rst index 8a04eb085..987b4eb9f 100644 --- a/docs/reference/pages/editing_api.rst +++ b/docs/reference/pages/editing_api.rst @@ -233,9 +233,13 @@ DocumentChooserPanel SnippetChooserPanel ------------------- -.. class:: wagtail.wagtailsnippets.edit_handlers.SnippetChooserPanel(field_name, model) +.. versionchanged:: 1.1 - Snippets are vanilla Django models you create yourself without a Wagtail-provided base class. So using them as a field in a page requires specifying your own ``appname.modelname``. A chooser, ``SnippetChooserPanel``, is provided which takes the field name and snippet class. + Before Wagtail 1.1, it was necessary to pass the snippet model class as a second parameter to ``SnippetChooserPanel``. This is now automatically picked up from the field. + +.. class:: wagtail.wagtailsnippets.edit_handlers.SnippetChooserPanel(field_name, snippet_type=None) + + Snippets are vanilla Django models you create yourself without a Wagtail-provided base class. A chooser, ``SnippetChooserPanel``, is provided which takes the field name as an argument. .. code-block:: python @@ -251,7 +255,7 @@ SnippetChooserPanel ) content_panels = Page.content_panels + [ - SnippetChooserPanel('advert', Advert), + SnippetChooserPanel('advert'), ] See :ref:`snippets` for more information. diff --git a/docs/topics/snippets.rst b/docs/topics/snippets.rst index f0c76c451..8e4defe79 100644 --- a/docs/topics/snippets.rst +++ b/docs/topics/snippets.rst @@ -112,7 +112,7 @@ In the above example, the list of adverts is a fixed list, displayed as part of BookPage.content_panels = [ - SnippetChooserPanel('advert', Advert), + SnippetChooserPanel('advert'), # ... ] @@ -142,7 +142,7 @@ To attach multiple adverts to a page, the ``SnippetChooserPanel`` can be placed verbose_name_plural = "Advert Placements" panels = [ - SnippetChooserPanel('advert', Advert), + SnippetChooserPanel('advert'), ] def __str__(self): # __unicode__ on Python 2 From 1e4618935d69df3464c48bd54c4d807308359200 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 28 Aug 2015 10:59:19 +0100 Subject: [PATCH 9/9] Release note for #1375 --- CHANGELOG.txt | 1 + docs/releases/1.1.rst | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 3f1dbcde8..986385d03 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -26,6 +26,7 @@ Changelog * Added database indexes on PageRevision and Image to improve performance on large sites * Search in page chooser now uses Wagtail's search framework, to order results by relevance * `PageChooserPanel` now supports passing a list (or tuple) of accepted page types + * The snippet type parameter of `SnippetChooserPanel` can now be omitted, or passed as a model name string rather than a model class * Fix: Text areas in the non-default tab of the page editor now resize to the correct height * Fix: Tabs in "insert link" modal in the rich text editor no longer disappear (Tim Heap) * Fix: H2 elements in rich text fields were accidentally given a click() binding when put insite a collapsible multi field panel diff --git a/docs/releases/1.1.rst b/docs/releases/1.1.rst index a92b7e876..0ed3c4c9e 100644 --- a/docs/releases/1.1.rst +++ b/docs/releases/1.1.rst @@ -63,6 +63,7 @@ Minor features * Added database indexes on PageRevision and Image to improve performance on large sites * Search in page chooser now uses Wagtail's search framework, to order results by relevance * ``PageChooserPanel`` now supports passing a list (or tuple) of accepted page types + * The snippet type parameter of ``SnippetChooserPanel`` can now be omitted, or passed as a model name string rather than a model class Bug fixes ~~~~~~~~~