Re-add alphabetical sorting as default (fixes #1415)

[filipweidemann]

Description

This PR intends to hotfix #1415 by reintroducing the .sort() function on file_qs inside the filer/admin/folderadmin.py logic.

We did not modify any existing tests or added new ones because the function is returning rendered HTML and it is currently unclear how we could test this in an elegant way.

We are also aware that there are better ways to accomplish the same result, e.g. by using Django's QuerySet.annotate and Coalesce DB functions to add a correct sort target to the queryset, so that .order_by() works as intended.

However, we did not want to refactor the existing solution right now but rather hotfix the (currently broken) behaviour.

Related resources

• See issue #1415
• https://docs.djangoproject.com/en/4.2/topics/db/aggregation/
• https://docs.djangoproject.com/en/4.2/ref/models/database-functions/#coalesce

Checklist

• [x] I have opened this pull request against master
• [x] I have added or modified the tests when changing logic
• [x] I have followed the conventional commits guidelines to add meaningful information into the changelog
• [x] I have read the contribution guidelines and I have joined #workgroup-pr-review on Slack to find a “pr review buddy” who is going to review my pull request.

[codecov[bot]]

Codecov Report

Patch coverage: 87.50% and project coverage change: +0.20% :tada:

Comparison is base (3940e1c) 75.00% compared to head (07a1e0f) 75.21%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1416 +/- ## ========================================== + Coverage 75.00% 75.21% +0.20% ========================================== Files 75 75 Lines 3449 3454 +5 Branches 554 555 +1 ========================================== + Hits 2587 2598 +11 + Misses 694 691 -3 + Partials 168 165 -3 ``` | [Files Changed](https://app.codecov.io/gh/django-cms/django-filer/pull/1416?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=django-cms) | Coverage Δ | | |---|---|---| | [filer/admin/folderadmin.py](https://app.codecov.io/gh/django-cms/django-filer/pull/1416?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=django-cms#diff-ZmlsZXIvYWRtaW4vZm9sZGVyYWRtaW4ucHk=) | `75.65% <87.50%> (+0.46%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/django-cms/django-filer/pull/1416/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=django-cms)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[fsbraun]

@filipweidemann Thanks for creating this PR! Just a quick question: Since you seem to have run black it is hard for me to identify the actual content changes. I assume it's the file_list, its sorting and passing the file_list into the context if it exists. Did I miss anything?

Can you add a test that verifies files are sorted? This feature got lost since a test did not cover it. It would be great if we fixed this for the future.

[filipweidemann]

@fsbraun okay good catch, running black was not intentional, I just use it for everything by default and forgot to turn it off. I'll fix that and try to add some tests as well.

And yes, file_list is basically all of the fix, we just have to make sure that it'll only be created when there is no explicit order_by set, so it is wrapped inside a conditional check and only gets passed into the context of the render function when it is explicitly used, else we just use file_qs.

[filipweidemann]

Okay @fsbraun, here we go.

I reverted the black formatting and added some tests with the expected sort behaviour.

I really was not happy with using QuerySet objects by default and falling back to a list just to do the sorting inside the Python code.

So I did a quick refactoring and implemented the solution I already talked about inside the PR description. Currently we are using Coalesce to create a temporary coalesce_sort_field that priorititzes the name field and falls back to the original_filename for all objects without a set name. I used Case to catch the empty strings of the name field since the default behaviour of Coalesce does not interpret empty strings and NoneType equally, but we want to fall back on empty strings as well.

Would be nice if you could have a quick look & maybe kick off the CI again.

If something is still unclear or needs another refactor, let me know. Thanks.

[filipweidemann]

Hey, quick question: when will this get merged & released? I don't want to pressure anyone but a quick estimate would be nice because right now I'm thinking about shipping the fork to prod environments to bridge the gap until the release is there.

But I would obviously avoid that if you're saying that this will be released this week or something :)

Thanks!

[fsbraun]

@filipweidemann Will release it the next days as 3.0.6. (Merged right now. - I typically do not merge immediately after reviewing to give other community members a chance of looking at it.)

[filipweidemann]

Great stuff, that's all I needed to know. :) Thanks for explaining the workflow and handling the release!