#17522: ModelAdmin.ordering validation too strict
-------------------------------------+-------------------------------------
Reporter: sebastian | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: SVN
Severity: Normal | Resolution:
Keywords: admin, validation, | Triage Stage:
ordering, strict | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by sebastian):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Let me add some thoughts as to why I think the current validation should
be changed.
It is quite possible to add annotations to the change list queryset with
`queryset()`. When used with `admin_order_field`, such columns are
perfectly sortable in the change list view by clicking the corresponding
column. What is not possible, however, as of r17372, is to specify these
columns as ''default'' sorting for the corresponding change list view.
{{{#!python
# models.py
class Foo(models.Model):
pass
class Bar(models.Model):
foo = models.ForeignKey(Foo)
# admin.py
class FooAdmin(admin.ModelAdmin):
list_display = ('bar_count',)
def queryset(self, request):
return super(FooAdmin,
self).queryset(request).annotate(bar_count=models.Count('bar_set'))
def bar_count(self, foo):
return foo.bar_count
bar_count.admin_order_field = 'bar_count'
# This does not work:
#ordering = ('bar_count',)
}}}
I understand the motivation for doing some basic sanity checks on the
model admin's attributes. In this case, however, I think these are too
strict, i.e. checking only if a matching field exists on the model is not
what should be done.
On the other hand, instead of entirely removing the checks so that we
simply assume that the corresponding attribute will exist eventually at
the time the query is executed, I propose this compromise: check if there
is (a) an attribute, or (b) a method on either the model or admin class
with `admin_order_field` set.
This way, we can still specify arbitrary fields, such as produced by
annotations, while retaining the basic sanity check that there must be at
least ''something'' declared manually, either a field or a method. This
should prevent most typos while allowing the necessary freedom to add
default-sortable columns based on annotation fields.
BTW, this is exactly how `admin_order_field` works in regular
`list_display` in the first place. No checks for the existence of the
given field are done in the validation code, for the same reason: to be
able to include annotations as sortable columns.
--
Ticket URL: <https://code.djangoproject.com/ticket/17522#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
