Merge pull request #599 from gasman/fix/child_relations

Fix page copy and replace_text to pick up child relations that are defined on a superclass

CHANGELOG.txt

@@ -12,6 +12,7 @@ Changelog
  * Fix: Page URL generation now returns correct URLs for sites that have the main 'serve' view rooted somewhere other than '/'
  * Fix: Search results in the page chooser now respect the page_type parameter on PageChooserPanel
  * Fix: Rendition filenames are now prevented from going over 60 chars, even with a large focal_point_key
+ * Fix: Child relations that are defined on a model's superclass (such as the base Page model) are now picked up correctly by the page editing form, page copy operations and the replace_text management command
 
 0.5 (01.08.2014)
 ~~~~~~~~~~~~~~~~

docs/releases/0.6.rst

@@ -30,6 +30,7 @@ Bug fixes
  * Page URL generation now returns correct URLs for sites that have the main 'serve' view rooted somewhere other than '/'.
  * Search results in the page chooser now respect the page_type parameter on PageChooserPanel.
  * Rendition filenames are now prevented from going over 60 chars, even with a large focal_point_key.
+ * Child relations that are defined on a model's superclass (such as the base Page model) are now picked up correctly by the page editing form, page copy operations and the replace_text management command.
 
 
 Upgrade considerations

wagtail/tests/fixtures/test.json

@@ -90,8 +90,8 @@
     "model": "tests.eventpagespeaker",
     "fields": {
         "page": 4,
-        "first_name": "Santa",
-        "last_name": "Claus",
+        "first_name": "Father",
+        "last_name": "Christmas",
         "sort_order": 0
     }
 },
@@ -567,7 +567,17 @@
     "model": "tests.AdvertPlacement",
     "fields": {
         "page": 2,
-        "advert": 1
+        "advert": 1,
+        "colour": "yellow"
+        }
+},
+{
+    "pk": 2,
+    "model": "tests.AdvertPlacement",
+    "fields": {
+        "page": 4,
+        "advert": 1,
+        "colour": "greener than a Christmas tree"
         }
 },
 {

wagtail/tests/migrations/0003_auto_20140905_0634.py

@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('tests', '0002_auto_20140827_0908'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='advertplacement',
+            name='colour',
+            field=models.CharField(default='blue', max_length=255),
+            preserve_default=False,
+        ),
+    ]

wagtail/tests/models.py

@@ -333,6 +333,7 @@ FormPage.content_panels = [
 class AdvertPlacement(models.Model):
     page = ParentalKey('wagtailcore.Page', related_name='advert_placements')
     advert = models.ForeignKey('tests.Advert', related_name='+')
+    colour = models.CharField(max_length=255)
 
 @python_2_unicode_compatible
 class Advert(models.Model):

wagtail/wagtailadmin/tests/test_pages_views.py

@@ -1738,6 +1738,7 @@ class TestChildRelationsOnSuperclass(TestCase, WagtailTestUtils):
             'advert_placements-INITIAL_FORMS': '0',
             'advert_placements-MAX_NUM_FORMS': '1000',
             'advert_placements-0-advert': '1',
+            'advert_placements-0-colour': 'yellow',
             'advert_placements-0-id': '',
         }
         response = self.client.post(reverse('wagtailadmin_pages_create', args=('tests', 'standardindex', self.root_page.id)), post_data)
@@ -1769,8 +1770,10 @@ class TestChildRelationsOnSuperclass(TestCase, WagtailTestUtils):
             'advert_placements-INITIAL_FORMS': '1',
             'advert_placements-MAX_NUM_FORMS': '1000',
             'advert_placements-0-advert': '1',
+            'advert_placements-0-colour': 'yellow',
             'advert_placements-0-id': self.index_page.advert_placements.first().id,
             'advert_placements-1-advert': '1',
+            'advert_placements-1-colour': 'purple',
             'advert_placements-1-id': '',
             'action-publish': "Publish",
         }

wagtail/wagtailcore/management/commands/replace_text.py

@@ -1,6 +1,8 @@
 from django.core.management.base import BaseCommand
 from django.db import models
 
+from modelcluster.models import get_all_child_relations
+
 from wagtail.wagtailcore.models import PageRevision, get_page_types
 
 
