From 5df60caef3b0617f78665135142bd756629fa267 Mon Sep 17 00:00:00 2001 From: Tim Heap Date: Fri, 21 Aug 2015 20:09:36 +1000 Subject: [PATCH 1/4] Rename `is_abstract` to `is_creatable` `is_creatable` better reflects what it is used for, and stops any confusion between Wagtail's `is_abstract` and Django's `Meta.abstract`. `is_creatable` takes in to account `Meta.abstract` now, so developers will no longer need to set both `is_abstract` and `Meta.abstract`. Documentation for the new attribute has been added, as well as tests. --- docs/reference/pages/model_reference.rst | 4 +++ wagtail/contrib/wagtailroutablepage/models.py | 2 -- wagtail/contrib/wagtailroutablepage/tests.py | 2 -- .../0010_mtibasepage_mtichildpage.py | 35 +++++++++++++++++++ wagtail/tests/testapp/models.py | 14 ++++++++ wagtail/wagtailcore/models.py | 14 ++++---- wagtail/wagtailcore/tests/test_page_model.py | 34 ++++++++++++++++-- wagtail/wagtailforms/models.py | 2 -- 8 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 wagtail/tests/testapp/migrations/0010_mtibasepage_mtichildpage.py diff --git a/docs/reference/pages/model_reference.rst b/docs/reference/pages/model_reference.rst index 3e74fdb4c..bee258f99 100644 --- a/docs/reference/pages/model_reference.rst +++ b/docs/reference/pages/model_reference.rst @@ -153,6 +153,10 @@ In addition to the model fields provided, ``Page`` has many properties and metho Defines which template file should be used to render the login form for Protected pages using this model. This overrides the default, defined using ``PASSWORD_REQUIRED_TEMPLATE`` in your settings. See :ref:`private_pages` + .. attribute:: is_creatable + + Controls if this page can be created through the Wagtail administration. Defaults to True, and is not inherited by subclasses. This is useful when using `multi-table inheritance `_, to stop the base model from being created as an actual page. + ``Site`` ======== diff --git a/wagtail/contrib/wagtailroutablepage/models.py b/wagtail/contrib/wagtailroutablepage/models.py index faa20efee..832fb7ac4 100644 --- a/wagtail/contrib/wagtailroutablepage/models.py +++ b/wagtail/contrib/wagtailroutablepage/models.py @@ -138,7 +138,5 @@ class RoutablePage(RoutablePageMixin, Page): added to it. """ - is_abstract = True - class Meta: abstract = True diff --git a/wagtail/contrib/wagtailroutablepage/tests.py b/wagtail/contrib/wagtailroutablepage/tests.py index 48df2b8a7..64e247276 100644 --- a/wagtail/contrib/wagtailroutablepage/tests.py +++ b/wagtail/contrib/wagtailroutablepage/tests.py @@ -151,8 +151,6 @@ class TestOldStyleRoutablePage(TestNewStyleRoutablePage, WagtailTestUtils): # prevent this class appearing in the global PAGE_MODEL_CLASSES list, as # its non-standard location causes failures when translating from content types # back to models - is_abstract = True - class Meta: abstract = True diff --git a/wagtail/tests/testapp/migrations/0010_mtibasepage_mtichildpage.py b/wagtail/tests/testapp/migrations/0010_mtibasepage_mtichildpage.py new file mode 100644 index 000000000..06007a721 --- /dev/null +++ b/wagtail/tests/testapp/migrations/0010_mtibasepage_mtichildpage.py @@ -0,0 +1,35 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('wagtailcore', '0019_verbose_names_cleanup'), + ('tests', '0009_auto_20150820_0419'), + ] + + operations = [ + migrations.CreateModel( + name='MTIBasePage', + fields=[ + ('page_ptr', models.OneToOneField(parent_link=True, auto_created=True, primary_key=True, serialize=False, to='wagtailcore.Page')), + ], + options={ + 'abstract': False, + }, + bases=('wagtailcore.page',), + ), + migrations.CreateModel( + name='MTIChildPage', + fields=[ + ('mtibasepage_ptr', models.OneToOneField(parent_link=True, auto_created=True, primary_key=True, serialize=False, to='tests.MTIBasePage')), + ], + options={ + 'abstract': False, + }, + bases=('tests.mtibasepage',), + ), + ] diff --git a/wagtail/tests/testapp/models.py b/wagtail/tests/testapp/models.py index c2334dca2..277832b85 100644 --- a/wagtail/tests/testapp/models.py +++ b/wagtail/tests/testapp/models.py @@ -440,3 +440,17 @@ class StreamPage(Page): ]) api_fields = ('body',) + + +class MTIBasePage(Page): + is_creatable = False + + +class MTIChildPage(MTIBasePage): + # Should be creatable by default, no need to set anything + pass + + +class AbstractPage(Page): + class Meta: + abstract = True diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 12a59bd39..581693d48 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -263,11 +263,12 @@ class PageBase(models.base.ModelBase): cls._clean_subpage_types = None # to be filled in on first call to cls.clean_subpage_types cls._clean_parent_page_types = None # to be filled in on first call to cls.clean_parent_page_types - if not dct.get('is_abstract'): - # subclasses are only abstract if the subclass itself defines itself so - cls.is_abstract = False + # All pages should be creatable unless explicitly set otherwise. + # This attribute is not inheritable. + if 'is_creatable' not in dct: + cls.is_creatable = not cls._meta.abstract - if not cls.is_abstract: + if cls.is_creatable: # register this type in the list of page content types PAGE_MODEL_CLASSES.append(cls) @@ -309,6 +310,9 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed index.FilterField('show_in_menus'), ) + # Do not allow plain Page instances to be created through the Wagtail admin + is_creatable = False + def __init__(self, *args, **kwargs): super(Page, self).__init__(*args, **kwargs) if not self.id and not self.content_type_id: @@ -320,8 +324,6 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed def __str__(self): return self.title - is_abstract = True # don't offer Page in the list of page types a superuser can create - def set_url_path(self, parent): """ Populate the url_path field based on this page's slug and the specified parent page. diff --git a/wagtail/wagtailcore/tests/test_page_model.py b/wagtail/wagtailcore/tests/test_page_model.py index c9c81f004..9ac12ab23 100644 --- a/wagtail/wagtailcore/tests/test_page_model.py +++ b/wagtail/wagtailcore/tests/test_page_model.py @@ -10,8 +10,11 @@ from django.contrib.contenttypes.models import ContentType from django.contrib.auth import get_user_model from django.contrib.auth.models import AnonymousUser -from wagtail.wagtailcore.models import Page, Site -from wagtail.tests.testapp.models import SingleEventPage, EventPage, EventIndex, SimplePage, BusinessIndex, BusinessSubIndex, BusinessChild, StandardIndex +from wagtail.wagtailcore.models import Page, Site, PAGE_MODEL_CLASSES +from wagtail.tests.testapp.models import ( + SingleEventPage, EventPage, EventIndex, SimplePage, + BusinessIndex, BusinessSubIndex, BusinessChild, StandardIndex, + MTIBasePage, MTIChildPage, AbstractPage) class TestSiteRouting(TestCase): @@ -731,3 +734,30 @@ class TestIssue1216(TestCase): new_christmas_event = EventPage.objects.get(id=christmas_event.id) expected_url_path = "/home/%s/%s/" % (new_event_index_slug, new_christmas_slug) self.assertEqual(new_christmas_event.url_path, expected_url_path) + + +class TestIsCreatable(TestCase): + def test_is_creatable_default(self): + """By default, pages should be creatable""" + self.assertTrue(SimplePage.is_creatable) + self.assertIn(SimplePage, PAGE_MODEL_CLASSES) + + def test_is_creatable_false(self): + """Page types should be able to disable their creation""" + self.assertFalse(MTIBasePage.is_creatable) + self.assertNotIn(MTIBasePage, PAGE_MODEL_CLASSES) + + def test_is_creatable_not_inherited(self): + """ + is_creatable should not be inherited in the normal manner, and should + default to True unless set otherwise + """ + self.assertTrue(MTIChildPage.is_creatable) + self.assertIn(MTIChildPage, PAGE_MODEL_CLASSES) + + def test_abstract_pages(self): + """ + Abstract models should not be creatable + """ + self.assertFalse(AbstractPage.is_creatable) + self.assertNotIn(AbstractPage, PAGE_MODEL_CLASSES) diff --git a/wagtail/wagtailforms/models.py b/wagtail/wagtailforms/models.py index 92ec6caf2..98bc0f25b 100644 --- a/wagtail/wagtailforms/models.py +++ b/wagtail/wagtailforms/models.py @@ -131,7 +131,6 @@ class AbstractForm(Page): """ form_builder = FormBuilder - is_abstract = True # Don't display me in "Add" def __init__(self, *args, **kwargs): super(AbstractForm, self).__init__(*args, **kwargs) @@ -207,7 +206,6 @@ class AbstractEmailForm(AbstractForm): """ A Form Page that sends email. Pages implementing a form to be send to an email should inherit from it """ - is_abstract = True # Don't display me in "Add" to_address = models.CharField(verbose_name=_('To address'), max_length=255, blank=True, help_text=_("Optional - form submissions will be emailed to this address")) from_address = models.CharField(verbose_name=_('From address'), max_length=255, blank=True) From e87cecdcaf18c07c203bd480fe9351d810c1d8b2 Mon Sep 17 00:00:00 2001 From: Tim Heap Date: Fri, 4 Sep 2015 09:56:15 +1000 Subject: [PATCH 2/4] Add deprecated is_abstract support for backwards compatibility It raises a RemovedInWagtail13Warning when used, and is only checked if `is_creatable` is not present. --- wagtail/wagtailcore/models.py | 14 +++++++++++--- wagtail/wagtailcore/tests/test_page_model.py | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 581693d48..90d2c2aa8 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -2,10 +2,10 @@ from __future__ import unicode_literals import logging import json +import warnings + from collections import defaultdict - from modelcluster.models import ClusterableModel, get_all_child_relations - import django from django.db import models, connection, transaction from django.db.models import Q @@ -42,6 +42,8 @@ from wagtail.wagtailcore.signals import page_published, page_unpublished from wagtail.wagtailsearch import index from wagtail.wagtailsearch.backends import get_search_backend +from wagtail.utils.deprecation import RemovedInWagtail13Warning + logger = logging.getLogger('wagtail.core') @@ -266,7 +268,13 @@ class PageBase(models.base.ModelBase): # All pages should be creatable unless explicitly set otherwise. # This attribute is not inheritable. if 'is_creatable' not in dct: - cls.is_creatable = not cls._meta.abstract + if 'is_abstract' in dct: + warnings.warn( + "The is_abstract flag is deprecated - use is_creatable instead.", + RemovedInWagtail13Warning) + cls.is_creatable = not dct['is_abstract'] + else: + cls.is_creatable = not cls._meta.abstract if cls.is_creatable: # register this type in the list of page content types diff --git a/wagtail/wagtailcore/tests/test_page_model.py b/wagtail/wagtailcore/tests/test_page_model.py index 9ac12ab23..81a2f6c1e 100644 --- a/wagtail/wagtailcore/tests/test_page_model.py +++ b/wagtail/wagtailcore/tests/test_page_model.py @@ -1,5 +1,6 @@ import datetime import json +import warnings import pytz @@ -9,6 +10,7 @@ from django.http import HttpRequest, Http404 from django.contrib.contenttypes.models import ContentType from django.contrib.auth import get_user_model from django.contrib.auth.models import AnonymousUser +from django.utils.six import text_type from wagtail.wagtailcore.models import Page, Site, PAGE_MODEL_CLASSES from wagtail.tests.testapp.models import ( @@ -761,3 +763,19 @@ class TestIsCreatable(TestCase): """ self.assertFalse(AbstractPage.is_creatable) self.assertNotIn(AbstractPage, PAGE_MODEL_CLASSES) + + def test_is_abstract(self): + """ + is_abstract has been deprecated. Check that it still works, but issues + a deprecation warning + """ + with warnings.catch_warnings(record=True) as ws: + class IsAbstractPage(Page): + is_abstract = True + + class Meta: + abstract = True + + self.assertEqual(len(ws), 1) + warning = ws[0] + self.assertIn("is_creatable", text_type(warning.message)) From c3beacacf0e86077b804fb91c25a145e4a8fed8d Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 4 Sep 2015 12:19:50 +0100 Subject: [PATCH 3/4] Test that the deprecated is_abstract=True flag is equivalent to is_creatable=False --- wagtail/wagtailcore/tests/test_page_model.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/wagtail/wagtailcore/tests/test_page_model.py b/wagtail/wagtailcore/tests/test_page_model.py index 81a2f6c1e..5483b31ca 100644 --- a/wagtail/wagtailcore/tests/test_page_model.py +++ b/wagtail/wagtailcore/tests/test_page_model.py @@ -779,3 +779,6 @@ class TestIsCreatable(TestCase): self.assertEqual(len(ws), 1) warning = ws[0] self.assertIn("is_creatable", text_type(warning.message)) + + self.assertFalse(AbstractPage.is_creatable) + self.assertNotIn(AbstractPage, PAGE_MODEL_CLASSES) From c17c7b1b87ec910aa81782c687eb381cfb847e27 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 4 Sep 2015 12:42:36 +0100 Subject: [PATCH 4/4] Release note and upgrade consideration for #1631 --- CHANGELOG.txt | 1 + docs/releases/1.1.rst | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index fdbd8f67f..969b64eae 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -31,6 +31,7 @@ Changelog * Added signposting text to the explorer to steer editors away from creating pages at the root level unless they are setting up new sites * "Clear choice" and "Edit this page" buttons are no longer shown on the page field of the group page permissions form * Altered styling of stream controls to be more like all other buttons + * Added ability to mark page models as not available for creation using the flag `is_creatable`; pages that are abstract Django models are automatically made non-creatable * 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 147906912..6f83a717e 100644 --- a/docs/releases/1.1.rst +++ b/docs/releases/1.1.rst @@ -68,6 +68,7 @@ Minor features * Added signposting text to the explorer to steer editors away from creating pages at the root level unless they are setting up new sites * "Clear choice" and "Edit this page" buttons are no longer shown on the page field of the group page permissions form * Altered styling of stream controls to be more like all other buttons + * Added ability to mark page models as not available for creation using the flag ``is_creatable``; pages that are abstract Django models are automatically made non-creatable Bug fixes ~~~~~~~~~ @@ -111,3 +112,13 @@ project, you will need to update these to point to the :mod:`wagtail.contrib.wag If you created your project using the ``wagtail start`` command with Wagtail 1.0, you will probably have references to this model in the ``search/views.py`` file. + + +``is_abstract`` flag on page models has been replaced by ``is_creatable`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Previous versions of Wagtail provided an undocumented ``is_abstract`` flag on page models - not to be confused with Django's ``abstract`` Meta flag - to indicate that it should not be included in the list of available page types for creation. (Typically this would be used on model classes that were designed to be subclassed to create new page types, rather than used directly.) To avoid confusion with Django's distinct concept of abstract models, this has now been replaced by a new flag, ``is_creatable``. + +If you have used ``is_abstract = True`` on any of your models, you should now change this to ``is_creatable = False``. + +It is not necessary to include this flag if the model is abstract in the Django sense (i.e. it has ``abstract = True`` in the model's ``Meta`` class), since it would never be valid to create pages of that type.