From 006793597133ecc542b4ca7a6338cb9f0e878963 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Thu, 30 Oct 2014 10:23:35 +0000 Subject: [PATCH 1/2] Added failing test for #735 --- wagtail/wagtailcore/tests/test_page_model.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/wagtail/wagtailcore/tests/test_page_model.py b/wagtail/wagtailcore/tests/test_page_model.py index 0b44e2ba04..8e767c730d 100644 --- a/wagtail/wagtailcore/tests/test_page_model.py +++ b/wagtail/wagtailcore/tests/test_page_model.py @@ -600,3 +600,23 @@ class TestSubpageTypeBusinessRules(TestCase): # BusinessSubIndex only allows BusinessIndex as a parent self.assertNotIn(ContentType.objects.get_for_model(SimplePage), BusinessSubIndex.allowed_parent_page_types()) self.assertIn(ContentType.objects.get_for_model(BusinessIndex), BusinessSubIndex.allowed_parent_page_types()) + + +class TestIssue735(TestCase): + """ + Issue 735 reports that URL paths of child pages are not + updated correctly when slugs of parent pages are updated + """ + fixtures = ['test.json'] + + def test_child_urls_updated_on_parent_publish(self): + event_index = Page.objects.get(url_path='/home/events/') + christmas_event = EventPage.objects.get(url_path='/home/events/christmas/') + + # Change the event index slug and publish it + event_index.slug = 'old-events' + event_index.save_revision().publish() + + # Check that the christmas events url path updated correctly + new_christmas_event = EventPage.objects.get(id=christmas_event.id) + self.assertEqual(new_christmas_event.url_path, '/home/old-events/christmas/') From 5db1539e0adf7b4fbd8292cf96de4f74bb19df0c Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Thu, 30 Oct 2014 10:44:05 +0000 Subject: [PATCH 2/2] Don't update descendant urls if slug not updated Fixes #735 Previously, if you called Page.save with update_fields and didn't include 'slug', it would still attempt to update URL paths of descendant pages. Wagtail 0.7 introduced a feature where it saves the latest revisions created at date on the Page. This requires a Page.save call which used update_fields. The bug caused update_descendant_paths to be called anyway even though the slug wasn't committed to the database. This caused Issue 735 because publishing a page will both save a revision and save a page causing two hits to update_descendant_paths. Running this method twice caused all descendant pages url_path attributes to get mangled. --- wagtail/wagtailcore/models.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 43f09d5b3b..801a35534a 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -337,14 +337,17 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, index.Indexed # has been set and so we can safely call get_parent self.set_url_path(self.get_parent()) else: - # see if the slug has changed from the record in the db, in which case we need to - # update url_path of self and all descendants - old_record = Page.objects.get(id=self.id) - if old_record.slug != self.slug: - self.set_url_path(self.get_parent()) - update_descendant_url_paths = True - old_url_path = old_record.url_path - new_url_path = self.url_path + # Check that we are committing the slug to the database + # Basically: If update_fields has been specified, and slug is not included, skip this step + if not ('update_fields' in kwargs and 'slug' not in kwargs['update_fields']): + # see if the slug has changed from the record in the db, in which case we need to + # update url_path of self and all descendants + old_record = Page.objects.get(id=self.id) + if old_record.slug != self.slug: + self.set_url_path(self.get_parent()) + update_descendant_url_paths = True + old_url_path = old_record.url_path + new_url_path = self.url_path result = super(Page, self).save(*args, **kwargs)