Move method properties for admin list customisation to ModelAdmin

[Daniel Pope <dan@…>]

The current approach for customising list_display seems contrary to the spirit of newforms-admin. Method properties such as .short_description, .boolean and .allow_tags rarely belong in the model definition.

In fact, I noted that these options, for customising the look of the list column, duplicate filter functionality. .allow_tags is exactly |safe; |boolean could be a filter provided by django.contrib.admin.templatetags. The case is much more convincing when you take into account user-defined filters: if I already have a filter for presenting values for user consumption, I should not need to provide a model method to replicate or apply that filter.

I propose a syntax like this:

class AccountAdmin(admin.ModelAdmin):
    foo_column = admin.ListColumn('foo', heading='Foo Description', filter='boolean')
    get_bar_column = admin.ListColumn('get_bar', filter='safe', order_field='bar')

    list_display = [foo_column, get_bar_column, 'baz', 'bonk']

Filter would be specified in exactly template syntax for convenience, so

   get_bar_column = admin.ListColumn('get_bar', filter='truncatewords:"2"|slugify')

would render exactly the same as

{{row.get_bar|truncatewords:"2"|slugify}}

[Daniel Pope <dan@…>]

The reason I suggested filters be specified like this was because I thought it would be simpler for users to re-use this element of template syntax than to import the actual filter functions, especially because these would have to be curried to pass arguments.

ListColumn would probably also need to take a load_filters=[] kwarg to address templatetag library loading.

[dan@…]

Initial patch

[Daniel Pope <dan@…>]

I have tried coding up an implementation of this ticket (exactly as described above except the kwarg is 'header' not 'heading'). Patch needs improvement as it isn't pep8-compliant.

[Brian Rosner]

Not much time to get this in before feature-freeze.

[Daniel Pope <dan@…>]

Fixed sorting with ListColumns

[Brian Rosner]

I am going to take a step back and put this on 1.0-beta. However, the implementation is a bit too much for this. I want to just decouple the list_display methods from the model. It will be backward incompatible.

[Malcolm Tredinnick]

@brosner: allow list_display methods/functions to also exist on the model instance would be good, though. There are plenty of methods that naturally belong on models that are useful in the admin display.

[Brian Rosner]

@malcolmt: Good point. I was just talking with Joseph and I was wondering about the naming of admin_order_field. If we have methods in ModelAdmin that admin bit is redundant. Should that be dropped or maintained?

[Daniel Pope <dan@…>]

Would you just search both the model instance and the ModelAdmin for callables matching the list_display column? My patch should be backwards compatible, so it could be backwards compatible with yours if you are implementing it like that.

This turned out to be quite a nice refactor, as it broke up the monolithic functions for rendering ChangeLists (actually they were template inclusion tags) into distinct stages of working out how to render each column, then what to render, then rendering it, so that's something else to be said for developing this post-1.0, not least the neat separation between content and presentation that the filters kwarg allows.

[Brian Rosner]

@Daniel: I have sort of hijacked this ticket. You sort of described two different things one of which needs to get in before 1.0. The new API bits you have described above are worthy once we look at modified the API a bit more for the ChangeList stuff. The way I have it setup is it looks at the ModelAdmin first and if not found looks at the model level for non-field, non-callable (a new feature i am adding) in list_display.

[Daniel Pope <dan@…>]

That's fine. So you'll also implement #7503?

Yes, I realise it was really two separate things. It started out as just noting that in ModelAdmin there's now space to tidy up the system of method attributes, but somewhere along the way my main motivation became wanting to apply template filters. I noticed it made a lot of sense not just for the existing modifiers I mentioned, but practical sense in several of my apps, and that's a much bigger win than just API pointling.

For instance, you can add useful columns entirely with ListColumn, for example:

class SpecialOffer(models.Model):
    name = models.CharField(...)
    ends = models.DateTimeField()

class SpecialOfferAdmin(admin.ModelAdmin):
    list_display = [
        'name',
        'ends',
         admin.ListColumn('ends', filter='timeuntil', heading="Ending in")
    ]

[Brian Rosner]

(In [8352]) Fixed #7503 -- Allow callables in list_display. This also does a lookup on the ModelAdmin for the method if the value is a string before looking on the model. Refs #8054. Thanks qmanic and Daniel Pope for tickets and patches.

[Brian Rosner]

Moving to post-1.0 for the API changing parts. This should be looked at with a ChangeList and templatetag refactor of the admin. Not sure when or how, but this is related.

[(none)]

Milestone post-1.0 deleted

[Alex Kamedov]

update patch to django rev. 14124

[Alex Kamedov]

Update patch. Now it passes all existing regression tests.

[Alex Kamedov]

Update patch. Add regression tests and fix some bugs.

[Alex Kamedov]

Wiki page with this ticket goals and documentation - ListColumns

[Yuri Baburov]

Patch is looking good, except few small things:

1) value_map is not a function what can be read from its name, but a dict.
If it is used mostly for choices, it should be named so.

2) There's no line in docs mentioning value_map is applied *before* filters?

3) I'm not sure if we need 200 Kb fixture file called listcolumns.json
Can we strip it to few lines, showcasing all the features?
Tests themselves are very good!

4)
This is backward compatibility hack one copied from django.contrib.admin.util.label_for_field, but it's completely undocumented (and i'm not sure if it's required in django 1.3!):

 	145	            if self.field_name == '__unicode__': 
 	146	                self.header = force_unicode(model._meta.verbose_name) 
 	147	            elif self.field_name == '__str__': 
 	148	                self.header = smart_str(model._meta.verbose_name)

Why can't you just reuse label_for_field to always provide header info for you?

5) Wiki docs don't mention how ListColumn can be used together with list_display_links.

6) list_display_links should take ListColumn, or some special instance, but not its field_name attribute, or you might have 2 fields selected if they just have different filters applied to the same model or modeladmin attribute.
However this might be mention in docs as a known limitation and workaround shown.

7) Typos in wiki docs: 'tagging_autocomplite_tags' -> 'tagging_autocomplete_tags', 'calable' -> 'callable'

8) Wiki docs don't list admin filters that can be used now (boolean_icon)

[Alex Kamedov]

Update patch and wiki page according to Yuri Baburov (buriy) recomendations

[Carl Meyer]

Closing this ticket to avoid any further confusion, based on mailing list discussion here: ​http://groups.google.com/group/django-developers/browse_thread/thread/e263cb72eb25f8f8

Given that there doesn't appear to be any core dev support for the list_display syntax changes in the patch here, and the feature described by the summary/title is already present in trunk, I don't see any benefit to keeping this ticket open, and there appears to be a significant cost in confusion. The only actual "todo" I see remaining here is that the admin ChangeList needs a refactor; if that needs a ticket, it should get a new one that more clearly states that goal alone.

Resolving as fixed because all of the capabilities requested in the ticket are now possible, albeit with a different syntax than originally proposed.

[Jacob]

Milestone 1.3 deleted