Ordr collection listings by name (#4874)

CHANGELOG.txt

@@ -9,6 +9,7 @@ Changelog
  * Document and image choosers now show the document / image's collection (Alejandro Garza, Janneke Janssen)
  * Added new "Welcome to your Wagtail site" Starter Page when using wagtail start command (Timothy Allen, Scott Cranfill)
  * Added ability to run individual tests through tox (Benjamin Bach)
+ * Collection listings are now ordered by name (Seb Brown)
  * Fix: Query objects returned from `PageQuerySet.type_q` can now be merged with `|` (Brady Moe)
  * Fix: Add `rel="noopener noreferrer"` to target blank links (Anselm Bradford)
  * Fix: Additional fields on custom document models now show on the multiple document upload view (Robert Rollins, Sergey Fedoseev)

CONTRIBUTORS.rst

@@ -331,6 +331,7 @@ Contributors
 * gmmoraes
 * Justin Focus
 * Fedor Selitsky
+* Seb Brown
 
 Translators
 ===========

docs/releases/2.4.rst

@@ -27,6 +27,7 @@ Other features
  * Document and image choosers now show the document / image's collection (Alejandro Garza, Janneke Janssen)
  * New ``image_url`` template tag allows to generate dynamic image URLs, so image renditions are being created outside the main request which improves performance. Requires extra configuration, see :doc:`/advanced_topics/images/image_serve_view` (Yannick Chabbert, Dan Braghis).
  * Added ability to run individual tests through tox (Benjamin Bach)
+ * Collection listings are now ordered by name (Seb Brown)
 
 Bug fixes
 ~~~~~~~~~

wagtail/admin/tests/test_collections_views.py

@@ -32,6 +32,16 @@ class TestCollectionsIndexView(TestCase, WagtailTestUtils):
         self.assertNotContains(response, "No collections have been created.")
         self.assertContains(response, "Holiday snaps")
 
+    def test_ordering(self):
+        root_collection = Collection.get_first_root_node()
+        root_collection.add_child(name="Milk")
+        root_collection.add_child(name="Bread")
+        root_collection.add_child(name="Avacado")
+        response = self.get()
+        self.assertEqual(
+            [collection.name for collection in response.context['object_list']],
+            ['Avacado', 'Bread', 'Milk'])
+
 
 class TestAddCollection(TestCase, WagtailTestUtils):
     def setUp(self):

wagtail/admin/views/collections.py

@@ -22,7 +22,7 @@ class Index(IndexView):
 
     def get_queryset(self):
         # Only return children of the root node, so that the root is not editable
-        return Collection.get_first_root_node().get_children()
+        return Collection.get_first_root_node().get_children().order_by('name')
 
 
 class Create(CreateView):
@@ -59,7 +59,7 @@ class Edit(EditView):
 
     def get_queryset(self):
         # Only return children of the root node, so that the root is not editable
-        return Collection.get_first_root_node().get_children()
+        return Collection.get_first_root_node().get_children().order_by('name')
 
 
 class Delete(DeleteView):
@@ -74,7 +74,7 @@ class Delete(DeleteView):
 
     def get_queryset(self):
         # Only return children of the root node, so that the root is not editable
-        return Collection.get_first_root_node().get_children()
+        return Collection.get_first_root_node().get_children().order_by('name')
 
     def get_collection_contents(self):
         collection_contents = [

wagtail/core/models.py

@@ -13,7 +13,7 @@ from django.core.exceptions import ValidationError
 from django.core.handlers.base import BaseHandler
 from django.core.handlers.wsgi import WSGIRequest
 from django.db import models, transaction
-from django.db.models import Q, Value
+from django.db.models import Case, Q, Value, When
 from django.db.models.functions import Concat, Substr
 from django.http import Http404
 from django.template.response import TemplateResponse
@@ -1992,6 +1992,14 @@ class Collection(MP_Node):
         """Return a query set of all collection view restrictions that apply to this collection"""
         return CollectionViewRestriction.objects.filter(collection__in=self.get_ancestors(inclusive=True))
 
+    @staticmethod
+    def order_for_display(queryset):
+        return queryset.annotate(
+            display_order=Case(
+                When(depth=1, then=Value('')),
+                default='name')
+        ).order_by('display_order')
+
     class Meta:
         verbose_name = _('collection')
         verbose_name_plural = _('collections')

wagtail/core/permission_policies/collections.py

@@ -67,7 +67,7 @@ class CollectionPermissionLookupMixin:
             for path in collection_root_paths[1:]:
                 collection_path_filter = collection_path_filter | Q(path__startswith=path)
 
-            return Collection.objects.filter(collection_path_filter)
+            return Collection.objects.all().filter(collection_path_filter)
         else:
             # no matching collections
             return Collection.objects.none()

wagtail/documents/tests/test_admin_views.py

@@ -88,12 +88,16 @@ class TestDocumentIndexView(TestCase, WagtailTestUtils):
     def test_index_with_collection(self):
         root_collection = Collection.get_first_root_node()
         root_collection.add_child(name="Evil plans")
+        root_collection.add_child(name="Good plans")
 
         self.make_docs()
 
         response = self.client.get(reverse('wagtaildocs:index'))
         self.assertContains(response, '<th>Collection</th>')
         self.assertContains(response, '<td>Root</td>')
+        self.assertEqual(
+            [collection.name for collection in response.context['collections']],
+            ['Root', 'Evil plans', 'Good plans'])
 
 
 class TestDocumentAddView(TestCase, WagtailTestUtils):

wagtail/documents/views/chooser.py

@@ -91,6 +91,8 @@ def chooser(request):
         collections = Collection.objects.all()
         if len(collections) < 2:
             collections = None
+        else:
+            collections = Collection.order_for_display(collections)
 
         documents = documents.order_by('-created_at')
         documents_exist = documents.exists()

wagtail/documents/views/documents.py

@@ -63,6 +63,8 @@ def index(request):
     )
     if len(collections) < 2:
         collections = None
+    else:
+        collections = Collection.order_for_display(collections)
 
     # Create response
     if request.is_ajax():

wagtail/documents/views/multiple.py

@@ -7,6 +7,7 @@ from django.views.decorators.http import require_POST
 from django.views.decorators.vary import vary_on_headers
 
 from wagtail.admin.utils import PermissionPolicyChecker
+from wagtail.core.models import Collection
 from wagtail.search.backends import get_search_backends
 
 from ..forms import get_document_form, get_document_multi_form
@@ -25,7 +26,7 @@ def add(request):
 
     collections = permission_policy.collections_user_has_permission_for(request.user, 'add')
     if len(collections) > 1:
-        collections_to_choose = collections
+        collections_to_choose = Collection.order_for_display(collections)
     else:
         # no need to show a collections chooser
         collections_to_choose = None

wagtail/images/tests/test_admin_views.py

@@ -75,6 +75,16 @@ class TestImageIndexView(TestCase, WagtailTestUtils):
             response = self.get({'ordering': ordering})
             self.assertEqual(response.status_code, 200)
 
+    def test_collection_order(self):
+        root_collection = Collection.get_first_root_node()
+        root_collection.add_child(name="Evil plans")
+        root_collection.add_child(name="Good plans")
+
+        response = self.get()
+        self.assertEqual(
+            [collection.name for collection in response.context['collections']],
+            ['Root', 'Evil plans', 'Good plans'])
+
 
 class TestImageAddView(TestCase, WagtailTestUtils):
     def setUp(self):

wagtail/images/views/chooser.py

@@ -52,6 +52,8 @@ def get_chooser_context(request):
     collections = Collection.objects.all()
     if len(collections) < 2:
         collections = None
+    else:
+        collections = Collection.order_for_display(collections)
 
     return {
         'searchform': SearchForm(),

wagtail/images/views/images.py

@@ -61,6 +61,8 @@ def index(request):
     )
     if len(collections) < 2:
         collections = None
+    else:
+        collections = Collection.order_for_display(collections)
 
     # Create response
     if request.is_ajax():

wagtail/images/views/multiple.py

@@ -7,6 +7,7 @@ from django.views.decorators.http import require_POST
 from django.views.decorators.vary import vary_on_headers
 
 from wagtail.admin.utils import PermissionPolicyChecker
+from wagtail.core.models import Collection
 from wagtail.images import get_image_model
 from wagtail.images.fields import ALLOWED_EXTENSIONS
 from wagtail.images.forms import get_image_form
@@ -42,7 +43,7 @@ def add(request):
 
     collections = permission_policy.collections_user_has_permission_for(request.user, 'add')
     if len(collections) > 1:
-        collections_to_choose = collections
+        collections_to_choose = Collection.order_for_display(collections)
     else:
         # no need to show a collections chooser
         collections_to_choose = None