diff --git a/filer/admin/folderadmin.py b/filer/admin/folderadmin.py index 822e3aa7d..4bd0a5bc9 100644 --- a/filer/admin/folderadmin.py +++ b/filer/admin/folderadmin.py @@ -13,7 +13,8 @@ from django.core.exceptions import PermissionDenied, ValidationError from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator from django.db import models, router -from django.db.models import F, OuterRef, Subquery +from django.db.models import F, OuterRef, Subquery, Case, When +from django.db.models.functions import Coalesce, Lower from django.http import HttpResponse, HttpResponseRedirect from django.shortcuts import get_object_or_404, render from django.urls import path, reverse @@ -317,13 +318,24 @@ def directory_listing(self, request, folder_id=None, viewtype=None): folder_qs = folder_qs.order_by('name').select_related("owner") order_by = request.GET.get('order_by', None) + order_by_annotation = None if order_by is None: - order_by = "file" - order_by = order_by.split(',') + file_qs = file_qs.annotate(coalesce_sort_field=Coalesce( + Case( + When(name__exact='', then=None), + When(name__isnull=False, then='name') + ), + 'original_filename' + )) + order_by_annotation = Lower('coalesce_sort_field') + + order_by = order_by.split(',') if order_by else [] order_by = [field for field in order_by if re.sub(r'^-', '', field) in self.order_by_file_fields] if len(order_by) > 0: file_qs = file_qs.order_by(*order_by) + elif order_by_annotation: + file_qs = file_qs.order_by(order_by_annotation) if folder.is_root and not search_mode: virtual_items = folder.virtual_folders diff --git a/tests/test_admin.py b/tests/test_admin.py index 67f2f4ac2..c4d22bfab 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -748,6 +748,94 @@ def create_file(self, folder, filename=None): return file_obj +class FolderAndFileSortingMixin(BulkOperationsMixin): + def setUp(self): + self.superuser = create_superuser() + self.client.login(username='admin', password='secret') + self.img = create_image() + self.folder_1 = Folder(name='Pictures', parent=None) + self.folder_1.save() + self.nested_folder_2 = Folder(name='Nested 2', parent=self.folder_1) + self.nested_folder_1 = Folder(name='Nested 1', parent=self.folder_1) + self.nested_folder_1.save() + self.nested_folder_2.save() + self.create_file(folder=self.folder_1, filename='background.jpg') + self.create_file(folder=self.folder_1, filename='A_Testfile.jpg') + self.create_file(folder=self.folder_1, filename='Another_Test.jpg') + newspaper_file = self.create_file(folder=self.folder_1, filename='Newspaper.pdf') + newspaper_file.name = 'Zeitung' + newspaper_file.save() + renamed_file = self.create_file(folder=self.folder_1, filename='last_when_sorting_by_filename.jpg') + renamed_file.name = 'A cute dog' + renamed_file.save() + + def tearDown(self): + self.client.logout() + for f in File.objects.all(): + f.delete() + for folder in Folder.objects.all(): + folder.delete() + + +class FilerFolderAndFileSortingTests(FolderAndFileSortingMixin, TestCase): + # Assert that the folders are correctly sorted + def test_filer_folder_sorting(self): + response = self.client.get(reverse('admin:filer-directory_listing', kwargs={ + 'folder_id': self.folder_1.pk + })) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.context['folder_children'].count(), 2) + self.assertEqual(response.context['folder_children'][0].name, 'Nested 1') + self.assertEqual(response.context['folder_children'][1].name, 'Nested 2') + + # Default sorting should be alphabetically + def test_filer_directory_listing_default_sorting(self): + response = self.client.get(reverse('admin:filer-directory_listing', kwargs={ + 'folder_id': self.folder_1.pk + })) + self.assertEqual(response.status_code, 200) + # when using the default sort, the folder_files are of type `list`, + # so we assert the length. + self.assertEqual(len(response.context['folder_files']), 5) + + expected_filenames = ['A cute dog', 'A_Testfile.jpg', 'Another_Test.jpg', 'background.jpg', 'Zeitung'] + for index, expected_filename in enumerate(expected_filenames): + self.assertEqual(str(response.context['folder_files'][index]), expected_filename) + + # Now, all columns with empty name should be alphabetically sorted by their filename, + # after that, at the end of the list, all files with and explicit name should appear; + # however, since we ONLY sort by name, the order of items without name is not defined + # by their filename but rather by their creation date. + # So, the order is expected to be ordered as they are created in the setUp method. + def test_filer_directory_listing_sorting_with_order_by_param(self): + response = self.client.get(reverse('admin:filer-directory_listing', kwargs={ + 'folder_id': self.folder_1.pk + }), {'order_by': 'name'}) + self.assertEqual(response.status_code, 200) + # when using the default sort, the folder_files are of type `list`, + # so we assert the length. + self.assertEqual(len(response.context['folder_files']), 5) + + expected_filenames = ['background.jpg', 'A_Testfile.jpg', 'Another_Test.jpg', 'A cute dog', 'Zeitung'] + for index, expected_filename in enumerate(expected_filenames): + self.assertEqual(str(response.context['folder_files'][index]), expected_filename) + + # Finally, we can define a fallback column to pass into `order_by` so that files without + # any name are still sorted by something (in this case, their original_filename). + # This should yield the expected order as well, but NOT the exact same order as the default sorting, + # since we sort by name FIRST and all items with the same value again by original_filename. + def test_filer_directory_listing_sorting_with_multiple_order_by_params(self): + response = self.client.get(reverse('admin:filer-directory_listing', kwargs={ + 'folder_id': self.folder_1.pk + }), {'order_by': 'name,original_filename'}) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.context['folder_files']), 5) + + expected_filenames = ['A_Testfile.jpg', 'Another_Test.jpg', 'background.jpg', 'A cute dog', 'Zeitung'] + for index, expected_filename in enumerate(expected_filenames): + self.assertEqual(str(response.context['folder_files'][index]), expected_filename) + + class FilerBulkOperationsTests(BulkOperationsMixin, TestCase): def test_move_files_and_folders_action(self): # TODO: Test recursive (files and folders tree) move