Fix admin sortable when using callable functions in list_display

[rgl]

We can use callable functions when we define the ModelAdmin.list_display property, but when we try to short then, an TypeError exception is raised:

Exception Type:		TypeError
Exception Value:	getattr(): attribute name must be string
Exception Location:	/home/rgl/Projects/django/django-trunk/django/contrib/admin/views/main.py in get_ordering, line 160

The attached patch fixes this.

This problem can be seen in the following ModelAdmin example; it uses two callables "admin_lat" and "admin_lng":

from django.contrib import admin
from airports.models import Airport

# See "callable" at http://docs.djangoproject.com/en/dev/ref/contrib/admin/#list-display
def admin_lat(airport):
    return '%.6f' % airport.lat
admin_lat.short_description = "Latitude"
admin_lat.admin_order_field = 'lat'
def admin_lng(airport):
    return '%.6f' % airport.lng
admin_lng.short_description = "Longitude"
admin_lng.admin_order_field = 'lng'

class AirportAdmin(admin.ModelAdmin):
    list_display = ('iata_code', 'name', admin_lat, admin_lng, 'tz', 'country')
    list_filter = ('country', )
    search_fields = ('=iata_code', )
admin.site.register(Airport, AirportAdmin)

[Julien Phalip]

To my knowledge, you're not supposed to sort by a callable. Only database field sorting is supported. So I'd rather have it throw an error than just ignore it.

[Karen Tracey]

Callables can be made to be sortable if you specify an admin_order_field for them, as described under list_display here:

​http://docs.djangoproject.com/en/dev/ref/contrib/admin/#list-display

(scroll down to just above list_display_links)

The problem here is these "fields" are not enclosed in quotes in list_display. They should be, just like all the others and like the example in the doc. Put them in quotes and then it will work without any change needed in the admin code.

[rgl]

The docs say and show that I can use a function variable in list_display and even sort by it (by setting admin_order_field like I did)... but its not working; so, how come this was closed as invalid?

[Karen Tracey]

It was closed as invalid because the way you have specified list_display is invalid. It should be:

list_display = ('iata_code', 'name', 'admin_lat', 'admin_lng', 'tz', 'country')

not your version which did not have quotes around admin_lat and admin_lng. This is consistent with what is in the docs, they do not show including callables unquoted in list_display.

[rgl]

Huh?

Please, check the docs:

"""

You have four possible values that can be used in list_display:
...
class PersonAdmin(admin.ModelAdmin):
    list_display = (upper_case_name,)

""" -- ​http://docs.djangoproject.com/en/dev/ref/contrib/admin/#list-display

[Karen Tracey]

Look further down, where admin_order_field is described. That callable (colored_first_name) is included in quotes when specified in list_display:

class PersonAdmin(admin.ModelAdmin):
    list_display = ('first_name', 'colored_first_name')

I do not know without further investigation that I do not have time for at the moment if the instance you point out is a doc bug or a code bug. For right now, if you would just put your fields in quotes it will work. I have tested that.

[Karen Tracey]

Oh, OK I see now. A callable that's not a model method, which was a late-added feature. Sorry to be dense.

[Ramiro Morales]

The documentation actually says that the string version of the method names should be used in list_display when the callable is effectively a method (of the model or of the user-provided ModelAdmin subclass, it doesn't explicitly states the same is true for the non-method callables (bare functions that take one parameter).

It also doesn't explicitly states that it is possible to sort the admin display by such callables. It just says you can only use the model method kind of callables when they effectively represent a DB field (and in fact the sorting is performed using that DB field, that's what .admin_order_field is for).

Just my two cents, I don't know what's the problem (if any) here either: The documentation or the code implementation

[Karen Tracey]

Replying to ramiro:

Just my two cents, I don't know what's the problem (if any) here either: The documentation or the code implementation

There is a code bug here, the admin makes the column sortable but then raises an exception if the user actually clicks on the link admin placed in the column header.

If these non-model-method callables aren't supposed to be allowed to be sorted, then admin shouldn't put the link to sort them in the column header, even if the user has specified the admin_order_field. But I suspect that the admin_order_field feature was just overlooked when non-model-method callables in list_display was implemented, rather than a conscious decision being made to not support sorting by them. From a brief look it seems easier to just complete the implementation of non-model-method callables and support sorting by them (there it is in the provided patch, after all) than to add the checks and doc required to properly disallow it instead of crashing and burning.

[Jenan Wise]

That patch looks good, but why not fix the case of field_name referring to an attr of self.model_admin as well?

if callable(field_name):
    attr = field_name
elif hasattr(self.model_admin, field_name):
    attr = getattr(self.model_admin, field_name)
else:
    attr = getattr(self.model, field_name)

[Karen Tracey]

Replying to jenan:

That patch looks good, but why not fix the case of field_name referring to an attr of self.model_admin as well?

Yes, some testing shows that while the admin does not crash and burn on sorting on a model_admin method, it also doesn't actually perform a sort on the specified field either. I plan to fix the code and add some tests that actually ensure these things work.

[Karen Tracey]

(In [9211]) Fixed #9053 -- Allowed for sorting of callable and ModelAdmin methods specified in list_display (added in r8352). Previously attempting to sort on the former would raise an exception and the latter simply didn't sort. Also added tests for this function. Thanks rgl and jenan.

[Karen Tracey]

(In [9212]) [1.0.X] Fixed #9053 -- Allowed for sorting of callable and ModelAdmin methods specified in list_display (added in r8352). Previously attempting to sort on the former would raise an exception and the latter simply didn't sort. Also added tests for this function. Thanks rgl and jenan.

Backport of [9211], also updated svnmerge.py metatdata.