From 078491b6e068cb5bf7ba9f190cfc3ac085548c28 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 22 Dec 2015 09:36:21 +0000 Subject: [PATCH] Prevent cascade deletion of pages when a ContentType is deleted Fixes #2024 --- CHANGELOG.txt | 1 + docs/releases/1.4.rst | 1 + ...r_page_content_type_on_delete_behaviour.py | 21 +++++++++++++++++++ wagtail/wagtailcore/models.py | 17 ++++++++++++--- wagtail/wagtailcore/tests/test_page_model.py | 20 ++++++++++++++++++ 5 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 wagtail/wagtailcore/migrations/0024_alter_page_content_type_on_delete_behaviour.py diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 3d9abccc7..5eece8552 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -7,6 +7,7 @@ Changelog * The `Document` model can now be overridden using the new `WAGTAILDOCS_DOCUMENT_MODEL` setting (Alex Gleason) * Fix: Custom page managers no longer raise an error when used on an abstract model * Fix: Wagtail's migrations are now all reversible (benjaoming) + * Fix: Deleting a page content type now preserves existing pages as basic Page instances, to prevent tree corruption 1.3.1 (05.01.2016) ~~~~~~~~~~~~~~~~~~ diff --git a/docs/releases/1.4.rst b/docs/releases/1.4.rst index 489a4fee0..977c68cf5 100644 --- a/docs/releases/1.4.rst +++ b/docs/releases/1.4.rst @@ -24,6 +24,7 @@ Bug fixes * Custom page managers no longer raise an error when used on an abstract model * Wagtail's migrations are now all reversible (benjaoming) + * Deleting a page content type now preserves existing pages as basic Page instances, to prevent tree corruption Upgrade considerations diff --git a/wagtail/wagtailcore/migrations/0024_alter_page_content_type_on_delete_behaviour.py b/wagtail/wagtailcore/migrations/0024_alter_page_content_type_on_delete_behaviour.py new file mode 100644 index 000000000..36940a25a --- /dev/null +++ b/wagtail/wagtailcore/migrations/0024_alter_page_content_type_on_delete_behaviour.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.9 on 2015-12-22 09:34 +from __future__ import unicode_literals + +from django.db import migrations, models +import wagtail.wagtailcore.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('wagtailcore', '0023_alter_page_revision_on_delete_behaviour'), + ] + + operations = [ + migrations.AlterField( + model_name='page', + name='content_type', + field=models.ForeignKey(on_delete=models.SET(wagtail.wagtailcore.models.get_default_page_content_type), related_name='pages', to='contenttypes.ContentType', verbose_name='content type'), + ), + ] diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index af64d40d8..da7acb1fb 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -205,6 +205,14 @@ def get_page_models(): return PAGE_MODEL_CLASSES +def get_default_page_content_type(): + """ + Returns the content type to use as a default for pages whose content type + has been deleted. + """ + return ContentType.objects.get_for_model(Page) + + def get_page_types(): """ DEPRECATED. @@ -277,7 +285,12 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed ) # TODO: enforce uniqueness on slug field per parent (will have to be done at the Django # level rather than db, since there is no explicit parent relation in the db) - content_type = models.ForeignKey('contenttypes.ContentType', verbose_name=_('content type'), related_name='pages') + content_type = models.ForeignKey( + 'contenttypes.ContentType', + verbose_name=_('content type'), + related_name='pages', + on_delete=models.SET(get_default_page_content_type) + ) live = models.BooleanField(verbose_name=_('live'), default=True, editable=False) has_unpublished_changes = models.BooleanField( verbose_name=_('has unpublished changes'), @@ -452,8 +465,6 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed for model in [cls] + list(cls._meta.get_parent_list()) for field in model._meta.parents.values() if field] - field_exceptions += ['content_type'] - for field in cls._meta.fields: if isinstance(field, models.ForeignKey) and field.name not in field_exceptions: if field.rel.on_delete == models.CASCADE: diff --git a/wagtail/wagtailcore/tests/test_page_model.py b/wagtail/wagtailcore/tests/test_page_model.py index 2a7ab86a7..6a0f86d84 100644 --- a/wagtail/wagtailcore/tests/test_page_model.py +++ b/wagtail/wagtailcore/tests/test_page_model.py @@ -1036,3 +1036,23 @@ class TestPageManager(TestCase): their custom Manager inherits from PageManager. """ self.assertIs(type(MyCustomPage.objects), CustomManager) + + +class TestIssue2024(TestCase): + """ + This tests that deleting a content type can't delete any Page objects. + """ + fixtures = ['test.json'] + + def test_delete_content_type(self): + event_index = Page.objects.get(url_path='/home/events/') + + # Delete the content type + event_index_content_type = event_index.content_type + event_index_content_type.delete() + + # Fetch the page again, it should still exist + event_index = Page.objects.get(url_path='/home/events/') + + # Check that the content_type changed to Page + self.assertEqual(event_index.content_type, ContentType.objects.get_for_model(Page))