misc: allow page with same slug but different parents (#8521)

combo/data/models.py

@@ -172,7 +172,7 @@ class Page(models.Model):
             i = 1
             while True:
                 try:
-                    Page.objects.get(slug=slug)
+                    Page.objects.get(slug=slug, parent_id=self.parent_id)
                 except ObjectDoesNotExist:
                     break
                 i += 1

combo/manager/forms.py

@@ -38,7 +38,7 @@ class PageEditSlugForm(forms.ModelForm):
         value = self.cleaned_data.get('slug')
         if self.instance.slug == value:
             return value
-        if Page.objects.filter(slug=value).count() > 0:
+        if Page.objects.filter(slug=value, parent_id=self.instance.parent_id).count() > 0:
             raise ValidationError(_('Slug must be unique'), code='unique')
         return value
 

combo/manager/views.py

@@ -14,6 +14,7 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import hashlib
 import json
 import os
 
@@ -452,14 +453,23 @@ def cell_order(request, page_pk):
 def page_order(request):
     new_order = [int(x) for x in request.GET['new-order'].split(',')]
     moved_page = Page.objects.get(id=request.GET['moved-page-id'])
+    current_parent_id = moved_page.parent_id
     if request.GET['moved-page-new-parent']:
         moved_page.parent_id = request.GET['moved-page-new-parent']
     else:
         moved_page.parent_id = None
     moved_page.save()
+    slug_conflict = False
     for page in Page.objects.filter(parent_id=moved_page.parent_id):
         page.order = new_order.index(page.id)+1
         page.save()
+        if moved_page.id != page.id and moved_page.slug == page.slug:
+            slug_conflict = True
+    if slug_conflict:
+        # slug conflict after a page got moved, reload and rename
+        moved_page = Page.objects.get(id=request.GET['moved-page-id'])
+        moved_page.slug = moved_page.slug + '-' + hashlib.md5(str(moved_page.id)).hexdigest()[:4]
+        moved_page.save()
     return redirect(reverse('combo-manager-homepage'))
 
 

combo/public/views.py

@@ -350,12 +350,24 @@ def page(request):
             request.session['visited'] = True
             return HttpResponseRedirect(settings.COMBO_WELCOME_PAGE_PATH)
 
+    slugs = {'parent__'*len(parts) + 'isnull': True}
+    for i, part in enumerate(reversed(parts)):
+        slugs['parent__'*i + 'slug'] = part
     try:
-        page = Page.objects.get(slug=parts[-1])
+        page = Page.objects.get(**slugs)
     except Page.DoesNotExist:
         if Page.objects.count() == 0 and parts == ['index']:
             return empty_site(request)
-        page = None
+        # maybe the page is a children of /index/, as /index/ is silent the
+        # page would appear directly under /; this is not a suggested practice.
+        parts = ['index'] + parts
+        slugs = {'parent__'*len(parts) + 'isnull': True}
+        for i, part in enumerate(reversed(parts)):
+            slugs['parent__'*i + 'slug'] = part
+        try:
+            page = Page.objects.get(**slugs)
+        except Page.DoesNotExist:
+            page = None
 
     if page is None or page.get_online_url() != url:
         if not url.endswith('/') and settings.APPEND_SLASH:

tests/test_manager.py

@@ -222,6 +222,48 @@ def test_delete_page_and_subpage(app, admin_user):
     assert resp.location.endswith('/manage/')
     assert Page.objects.count() == 0
 
+def test_page_reorder(app, admin_user):
+    Page.objects.all().delete()
+    page1 = Page(title='One', slug='one', parent=None, order=0, template_name='standard')
+    page1.save()
+    page2 = Page(title='Two', slug='two', parent=None, order=1, template_name='standard')
+    page2.save()
+    page3 = Page(title='Three', slug='three', parent=page2, order=2, template_name='standard')
+    page3.save()
+    page4 = Page(title='Four', slug='four', parent=page2, order=3, template_name='standard')
+    page4.save()
+
+    ordered_ids = [x.id for x in Page.get_as_reordered_flat_hierarchy(Page.objects.all())]
+    ordered_ids = [page1.id, page2.id, page3.id, page4.id]
+
+    # move page4 before page3
+    app = login(app)
+    resp = app.get('/manage/pages/order', params={
+        'moved-page-id': page4.id,
+        'moved-page-new-parent': page2.id,
+        'new-order': ','.join([str(x) for x in [page1.id, page2.id, page4.id, page3.id]])})
+
+    ordered_ids = [x.id for x in Page.get_as_reordered_flat_hierarchy(Page.objects.all())]
+    ordered_ids = [page1.id, page2.id, page4.id, page3.id]
+
+    # move page4 to level0
+    resp = app.get('/manage/pages/order', params={
+        'moved-page-id': page4.id,
+        'moved-page-new-parent': '',
+        'new-order': ','.join([str(x) for x in [page1.id, page4.id, page2.id, page3.id]])})
+    ordered_ids = [x.id for x in Page.get_as_reordered_flat_hierarchy(Page.objects.all())]
+    ordered_ids = [page1.id, page4.id, page2.id, page3.id]
+
+    # change slug to check for autochange on duplicate
+    page4.slug = 'three'
+    page4.save()
+    # move it as a sibling of page3
+    resp = app.get('/manage/pages/order', params={
+        'moved-page-id': page4.id,
+        'moved-page-new-parent': page2.id,
+        'new-order': ','.join([str(x) for x in [page1.id, page2.id, page4.id, page3.id]])})
+    assert Page.objects.get(id=page4.id).slug.startswith('three-')
+
 def test_export_page(app, admin_user):
     Page.objects.all().delete()
     page = Page(title='One', slug='one', template_name='standard')