Annotate admin_order_field to also accept a BaseExpression

[brianhelba]

Django's docs state:

Query expressions may be used in admin_order_field.

Internally, Django looks for the property resolve_expression on non-string admin_order_field values. This is provided by BaseExpression.

[coveralls]

Pull Request Test Coverage Report for Build 347679241

• 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 323194397:	0.0%
Covered Lines:	22
Relevant Lines:	22

---

💛 - Coveralls

[escaped]

Nice find @brianhelba! Thank you.

Implementation-wise it looks great, but I think the new functionality should be covered by the tests as well. Would you mind integrating the new case into the tests? If you need any help please let me know.

(...and while you at it, don't forget to add yourself to the CONTRIBUTORS.md :) )

[brianhelba]

@escaped Thanks for the quick review and feedback.

• I've added myself to CONTRIBUTORS.md
• I've added a sample usage to the tests, using the case from the Django docs

[brianhelba]

@escaped The tests are failing, but it looks to me like it might be an issue with master, not this branch. I'm not sure though.

[escaped]

Nice work. Some minor changes and the PR is ready to be merged.

I inspected the output of the CI pipeline and it looks like we need to make some minor adjustments to make it pass.
Furthermore, yes, there is an issue with master which states that isort is not configured correctly. I have to fix this in another project, but as it is just a warning and it should not affect this PR.

Please have a look at my comments and let me know if you need any help. Thank you.

[brianhelba]

@escaped Thanks for all the guidance.

The good news is that I found a way to further simplify the tests. Using repr for injected values is simpler than the special case of having to explicitly wrap string values in quotes.

The bad news is that Django's use of repr for query expressions is broken, as the returned value does not properly quote parameters. So, I had to add another special case to fix this.

[escaped]

Great work. Thank you, it's ready for merging :) Will definitely publish it this week (even though I left one discussion point).

[escaped]

Nice work! Today I learned something new :)

I like the solution; it's clever, but maybe too clever in a way that it will be hard for others to understand what's going on. So, I am a bit on a fence, and maybe we should just be explicit and redefine everything instead of importing the OPTIONS.

OPTIONS = [
    ('admin_order_field', '"radius"'),
    ('admin_order_field', 'Lower(F("person_name"))'),
    ('boolean', 'True'),
    ('empty_value_display', '"Undefined"'),
    ('short_description', '"Is big?"'),
]
if django.VERSION[:2] <= (1, 11):
    OPTIONS.append(('allow_tags', "True"))

I know you put a lot of effort into solving this issue, so I am curious to hear what you think about this.