Opened 10 years ago

Closed 9 years ago

Last modified 7 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: master
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 10 years ago.
First iteration, review welcome
patch_django_9749.20090111.2.diff (7.9 KB) - added by David Larlet 10 years ago.
More tests, against r9728
patch_django_9749.20090317.diff (8.4 KB) - added by David Larlet 10 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 10 years ago.
Just an update against r10087
patch_django_9749.20090323.diff (7.3 KB) - added by David Larlet 10 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 9 years ago.
Update against r10680
9749-different-approach.diff (4.5 KB) - added by floguy 9 years ago.
My version

Download all attachments as: .zip

Change History (28)

comment:1 Changed 10 years ago by David Larlet

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 Changed 10 years ago by Brian Rosner

Triage Stage: Design decision neededAccepted

comment:3 Changed 10 years ago by David Larlet

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

Changed 10 years ago by David Larlet

First iteration, review welcome

comment:4 Changed 10 years ago by David Larlet

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

comment:5 Changed 10 years ago by David Larlet

Patch needs improvement: set

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

Changed 10 years ago by David Larlet

More tests, against r9728

comment:6 Changed 10 years ago by David Larlet

Patch needs improvement: unset

admin_views' tests added.

comment:7 Changed 10 years ago by Brian Rosner

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

Changed 10 years ago by David Larlet

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

Changed 10 years ago by David Larlet

Just an update against r10087

Changed 10 years ago by David Larlet

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

comment:8 Changed 9 years ago by leitjohn

Cc: www_djangoproject_com@… added

comment:9 Changed 9 years ago by Jarek Zgoda

Cc: jarek.zgoda@… added

comment:10 Changed 9 years ago by anonymous

Cc: ales.zoulek@… added

Changed 9 years ago by David Larlet

Update against r10680

comment:11 Changed 9 years ago by Brian Rosner

milestone: 1.2

comment:12 Changed 9 years ago by dc

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

comment:13 Changed 9 years ago by David Larlet

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 Changed 9 years ago by Salva Pinyol

Cc: Salva Pinyol added

comment:15 Changed 9 years ago by Alex Gaynor

Triage Stage: AcceptedReady for checkin

comment:16 Changed 9 years ago by Luke Plant

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?

Changed 9 years ago by floguy

My version

comment:17 Changed 9 years ago by floguy

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 Changed 9 years ago by Jannis Leidel

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

comment:19 Changed 9 years ago by David Larlet

@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 Changed 9 years ago by Jannis Leidel

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 Changed 7 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

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