Django

Code

Ticket #9749 (closed: fixed)

Opened 1 year ago

Last modified 3 months ago

ModelAdmin should allow specifying Changelist class to further modify Changelist behavior

Reported by: anonymous Assigned to: jezdez
Milestone: 1.2 Component: django.contrib.admin
Version: SVN Keywords: ModelAdmin Changelist subclass
Cc: www_djangoproject_com@leithall.com, jarek.zgoda@gmail.com, ales.zoulek@gmail.com, david, spinyol, floguy@gmail.com Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

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

patch_django_9749.20090111.diff (5.3 kB) - added by david on 01/10/09 18:29:15.
First iteration, review welcome
patch_django_9749.20090111.2.diff (7.9 kB) - added by david on 01/10/09 19:02:56.
More tests, against r9728
patch_django_9749.20090317.diff (8.4 kB) - added by david on 03/16/09 19:39:01.
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 on 03/18/09 09:36:44.
Just an update against r10087
patch_django_9749.20090323.diff (7.3 kB) - added by david on 03/23/09 17:48:13.
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 on 05/07/09 04:26:58.
Update against r10680
9749-different-approach.diff (4.5 kB) - added by floguy on 12/17/09 16:15:11.
My version

Change History

12/30/08 04:12:21 changed by david

  • cc set to larlet@gmail.com.
  • needs_better_patch set to 1.
  • stage changed from Unreviewed to Design decision needed.
  • needs_tests set to 1.
  • needs_docs set to 1.

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).

01/08/09 11:21:38 changed by brosner

  • stage changed from Design decision needed to Accepted.

01/08/09 11:24:01 changed by david

  • cc deleted.
  • owner changed from nobody to david.
  • status changed from new to assigned.

01/10/09 18:29:15 changed by david

  • attachment patch_django_9749.20090111.diff added.

First iteration, review welcome

01/10/09 18:29:46 changed by david

  • needs_better_patch deleted.
  • has_patch set to 1.
  • needs_tests deleted.
  • needs_docs deleted.

01/10/09 18:34:11 changed by david

  • needs_better_patch set to 1.

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

01/10/09 19:02:56 changed by david

  • attachment patch_django_9749.20090111.2.diff added.

More tests, against r9728

01/10/09 19:04:50 changed by david

  • needs_better_patch deleted.

admin_views' tests added.

01/20/09 18:01:53 changed by brosner

  • owner changed from david to brosner.
  • status changed from assigned to new.

03/16/09 19:39:01 changed by david

  • attachment patch_django_9749.20090317.diff added.

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

03/18/09 09:36:44 changed by david

  • attachment patch_django_9749.20090318.diff added.

Just an update against r10087

03/23/09 17:48:13 changed by david

  • attachment patch_django_9749.20090323.diff added.

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

03/24/09 11:14:27 changed by leitjohn

  • cc set to www_djangoproject_com@leithall.com.

03/26/09 03:59:26 changed by zgoda

  • cc changed from www_djangoproject_com@leithall.com to www_djangoproject_com@leithall.com, jarek.zgoda@gmail.com.

05/07/09 04:13:21 changed by anonymous

  • cc changed from www_djangoproject_com@leithall.com, jarek.zgoda@gmail.com to www_djangoproject_com@leithall.com, jarek.zgoda@gmail.com, ales.zoulek@gmail.com.

05/07/09 04:26:58 changed by david

  • attachment patch_django_9749.20090507.diff added.

Update against r10680

06/11/09 13:19:23 changed by brosner

  • milestone set to 1.2.

07/13/09 09:56:32 changed by dc

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

07/15/09 05:20:45 changed by david

  • cc changed from www_djangoproject_com@leithall.com, jarek.zgoda@gmail.com, ales.zoulek@gmail.com to www_djangoproject_com@leithall.com, jarek.zgoda@gmail.com, ales.zoulek@gmail.com, david.

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...

08/07/09 18:57:58 changed by spinyol

  • cc changed from www_djangoproject_com@leithall.com, jarek.zgoda@gmail.com, ales.zoulek@gmail.com, david to www_djangoproject_com@leithall.com, jarek.zgoda@gmail.com, ales.zoulek@gmail.com, david, spinyol.

09/12/09 22:37:56 changed by Alex

  • stage changed from Accepted to Ready for checkin.

09/13/09 16:56:49 changed by lukeplant

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?

12/17/09 16:15:11 changed by floguy

  • attachment 9749-different-approach.diff added.

My version

12/17/09 16:19:22 changed by floguy

  • cc changed from www_djangoproject_com@leithall.com, jarek.zgoda@gmail.com, ales.zoulek@gmail.com, david, spinyol to www_djangoproject_com@leithall.com, jarek.zgoda@gmail.com, ales.zoulek@gmail.com, david, spinyol, floguy@gmail.com.

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.

12/17/09 16:45:24 changed by jezdez

  • owner changed from brosner to jezdez.
  • status changed from new to assigned.
  • version changed from 1.0 to SVN.

12/18/09 02:34:02 changed by david

@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 :)

12/18/09 04:08:05 changed by jezdez

  • status changed from assigned to closed.
  • resolution set to fixed.

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


Add/Change #9749 (ModelAdmin should allow specifying Changelist class to further modify Changelist behavior)




Change Properties
Action