Re: [Django] #5863: list_display does not allow functions of referenced objects
Django
-------------------------------------+-------------------------------------
Reporter: Beat | Owner: nobody
Bolli <me+django@…> | Status: reopened
Type: | Component: contrib.admin
Uncategorized | Severity: Normal
Milestone: | Keywords: list_display
Version: SVN | Has patch: 1
Resolution: | Needs tests: 0
Triage Stage: Design | Easy pickings: 0
decision needed |
Needs documentation: 1 |
Patch needs improvement: 1 |
-------------------------------------+-------------------------------------
Changes (by anonymous):
* status: closed => reopened
* resolution: wontfix =>
Comment:
I am re-opening this ticket once more to get some more attention on
this...the current proposed workarounds have serious deficiencies:
1. Write functions everywhere (this involves a lot of repetition for every
single foreign field)
2. Write a wrapper like @lukeplant suggested (even though it is far
superior solution to individual functions in every model)
are all HUGE violation of DRY principles on which sensible programming is
based toady. Since the parent__child syntax is being used for list filter,
search fields and everywhere else, why is the list_display so sacrosanct?
E.g. once a model field already has an attribute like verbose_name
defined...why should a function or a wrapper function have to define it
again. It is not feasible to copy that definition in the case of the
wrapper.
My question is, will this be accepted as a patch if contributed on the
trunk with test cases et all?
--
Ticket URL: <http://code.djangoproject.com/ticket/5863#comment:35>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Beat | Owner: nobody
Uncategorized | Severity: Normal
Milestone: | Keywords: list_display
Version: SVN | Has patch: 1
decision needed |
Needs documentation: 1 |
Patch needs improvement: 1 |
-------------------------------------+-------------------------------------
* status: reopened => closed
* resolution: => wontfix
Comment:
Please don't reopen tickets that were marked as wontfix by a core
developer but raise the issue on the django-developers list instead.
--
Ticket URL: <http://code.djangoproject.com/ticket/5863#comment:36>
Django
-------------------------------------+-------------------------------------
Reporter: Beat | Owner: nobody
Bolli <me+django@…> | Status: closed
Type: | Component: contrib.admin
Uncategorized | Severity: Normal
Milestone: | Keywords: list_display
Version: SVN | Has patch: 1
Resolution: wontfix | Needs tests: 0
Triage Stage: Design | Easy pickings: 0
decision needed |
Needs documentation: 1 |
Patch needs improvement: 1 |
-------------------------------------+-------------------------------------
Comment (by lukeplant):
Also, in response to andybak and anonymous, please note:
list_display is fundamentally different from list_filter and
search_fields. The other two are defining operations that must happen in
the database, whereas list_display is defining an operation that must
happen in Python code, and refers to Python functions/methods, and **not**
to database fields. The confusion is that a name that of a database field
is also the name of the corresponding attribute on the Python object, but
that is where the similarity ends.
So, for instance, you can set `'__unicode__'` in list_display, and it will
refer to the `__unicode__` method, and not attempt any field lookups,
despite the presence of double underscores. As it happens, this example
makes it obvious that we cannot sensibly add the feature as proposed - how
would you refer to the `__unicode__` method on a related object?
`'foreign_key____unicode__'`? How would you parse that? What about other
methods that people might choose to define, like `__html__`, etc.? The
only way this feature could get in is we are happy with a bunch of special
cases in the implementation, and a bunch of arbitrary limitations in
functionality.
--
Ticket URL: <http://code.djangoproject.com/ticket/5863#comment:37>
Django
-------------------------------------+-------------------------------------
Reporter: Beat | Owner: nobody
Bolli <me+django@…> | Status: closed
Type: | Component: contrib.admin
Uncategorized | Severity: Normal
Milestone: | Keywords: list_display
Version: SVN | Has patch: 1
Resolution: wontfix | Needs tests: 0
Triage Stage: Design | Easy pickings: 0
decision needed |
Needs documentation: 1 |
Patch needs improvement: 1 |
-------------------------------------+-------------------------------------
Comment (by brillgen):
@lukeplant, the basic problem you've correctly raised is that list_display
allows for callables and hence arbitrary names can be used which are
simiar to the syntax for foreign key fields.
However, this problem exists for list_filter also: someone may define a
field with the name class__field and try to use that in list_filter as a
foreign key field. (i've just checked that you can indeed name a field
like that)
So we can't really be compensating for dev stupidity at the cost of
functionality as we haven't done in list_filter also.
Since all FKs always start with an alphabet are in the regex format
(.+__.+.*), the __unicode etc cases can be easily and correctly filtered
as is required to be done in list_filter.
--
Ticket URL: <http://code.djangoproject.com/ticket/5863#comment:38>
Django
-------------------------------------+-------------------------------------
Reporter: Beat | Owner: nobody
Bolli <me+django@…> | Status: closed
Type: | Component: contrib.admin
Uncategorized | Severity: Normal
Milestone: | Keywords: list_display
Version: SVN | Has patch: 1
Resolution: wontfix | Needs tests: 0
Triage Stage: Design | Easy pickings: 0
decision needed |
Needs documentation: 1 |
Patch needs improvement: 1 |
-------------------------------------+-------------------------------------
Comment (by lukeplant):
If we are allowing field names to contain double underscores, that is
almost certainly a validation bug which should be fixed. We have
restrictions on how fields can be named, and don't have restrictions on
other methods or attributes, and I think we should keep it that way.
Adding attribute lookup into a syntax that has been designed for table
joins is a bad idea. It will certainly require limitations on the names of
attributes (e.g. 'private' attributes that start with a double
underscore). Remembering that there is no reason why the attribute lookup
should be at the end, it may not be possible at all - I'm not convinced
you easily handle methods like `__foo__` if it is followed by another
'join'.
However, using a **dot** lookup syntax would make sense here, because
list_display is about object **attributes** and **not** field joins. This
might be a sensible proposal, but not sure if would deal with all the
problems - you still can't get the fields verbose_name easily etc.
--
Ticket URL: <http://code.djangoproject.com/ticket/5863#comment:39>
Django
-------------------------------------+-------------------------------------
Reporter: Beat | Owner: nobody
Bolli <me+django@…> | Status: closed
Type: | Component: contrib.admin
Uncategorized | Severity: Normal
Milestone: | Keywords: list_display
Version: SVN | Has patch: 1
Resolution: wontfix | Needs tests: 0
Triage Stage: Design | Easy pickings: 0
decision needed |
Needs documentation: 1 |
Patch needs improvement: 1 |
-------------------------------------+-------------------------------------
Comment (by brillgen):
I agree that its better to restrict the field names than have restrictions
on other methods or attributes.
Based on the distinction between list_filter and list_display, I was just
thinking that a . lookup syntax for attributes (class attribute would be
closer to the idea that list_display is about object attributes and not
database joins. However, why would retrieving attributes difficult? The
admin has the model object and hence can span the model definitions to
determine the attributes (using the . syntax to identify members and their
attributes) ..In fact it would allow methods on FKs to be displayed as
well which was not covered so well with the __ syntax.
--
Ticket URL: <http://code.djangoproject.com/ticket/5863#comment:40>
Django
-------------------------------------+-------------------------------------
Reporter: Beat | Owner: nobody
Bolli <me+django@…> | Status: closed
Type: | Component: contrib.admin
Uncategorized | Severity: Normal
Milestone: | Keywords: list_display
Version: SVN | Has patch: 1
Resolution: wontfix | Needs tests: 0
Triage Stage: Design | Easy pickings: 0
decision needed |
Needs documentation: 1 |
Patch needs improvement: 1 |
-------------------------------------+-------------------------------------
Comment (by lukeplant):
I gave a more full reply on the list: http://groups.google.com/group
/django-developers/browse_thread/thread/790484bfbe1b421f
My apologies for bouncing around between Trac and the mailing list, I
should have stuck to the list as is our policy.
--
Ticket URL: <http://code.djangoproject.com/ticket/5863#comment:41>
Django
-------------------------------------+-------------------------------------
<me+django@…> | Status: closed
Type: Uncategorized | Version: master
Component: contrib.admin | Resolution: wontfix
Severity: Normal | Triage Stage: Design
Keywords: list_display | decision needed
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by jcushman):
* ui_ux: => 0
Comment:
If anyone else who wants to be able to refer to foreign keys with
underscore syntax in list_display comes across this ticket, I put together
a subclass of ModelAdmin to do that:
http://djangosnippets.org/snippets/2887/
So you can just do:
{{{
class FooAdmin(RelatedFieldAdmin):
list_display = ('address__phone','address__country__country_code')
}}}
and it figures it out.
The benefit of using a subclass over luke's suggestion is we can override
queryset() to automatically add the related models to select_related(),
which prevents a database hit for each row. It also sets sensible defaults
for short_description and admin_order_field. Would love to hear (over at
djangosnippets) if anyone has suggested improvements.
--
Ticket URL: <https://code.djangoproject.com/ticket/5863#comment:42>
Django
-------------------------------------+-------------------------------------
Reporter: Beat Bolli | Owner: nobody
<me+django@…> | Status: closed
Type: Uncategorized | Version: master
Component: contrib.admin | Resolution: wontfix
Severity: Normal | Triage Stage: Design
Keywords: list_display | decision needed
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
Changes (by jcushman):
* ui_ux: 0 => 1
Comment:
(Sorry, not sure why it unset UI/UX when I commented.)
--
Ticket URL: <https://code.djangoproject.com/ticket/5863#comment:43>
Django
-------------------------------------+-------------------------------------
Reporter: Beat Bolli | Owner: nobody
<me+django@…> | Status: closed
Type: Uncategorized | Version: master
Component: contrib.admin | Resolution: wontfix
Severity: Normal | Triage Stage: Design
Keywords: list_display | decision needed
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by petr.dlouhy@…):
I remade the Snippet 2996 in form of an application (for anyone interested
in working around this issue): https://github.com/PetrDlouhy/django-
related-admin
--
Ticket URL: <https://code.djangoproject.com/ticket/5863#comment:44>
Django
-------------------------------------+-------------------------------------
Reporter: Beat Bolli | Owner: nobody
<me+django@…> |
Component: contrib.admin | Version: master
Severity: Normal | Resolution: wontfix
Keywords: list_display | Triage Stage: Design
| decision needed
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* cc: hugo@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/5863#comment:45>
Django
-------------------------------------+-------------------------------------
Reporter: Beat Bolli | Owner: nobody
<me+django@…> |
Type: Uncategorized | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: wontfix
Keywords: list_display | Triage Stage: Design
| decision needed
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* cc: zachborboa@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/5863#comment:46>
Django
-------------------------------------+-------------------------------------
Reporter: Beat Bolli | Owner: nobody
<me+django@…> |
Type: Uncategorized | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: wontfix
Keywords: list_display | Triage Stage: Design
| decision needed
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Michael):
I would just like to note its not a "rare use case", the stack overflow
post on how to acocmplish this has 164K views, and the dissapointing
answer has about 500 uploads.
[https://stackoverflow.com/questions/163823/can-list-display-in-a-django-
modeladmin-display-attributes-of-foreignkey-field]
This I would dare guess it is the most common limitation people deal with
the admin interface.
--
Ticket URL: <https://code.djangoproject.com/ticket/5863#comment:47>
Django
-------------------------------------+-------------------------------------
Reporter: Beat Bolli | Owner: nobody
<me+django@…> |
Type: Uncategorized | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: wontfix
Keywords: list_display | Triage Stage: Design
| decision needed
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Michael):
Replying to [comment:18 Ramiro Morales]:
> I'd say this ticket can be closed (I found it when looking for material
related to #10230).
>
> Using callables (bare functions for example, i.e. no model methods nor
`ModelAdmin` subclasses methods) make the "It violates DRY: I have to
define the same function on each model that references the foreign model"
argument moot.
>
> Also, when it comes to the HTML table headers:
>
> 1. Seeing a bare 'Code' header (using the
[http://code.djangoproject.com/ticket/5863#comment:6 comment 6] example)
that refers to a `code` field of a `Client` model that is located one or
more FK-hops away isn't necessarily a good idea, it can be confusing
because the user could assume it refers about a (non-existent) `code`
field local to the model being displayed.
> 2. In the case of more that one level of indirection, having an
automatically-generated `'Hop1 Model > ... > HopN Model > Field'` header
wouldn´t be practical/scalable.
>
> It's more practical in that case that the developer sets explicitely an
appropiate, unambiguous header literal using the `.short_description`
facility.
>
> In other words, I'd say current functionality plus a bit of work satisfy
the needs expressed in this ticket discussion for this arguably rare
scenario.
Unless I am missing something, I don't see how `1.` solves the issue. Say
I have Model `A`, and it has a foreign key to `Z`. Then I have model B,
which has a foreign key to 'A'. Now in model `B` and `A` I wish to show
one of `Z`s field's admin, how do I use the common function?
And with regards to it being rare, as mentioned above about `164K` views
on stack overflow.
--
Ticket URL: <https://code.djangoproject.com/ticket/5863#comment:48>