manager: fix page ordering with missing page from arguments (#55288)

combo/manager/views.py

@@ -22,7 +22,8 @@ from operator import attrgetter
 
 from django.conf import settings
 from django.contrib import messages
-from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
+from django.core.exceptions import ObjectDoesNotExist
+from django.db import transaction
 from django.http import Http404, HttpResponse, HttpResponseRedirect
 from django.shortcuts import get_object_or_404, redirect, render
 from django.urls import reverse, reverse_lazy
@@ -704,12 +705,19 @@ def page_order(request):
     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
+    try:
+        with transaction.atomic():
+            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
+    except ValueError:
+        # missing child page in new_order, fail silently
+        return redirect(reverse('combo-manager-homepage'))
+
     if slug_conflict:
         # slug conflict after a page got moved, reload and rename
         moved_page = Page.objects.get(id=request.GET['moved-page-id'])

tests/test_manager.py

@@ -679,13 +679,26 @@ def test_page_reorder(app, admin_user):
     page3.save()
     page4 = Page(title='Four', slug='four', parent=page2, order=3, template_name='standard')
     page4.save()
+    app = login(app)
 
     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]
+    assert ordered_ids == [page1.id, page2.id, page3.id, page4.id]
+
+    # missing page3 in order
+    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 [page2.id, page1.id, page4.id]]),
+        },
+    )
+    # not changed
+    ordered_ids = [x.id for x in Page.get_as_reordered_flat_hierarchy(Page.objects.all())]
+    assert ordered_ids == [page1.id, page2.id, page3.id, page4.id]
 
     # move page4 before page3
-    app = login(app)
-    resp = app.get(
+    app.get(
         '/manage/pages/order',
         params={
             'moved-page-id': page4.id,
@@ -695,10 +708,10 @@ def test_page_reorder(app, admin_user):
     )
 
     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]
+    assert ordered_ids == [page1.id, page2.id, page4.id, page3.id]
 
     # move page4 to level0
-    resp = app.get(
+    app.get(
         '/manage/pages/order',
         params={
             'moved-page-id': page4.id,
@@ -707,13 +720,13 @@ def test_page_reorder(app, admin_user):
         },
     )
     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]
+    assert 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(
+    app.get(
         '/manage/pages/order',
         params={
             'moved-page-id': page4.id,
@@ -731,7 +744,7 @@ def test_page_reorder(app, admin_user):
     page2.save()
     page3.parent = page2
     page3.save()
-    resp = app.get(
+    app.get(
         '/manage/pages/order',
         params={
             'moved-page-id': page1.id,
@@ -750,7 +763,7 @@ def test_page_reorder(app, admin_user):
     page3.save()
     page4.parent = page3
     page4.save()
-    resp = app.get(
+    app.get(
         '/manage/pages/order',
         params={
             'moved-page-id': page2.id,