@@ -27,10 +29,7 @@ class Command(BaseCommand):
             self.stdout.write("scanning %s" % content_type.name)
             page_class = content_type.model_class()
 
-            try:
-                child_relation_names = [rel.get_accessor_name() for rel in page_class._meta.child_relations]
-            except AttributeError:
-                child_relation_names = []
+            child_relation_names = [rel.get_accessor_name() for rel in get_all_child_relations(page_class)]
 
             for page in page_class.objects.all():
                 replace_in_model(page, from_text, to_text)

wagtail/wagtailcore/models.py

@@ -5,7 +5,7 @@ from six import string_types
 from six import StringIO
 from six.moves.urllib.parse import urlparse
 
-from modelcluster.models import ClusterableModel
+from modelcluster.models import ClusterableModel, get_all_child_relations
 
 from django.db import models, connection, transaction
 from django.db.models import get_model, Q
@@ -618,7 +618,7 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed
 
         # Copy child objects
         specific_self = self.specific
-        for child_relation in getattr(specific_self._meta, 'child_relations', []):
+        for child_relation in get_all_child_relations(specific_self):
             parental_key_name = child_relation.field.attname
             child_objects = getattr(specific_self, child_relation.get_accessor_name(), None)
 

wagtail/wagtailcore/tests/test_management_commands.py

@@ -8,7 +8,7 @@ from django.utils import timezone
 
 from wagtail.wagtailcore.models import Page, PageRevision
 from wagtail.wagtailcore.signals import page_published, page_unpublished
-from wagtail.tests.models import SimplePage
+from wagtail.tests.models import SimplePage, EventPage
 
 
 class TestFixTreeCommand(TestCase):
@@ -82,13 +82,21 @@ class TestReplaceTextCommand(TestCase):
 
     def test_replace_text(self):
         # Check that the christmas page is definitely about christmas
-        self.assertEqual(Page.objects.get(url_path='/home/events/christmas/').title, "Christmas")
+        christmas_page = EventPage.objects.get(url_path='/home/events/christmas/')
+        self.assertEqual(christmas_page.title, "Christmas")
+        self.assertEqual(christmas_page.speakers.first().last_name, "Christmas")
+        self.assertEqual(christmas_page.advert_placements.first().colour, "greener than a Christmas tree")
 
         # Make it about easter
         self.run_command("Christmas", "Easter")
 
-        # Check that its now about easter
-        self.assertEqual(Page.objects.get(url_path='/home/events/christmas/').title, "Easter")
+        # Check that it's now about easter
+        easter_page = EventPage.objects.get(url_path='/home/events/christmas/')
+        self.assertEqual(easter_page.title, "Easter")
+
+        # Check that we also update the child objects (including advert_placements, which is defined on the superclass)
+        self.assertEqual(easter_page.speakers.first().last_name, "Easter")
+        self.assertEqual(easter_page.advert_placements.first().colour, "greener than a Easter tree")
 
 
 class TestPublishScheduledPagesCommand(TestCase):

wagtail/wagtailcore/tests/test_page_model.py

@@ -400,6 +400,11 @@ class TestCopyPage(TestCase):
         # Check that the speakers weren't removed from old page
         self.assertEqual(christmas_event.speakers.count(), 1, "Child objects were removed from the original page")
 
+        # Check that advert placements were also copied (there's a gotcha here, since the advert_placements
+        # relation is defined on Page, not EventPage)
+        self.assertEqual(new_christmas_event.advert_placements.count(), 1, "Child objects defined on the superclass weren't copied")
+        self.assertEqual(christmas_event.advert_placements.count(), 1, "Child objects defined on the superclass were removed from the original page")
+
     def test_copy_page_copies_child_objects_with_nonspecific_class(self):
         # Get chrismas page as Page instead of EventPage
         christmas_event = Page.objects.get(url_path='/home/events/christmas/')

wagtail/wagtailsnippets/tests.py

@@ -195,7 +195,7 @@ class TestUsageCount(TestCase):
     @override_settings(WAGTAIL_USAGE_COUNT_ENABLED=True)
     def test_snippet_usage_count(self):
         advert = Advert.objects.get(id=1)
-        self.assertEqual(advert.get_usage().count(), 1)
+        self.assertEqual(advert.get_usage().count(), 2)
 
 
 class TestUsedBy(TestCase):