Fixed #6307 -- Don't add a leading / character to url paths (#6311)

This only affected moving / publishing the homepage.

CHANGELOG.txt

@@ -3,6 +3,8 @@
 * Fixed a bug where shortcuts menu entry would stop working after toolbar reload
 * Fixed a race condition in frontend code that could lead to sideframe being
   opened with blank page
+* Fixed a bug where the direct children of the homepage would get a leading ``/``
+  character when the homepage was moved or published.
 
 
 === 3.5.1 (2018-03-05) ===

cms/models/pagemodel.py

@@ -283,8 +283,7 @@ def _update_title_path(self, language):
             base = ''
 
         title_obj = self.get_title_obj(language, fallback=False)
-        new_path = title_obj.get_path_for_base(base)
-        title_obj.path = new_path
+        title_obj.path = title_obj.get_path_for_base(base)
         title_obj.save()
 
     def _update_title_path_recursive(self, language):
@@ -294,14 +293,20 @@ def _update_title_path_recursive(self, language):
         if self.node.is_leaf() or language not in self.get_languages():
             return
 
-        base = self.get_path(language, fallback=True)
         pages = self.get_child_pages()
+        base = self.get_path(language, fallback=True)
+
+        if base:
+            new_path = Concat(models.Value(base), models.Value('/'), models.F('slug'))
+        else:
+            # User is moving the homepage
+            new_path = models.F('slug')
 
         (Title
          .objects
          .filter(language=language, page__in=pages)
          .exclude(has_url_overwrite=True)
-         .update(path=Concat(models.Value(base), models.Value('/'), models.F('slug'))))
+         .update(path=new_path))
 
         for child in pages.filter(title_set__language=language).iterator():
             child._update_title_path_recursive(language)
@@ -635,15 +640,13 @@ def copy(self, site, parent_node=None, language=None,
 
             if parent_page:
                 base = parent_page.get_path(title.language)
-                path = '%s/%s' % (base, title.slug)
+                path = '%s/%s' % (base, title.slug) if base else title.slug
             else:
                 base = ''
                 path = title.slug
 
-            slug = get_available_slug(site, path, title.language)
-
-            title.slug = slug
-            title.path = '%s/%s' % (base, slug) if base else slug
+            title.slug = get_available_slug(site, path, title.language)
+            title.path = '%s/%s' % (base, title.slug) if base else title.slug
             title.save()
 
             new_page.title_cache[title.language] = title
@@ -1116,10 +1119,14 @@ def mark_descendants_as_published(self, language):
             publisher_public__published=True,
         )
 
+        if base:
+            new_path = Concat(models.Value(base), models.Value('/'), models.F('slug'))
+        else:
+            # User is moving the homepage
+            new_path = models.F('slug')
+
         # Update public title paths
-        unpublished_public.exclude(has_url_overwrite=True).update(
-            path=Concat(models.Value(base), models.Value('/'), models.F('slug'))
-        )
+        unpublished_public.exclude(has_url_overwrite=True).update(path=new_path)
 
         # Set unpublished pending titles to published
         unpublished_public.filter(published=False).update(published=True)

cms/tests/test_page_admin.py

@@ -594,6 +594,39 @@ def test_delete_page_confirmation(self):
                 page_markup = row_markup % (edit_url, page.get_title('en'))
                 self.assertContains(response, page_markup, html=True)
 
+    def test_publish_homepage_with_children(self):
+        homepage = create_page("home", "nav_playground.html", "en", published=True)
+        homepage.set_as_homepage()
+        pending_child_1 =  create_page(
+            "child-1",
+            "nav_playground.html",
+            language="en",
+            parent=homepage,
+            published=True,
+        )
+        pending_child_2 = create_page(
+            "child-2",
+            "nav_playground.html",
+            language="en",
+            parent=homepage,
+            published=True,
+        )
+        endpoint = self.get_admin_url(Page, 'publish_page', homepage.pk, 'en')
+        expected_tree = [
+            (homepage, ''),
+            (pending_child_1, 'child-1'),
+            (pending_child_2, 'child-2'),
+        ]
+
+        with self.login_user_context(self.get_superuser()):
+            self.client.post(endpoint)
+
+            for page, url_path in expected_tree:
+                self.assertPublished(page)
+                page._clear_internal_cache()
+                self.assertEqual(page.get_path('en'), url_path)
+                self.assertEqual(page.publisher_public.get_path('en'), url_path)
+
     def test_copy_page(self):
         """
         Test that a page can be copied via the admin
@@ -615,6 +648,18 @@ def test_copy_page(self):
 
         self.assertEqual(Page.objects.drafts().count() - count, 3)
 
+    def test_copy_page_under_home(self):
+        """
+        Users should be able to copy a page and paste under the home page.
+        """
+        homepage = create_page("home", "nav_playground.html", "en", published=True)
+        homepage.set_as_homepage()
+
+        root_page_a = create_page("root-a", "nav_playground.html", "en", published=True)
+
+        with self.login_user_context(self.get_superuser()):
+            self.copy_page(root_page_a, homepage)
+
     def test_copy_page_with_plugins(self):
         """
         Copying a page with plugins should copy all plugins for each translation
@@ -1099,6 +1144,50 @@ def test_move_page(self):
             page3 = Page.objects.get(pk=page3.pk)
             self.assertEqual(page3.get_path(), page_data2['slug'] + "/" + page_data3['slug'])
 
+    def test_move_home_page(self):
+        """
+        Users should be able to move the home-page
+        anywhere on the root of the tree.
+        """
+        homepage = create_page("home", "nav_playground.html", "en", published=True)
+        homepage.set_as_homepage()
+        home_child_1 = create_page(
+            "child-1",
+            "nav_playground.html",
+            language="en",
+            parent=homepage,
+            published=True,
+        )
+        home_child_2 = create_page(
+            "child-2",
+            "nav_playground.html",
+            language="en",
+            parent=homepage,
+            published=True,
+        )
+        home_sibling_1 = create_page("root-1", "nav_playground.html", "en", published=True)
+
+        expected_tree = [
+            # Sadly treebeard doesn't switch the paths
+            (home_sibling_1, '0002', 'root-1'),
+            (homepage, '0003', ''),
+            (home_child_1, '00030001', 'child-1'),
+            (home_child_2, '00030002', 'child-2'),
+        ]
+
+        with self.login_user_context(self.get_superuser()):
+            # Moves the homepage to the second position in the tree
+            data = {'id': homepage.pk, 'position': 1}
+            endpoint = self.get_admin_url(Page, 'move_page', homepage.pk)
+            response = self.client.post(endpoint, data)
+            self.assertEqual(response.status_code, 200)
+
+            for page, node_path, url_path in expected_tree:
+                page._clear_internal_cache()
+                self.assertEqual(page.node.path, node_path)
+                self.assertEqual(page.get_path('en'), url_path)
+                self.assertEqual(page.publisher_public.get_path('en'), url_path)
+
     def test_move_page_integrity(self):
         superuser = self.get_superuser()
         with self.login_user_context(superuser):

cms/utils/page.py

@@ -188,7 +188,6 @@ def get_page_from_request(request, use_path=None, clean_path=None):
 
 
 def get_all_pages_from_path(site, path, language):
-    path = path.strip('/')
     pages = get_pages_from_path(site, path, draft=True)
     pages |= get_pages_from_path(site, path, preview=True, draft=False)
     return pages.filter(title_set__language=language)