[Django] #27752: Fix and test admin_order_field set for the __str__ of a model
Django
-----------------------------------------+------------------------
Reporter: Claude Paroz | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
While doing some force_str cleanup on Django 2.0, Tim Graham and me
noticed a possible issue with `admin_order_field` on a `__str__`method (in
`django.contrib.admin.utils.label_for_field`). It's apparently untested.
See this related commit [1], with a test that was later removed [2].
https://github.com/django/django/commit/2304ca423640a7b06390530bf61c879f9ff9e4ba
https://github.com/django/django/commit/65cc646c4801e3f3e1df1ab06dfaa763fa4b7b22
So a test for the proper label and sorting is wanted. See
`test_change_list_sorting_model_admin` in `tests/admin_views/tests.py` for
a similar test.
--
Ticket URL: <https://code.djangoproject.com/ticket/27752>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Claude Paroz | Owner: Renato
| dos Santos Oliveira
Type: Bug | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Renato dos Santos Oliveira):
* cc: renato+github@… (added)
* owner: nobody => Renato dos Santos Oliveira
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/27752#comment:1>
Django
Reporter: Claude Paroz | Owner: (none)
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Renato dos Santos Oliveira):
* status: assigned => new
* cc: renato+github@… (removed)
* owner: Renato dos Santos Oliveira => (none)
--
Ticket URL: <https://code.djangoproject.com/ticket/27752#comment:2>
Django
Reporter: Claude Paroz | Owner: (none)
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Renato dos Santos Oliveira):
* cc: renato+github@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/27752#comment:3>
Django
Reporter: Claude Paroz | Owner: (none)
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
For example, adding to the `Question` model in the tutorial:
`__str__.admin_order_field = 'question_text'`
and to `QuestionAdmin`:
`list_display = ('__str__', ...)`
should reproduce this. You'll see that the `__str__` column isn't
clickable for sorting like it should be. In the `lable_for_field`
function, I tried changing `attr = str` to `attr = model.__str__` -- this
makes the column clickable but sorting still doesn't seem to work
correctly.
--
Ticket URL: <https://code.djangoproject.com/ticket/27752#comment:4>
Django
Reporter: Claude Paroz | Owner: Renato
| dos Santos Oliveira
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Renato dos Santos Oliveira):
* status: new => assigned
* owner: (none) => Renato dos Santos Oliveira
--
Ticket URL: <https://code.djangoproject.com/ticket/27752#comment:5>
Django
Reporter: Claude Paroz | Owner: Renato
| dos Santos Oliveira
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Renato dos Santos Oliveira):
Hey, I think I found the issue here.
Changing as you said `attr = str` to `attr = model.__str__` it makes the
column clickable and adds the field position to the `o` querystring. The
problem is that when it gets to the admin view (here:
https://github.com/django/django/blob/master/django/contrib/admin/views/main.py#L227-L233)
it verifies first if the attribute is an instance of the model_admin class
and it turns out that ModelAdmin has a __str__ field, not carrying the
`model.__str__.admin_order_field` attribute to the ordering query.
this is the `attr` representation with the `model_admin` elif `<bound
method ModelAdmin.__str__ of <core.admin.QuestionAdmin object at
0x7f82d7bc7748>`
for testing purposes I removed the model_admin and got this
`<function Question.__str__ at 0x7f04c9ea5598>` and it had the
`admin_order_field` attribute, making ordering work properly.
I still don't know how to solve this and would be awesome to have your
help on this.
Thanks
--
Ticket URL: <https://code.djangoproject.com/ticket/27752#comment:6>
Django
Reporter: Claude Paroz | Owner: Renato Oliveira
Type: Bug | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Is it worth adding a special case in `get_ordering_field()` to get
`__str__()` from the model instead of the `ModelAdmin` (which probably
wouldn't have a useful `__str__()`? I think probably not. We might just
document that `admin_order_field` isn't usable with `Model.__str__()` and
move on to more important issues.
--
Ticket URL: <https://code.djangoproject.com/ticket/27752#comment:7>
Django
Reporter: Claude Paroz | Owner: Renato Oliveira
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Renato Oliveira):
I'm trying to write something here but nothing seems to fit. Maybe because
I'm not fluent on English
{{{
* The ``__str__()`` method is just as valid in ``list_display`` as any
other model method, so it's perfectly OK to do this::
list_display = ('__str__', 'some_other_field')
* Usually, elements of ``list_display`` that aren't actual database
fields can't be used in sorting (because Django does all the sorting
at the database level).
However, if an element of ``list_display`` represents a certain
database field, you can indicate this fact by setting the
``admin_order_field`` attribute of the item.
This rule doesn't apply to ``__str__`` method.
}}}
It's missing why it's not applicable, but It's not clear to me how to say
that
--
Ticket URL: <https://code.djangoproject.com/ticket/27752#comment:8>
Django
Reporter: Claude Paroz | Owner: (none)
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* owner: Renato Oliveira => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/27752#comment:9>
Django
Reporter: Claude Paroz | Owner: (none)
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Ülgen Sarıkavak):
* cc: Ülgen Sarıkavak (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/27752#comment:10>