id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
26372	admin_order_field ignored when shadowing model field	Julian Andrews	nobody	"* Create a method on a model admin which shadows a field on the model,
* Set an `admin_order_field` attribute on that method
* Add the method name to your model admin's `list_display` attribute

'''Result:'''

`ChangeList.get_ordering_field`  will return the field name instead of the associated `admin_order_field` value.

'''Expected Behavior:'''

`ChageList.get_ordering_field` should return the `adrmin_order_field` value.

Here's the relevant code:

{{{
    def get_ordering_field(self, field_name):
        """"""
        Returns the proper model field name corresponding to the given
        field_name to use for ordering. field_name may either be the name of a
        proper model field or the name of a method (on the admin or model) or a
        callable with the 'admin_order_field' attribute. Returns None if no
        proper model field name can be matched.
        """"""
        try:
            field = self.lookup_opts.get_field(field_name)
            return field.name
        except FieldDoesNotExist:
            # See whether field_name is a name of a non-field
            # that allows sorting.
            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)
            return getattr(attr, 'admin_order_field', None)
}}}

The core problem is that `ChangeList.get_ordering_field` is doing the field lookup first, and only trying to check for methods and other callables if that fails. I think that if a method or callable with a defined `admin_order_field` exists, it should return that rather than the field. Since shadowing a model field for the most part works fine, having the order behave strangely is counterintuitive.

It also seems that this code will return `None` if there's a callable that shadows the method name but has no `admin_order_field`, which may also be a bug.

I would propose that the order of priority for resolution should be:

1. methods with a defined admin_order_field
2. callables with a defined admin_order_field
3. model fields

I can see an argument for reversing 1 and 2 (if for nothing other than backwards compatibility) but I don't think a field should take precedence over a method with a clearly defined `admin_order_field` or that a callable without an `admin_order_field` should prevent ordering by a method.

I'd be happy to submit a patch if I can get a go-ahead to do it either as I described, or flipping 1 and 2."	Bug	new	contrib.admin	dev	Normal				Accepted	0	0	0	0	0	0