Opened 15 years ago

Closed 14 years ago

Last modified 13 years ago

#9749 closed (fixed)

ModelAdmin should allow specifying Changelist class to further modify Changelist behavior

Reported by: anonymous Owned by: Jannis Leidel
Component: contrib.admin Version: dev
Severity: Keywords: ModelAdmin Changelist subclass
Cc: www_djangoproject_com@…, jarek.zgoda@…, ales.zoulek@…, David Larlet, Salva Pinyol, floguy@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I've been poking around the Django source as I want to add a column to the changelist view. Specifically, this column is a consolidation of a ManyToMany relationship. I'll use it as an example here.

In options.py, class ModelAdmin defines a function that instantiates a Changelist:

class ModelAdmin(BaseModelAdmin):
    def __init__(self, model, admin_site):
        self.model = model
        self.opts = model._meta
        self.admin_site = admin_site
        #more code

    #more code, more functions

    def changelist_view(self, request, extra_context=None):
        "The 'change list' admin view for this model."
        #more code here
        from django.contrib.admin.views.main import ChangeList, ERROR_FLAG
        #more code here
        try:
            cl = ChangeList(request, self.model, self.list_display, self.list_display_links, self.list_filter,
                     self.date_hierarchy, self.search_fields, self.list_select_related, self.list_per_page, self)
        #more code here

I propose the class 'ChangeList' be easily overidable, like so:

from django.contrib.admin.views.main import ChangeList, ERROR_FLAG

    class ModelAdmin(BaseModelAdmin):
        changelist_class = ChangeList
        def __init__(self, model, admin_site):
            self.model = model
            self.opts = model._meta
            self.admin_site = admin_site
            self.changelist_class = changelist_class
            #more code
    def changelist_view(self, request, extra_context=None):
        "The 'change list' admin view for this model."
        #more code here
        from django.contrib.admin.views.main import ChangeList, ERROR_FLAG
        #more code here
        try:
            cl = self.changelist_class(request, self.model, self.list_display, self.list_display_links, self.list_filter,
                     self.date_hierarchy, self.search_fields, self.list_select_related, self.list_per_page, self)
        #more code here

This would allow one to easily override some ChangeList methods from an admin.py file:

    from models import Object, ObjectRelationToWidget #ObjectRelationToWidget acts as the ManyToMany bridge table
    from django.contrib.admin.views.main import ChangeList

    class ObjectChangeList(ChangeList):
	    def get_results(self, request):

                r = ObjectRelationToWidget.objects.all().select_related() #this puts all the Objects and Widgets into the queryset cache
                results = []
                for obj in r:
                    r.object._v_local = {}
                    r.object._v_local['widget_name'] = r.widget.name
                    r.object.widget_name = lambda x: return x._v_local['widget_name']
                    results.append(r.object)

 	        self.result_count = len(results)
 	        self.full_result_count = len(results)
 	        self.result_list = results



     class ObjectAdmin(admin.ModelAdmin):
        changelist_class = ObjectChangeList
        list_display = ["object_name", "widget_name"]

Are there any thoughts on this suggestion? It is simple to add as a feature, but I suppose the usefulness may be seen as somewhat dubious. Suggestions for 'doing this better' (putting a related ManyToMany field in the admin changelist) would be appreciated, also.

Attachments (7)

patch_django_9749.20090111.diff (5.3 KB ) - added by David Larlet 15 years ago.
First iteration, review welcome
patch_django_9749.20090111.2.diff (7.9 KB ) - added by David Larlet 15 years ago.
More tests, against r9728
patch_django_9749.20090317.diff (8.4 KB ) - added by David Larlet 15 years ago.
New patch against r10069 (another bug resolution was conflictual with tests), nothing has changed
patch_django_9749.20090318.diff (8.3 KB ) - added by David Larlet 15 years ago.
Just an update against r10087
patch_django_9749.20090323.diff (7.3 KB ) - added by David Larlet 15 years ago.
Update against r10123 with feature freeze it will be more simple to maintain now
patch_django_9749.20090507.diff (7.0 KB ) - added by David Larlet 15 years ago.
Update against r10680
9749-different-approach.diff (4.5 KB ) - added by floguy 14 years ago.
My version

