Account for deletions when constructing page paths for preview_on_create - fixes #2599

CHANGELOG.txt

@@ -59,6 +59,7 @@ Changelog
  * Fix: The "unsaved changes" confirmation was erroneously shown on IE / Firefox when previewing a page with validation errors (Matt Westcott)
  * Fix: The up / down / delete controls on the "Promoted search results" form no longer trigger a form submission (Matt Westcott)
  * Fix: Opening preview window no longer performs user-agent sniffing, and now works correctly on IE11 (Matt Westcott)
+ * Fix: Tree paths are now correctly assigned when previewing a newly-created page underneath a parent with deleted children (Matt Westcott)
 
 
 1.4.4 (10.05.2016)

docs/releases/1.4.5.rst

@@ -18,3 +18,4 @@ Bug fixes
  * The "unsaved changes" confirmation was erroneously shown on IE / Firefox when previewing a page with validation errors (Matt Westcott)
  * The up / down / delete controls on the "Promoted search results" form no longer trigger a form submission (Matt Westcott)
  * Opening preview window no longer performs user-agent sniffing, and now works correctly on IE11 (Matt Westcott)
+ * Tree paths are now correctly assigned when previewing a newly-created page underneath a parent with deleted children (Matt Westcott)

wagtail/wagtailadmin/tests/test_pages_views.py

@@ -2917,3 +2917,42 @@ class TestRevisions(TestCase, WagtailTestUtils):
         # Buttons should be relabelled
         self.assertContains(response, "Replace current draft")
         self.assertContains(response, "Publish this revision")
+
+
+class TestIssue2599(TestCase, WagtailTestUtils):
+    """
+    When previewing a page on creation, we need to assign it a path value consistent with its
+    (future) position in the tree. The naive way of doing this is to give it an index number
+    one more than numchild - however, index numbers are not reassigned on page deletion, so
+    this can result in a path that collides with an existing page (which is invalid).
+    """
+    def test_issue_2599(self):
+        homepage = Page.objects.get(id=2)
+
+        child1 = Page(title='child1')
+        homepage.add_child(instance=child1)
+        child2 = Page(title='child2')
+        homepage.add_child(instance=child2)
+
+        child1.delete()
+
+        self.login()
+        post_data = {
+            'title': "New page!",
+            'content': "Some content",
+            'slug': 'hello-world',
+            'action-submit': "Submit",
+        }
+        response = self.client.post(
+            reverse('wagtailadmin_pages:preview_on_add', args=('tests', 'simplepage', homepage.id)), post_data
+        )
+
+        # Check the response
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'tests/simple_page.html')
+        self.assertContains(response, "New page!")
+
+        # Check that the treebeard attributes were set correctly on the page object
+        self.assertEqual(response.context['self'].depth, homepage.depth + 1)
+        self.assertTrue(response.context['self'].path.startswith(homepage.path))
+        self.assertEqual(response.context['self'].get_parent(), homepage)

wagtail/wagtailadmin/views/pages.py

@@ -558,7 +558,13 @@ def preview_on_create(request, content_type_app_name, content_type_model_name, p
         # calling treebeard's internal _get_path method, we can set a 'realistic' value that
         # will hopefully enable tree traversal operations to at least partially work.
         page.depth = parent_page.depth + 1
-        page.path = page._get_path(parent_page.path, page.depth, parent_page.numchild + 1)
+
+        if parent_page.is_leaf():
+            # set the path as the first child of parent_page
+            page.path = page._get_path(parent_page.path, page.depth, 1)
+        else:
+            # add the new page after the last child of parent_page
+            page.path = parent_page.get_last_child()._inc_path()
 
         # ensure that our unsaved page instance has a suitable url set
         page.set_url_path(parent_page)