#17252: Multi-sort in admin does not respect initial admin_order_field
-------------------------------------+-------------------------------------
Reporter: sebastian | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: SVN
Severity: Release blocker | Resolution:
Keywords: multi-sort, admin, | Triage Stage: Accepted
ordering | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1
Needs tests: 0 | UI/UX: 1
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by julien):
* severity: Normal => Release blocker
Comment:
Replying to [comment:5 sebastian]:
> Sure, making `ordering` more explicit would be nice but IMHO that's
outside the scope of this ticket. Here, I'm only concerned with the
regression introduced by the multi-sort addition to the admin which causes
the initial sort indicator icons to not show up under certain
circumstances, i.e. whenever something is sorted with `admin_order_field`.
>
> This bug is not a functional issue per se, but hiding those icons when
the admin change list is actually sorted (which it is) is kind of
misleading to the user.
I confirm that this is a regression, thus marking as blocker. It's ok to
address only that regression in this ticket.
> Regarding the proposition of making `ordering` more explicit, I suggest
opening a new ticket for that and keeping this one for the regression at
hand. Some questions arise, however, how far this more explicit `ordering`
should reach: should method names also be allowed in models, ie. not only
model admins?
> ... snip ...
Yes, a separate ticket could be opened for that. `ModelAdmin.ordering`
should accept exactly the same options as `ModelAdmin.list_display`, i.e.
field names or `ModelAdmin` methods or `Model` methods that have a
`admin_ordering_field` pointing to an actual model field.
The patch looks good. The tests, however, don't seem to check which column
is actually sorted. They're also a bit fragile as they're testing HTML
code (i.e. '`class="sortable sorted ascending"`') which is always a bit
volatile in the admin templates. I think the tests could be made a little
more explicit and less fragile by testing the context value of
`cl.get_ordering_field_columns()`. Could you make that change before we
proceed? Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/17252#comment:7>
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.