Download all attachments as: .zip

Change History (28)

comment:1 by David Larlet, 15 years ago

Cc: larlet@… added
Needs documentation: set
Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedDesign decision needed

I second this request, this is useful and I ran into similar issue (copy/paste the whole changelist_view function just to change the ChangeList call).

comment:2 by Brian Rosner, 15 years ago

Triage Stage: Design decision neededAccepted

comment:3 by David Larlet, 15 years ago

Cc: larlet@… removed
Owner: changed from nobody to David Larlet
Status: newassigned

by David Larlet, 15 years ago

First iteration, review welcome

comment:4 by David Larlet, 15 years ago

Has patch: set
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:5 by David Larlet, 15 years ago

Patch needs improvement: set

It seems that more tests are required in admin_views. I'll work on that part.

by David Larlet, 15 years ago

More tests, against r9728

comment:6 by David Larlet, 15 years ago

Patch needs improvement: unset

admin_views' tests added.

comment:7 by Brian Rosner, 15 years ago

Owner: changed from David Larlet to Brian Rosner
Status: assignednew

by David Larlet, 15 years ago

New patch against r10069 (another bug resolution was conflictual with tests), nothing has changed

by David Larlet, 15 years ago

Just an update against r10087

by David Larlet, 15 years ago

Update against r10123 with feature freeze it will be more simple to maintain now

comment:8 by leitjohn, 15 years ago

Cc: www_djangoproject_com@… added

comment:9 by Jarek Zgoda, 15 years ago

Cc: jarek.zgoda@… added

comment:10 by anonymous, 15 years ago

Cc: ales.zoulek@… added

by David Larlet, 15 years ago

Update against r10680

comment:11 by Brian Rosner, 15 years ago

milestone: 1.2

comment:12 by dc, 15 years ago

Maybe use 'changelist' instead of 'changelist_class'? Because currently admin has attribute 'form' (not 'form_class').

comment:13 by David Larlet, 15 years ago

Cc: David Larlet added

To me it looks more explicit to use the _class suffix when it's a class and not an instance. But I agree that historically configuration doesn't specify it's a class...

comment:14 by Salva Pinyol, 15 years ago

Cc: Salva Pinyol added

comment:15 by Alex Gaynor, 15 years ago

Triage Stage: AcceptedReady for checkin

comment:16 by Luke Plant, 15 years ago

One comment - the type checking for a subclass of ChangeList seems to be pointless and un-pythonic (duck-typing means you allow any object that has the right interface). Is there a good reason for it that I'm missing?

by floguy, 14 years ago

My version

comment:17 by floguy, 14 years ago

Cc: floguy@… added

I've approached this in a slightly different way. My way involves mimicking the way some of the other methods on the ModelAdmin class works (get_form, etc) by implementing a method to provide the class. This way is slightly more flexible due to the availability of more information (like everything from the request). I hope this patch makes it in, because right now trying to just slightly change behavior involves a whole lot of copy/paste.

I understand one concern with this is that it could make ChangeList a more public API, but it's still really not. For example, it's still not documented. But it does give greater flexibility at a per-request level, and that's what I'm after.

comment:18 by Jannis Leidel, 14 years ago

Owner: changed from Brian Rosner to Jannis Leidel
Status: newassigned
Version: 1.0SVN

comment:19 by David Larlet, 14 years ago

@lukeplant: This is un-pythonic but that's the way it's done in admin.validation.validate to check others attrs. Anyway, floguy's solution solve the problem.

@floguy: I agree that it can be very interesting to get the request object in this case and I prefer your approach (see luke's comment).

@jezdez: thanks for taking this :)

comment:20 by Jannis Leidel, 14 years ago

Resolution: fixed
Status: assignedclosed

(In [11910]) Fixed #9749 - Added hook to ModelAdmin for specifying custom ChangeLists. Thanks to David Larlet and Eric Florenzano.

comment:21 by Jacob, 13 years ago

milestone: 1.2

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top