django-cms / django-filer

File and Image Management Application for django
https://django-filer.readthedocs.io/
Other
1.76k stars 577 forks source link

Crash when moving files from a filtered directory listing in the admin #1479

Closed W1ldPo1nter closed 2 months ago

W1ldPo1nter commented 3 months ago

The following issue occurs in the folder admin: if a directory listing is filtered via the search field (leading to the q query parameter being set) and then the "move" action (internal name: move_files_and_folders) is used on at least one of the files, the following error is raised:

Cannot resolve keyword 'coalesce_sort_field' into field. Choices are: _file_size, bootstrap4carouselslide, bootstrap4link, buttonpluginmodel, clipboarditem, content_customimage_file, description, file, folder, folder_id, has_all_mandatory_data, id, in_clipboards, is_public, link, mime_type, modified_at, name, original_filename, owner, owner_id, polymorphic_ctype, polymorphic_ctype_id, sha1, uploaded_at

This is an annotated field that is only added/used in a single place. I didn't check why this happens in depth, but have made the following observations:

A simple fix that wouldn't require an in-depth investigation could be to not use .annotate() at all and pass the entire annotation to .order_by() instead by changing this part:

        if order_by is None:
            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')

to:

        if order_by is None:
            order_by_annotation = Lower(Coalesce(
                Case(
                    When(name__exact='', then=None),
                    When(name__isnull=False, then='name')
                ),
                'original_filename'
            ))

That way, the ordering no longer relies on an annotation that is removed by Django. However, I don't know if that would work in all supported Django versions (I just quickly tested it with Django 4.2).

fsbraun commented 3 months ago

This looks like a manifest bug to me. 👍 Would you like to create a pull request to fix it, preferably with a small regression test?

W1ldPo1nter commented 2 months ago

I would like to, but I won't get around to it until the end of the month or beginning of August due to being on vacation. If the issue is still open at that point, I can prepare a PR.