#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)
Change History (28)
comment:1 by , 16 years ago
Cc: | added |
---|---|
Needs documentation: | set |
Needs tests: | set |
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Design decision needed |
comment:2 by , 16 years ago
Triage Stage: | Design decision needed → Accepted |
---|
comment:3 by , 16 years ago
Cc: | removed |
---|---|
Owner: | changed from | to
Status: | new → assigned |
by , 16 years ago
Attachment: | patch_django_9749.20090111.diff added |
---|
First iteration, review welcome
comment:4 by , 16 years ago
Has patch: | set |
---|---|
Needs documentation: | unset |
Needs tests: | unset |
Patch needs improvement: | unset |
comment:5 by , 16 years ago
Patch needs improvement: | set |
---|
It seems that more tests are required in admin_views. I'll work on that part.
comment:7 by , 16 years ago
Owner: | changed from | to
---|---|
Status: | assigned → new |
by , 16 years ago
Attachment: | patch_django_9749.20090317.diff added |
---|
New patch against r10069 (another bug resolution was conflictual with tests), nothing has changed
by , 16 years ago
Attachment: | patch_django_9749.20090323.diff added |
---|
Update against r10123 with feature freeze it will be more simple to maintain now
comment:8 by , 16 years ago
Cc: | added |
---|
comment:9 by , 16 years ago
Cc: | added |
---|
comment:10 by , 16 years ago
Cc: | added |
---|
comment:11 by , 16 years ago
milestone: | → 1.2 |
---|
comment:12 by , 15 years ago
Maybe use 'changelist' instead of 'changelist_class'? Because currently admin has attribute 'form' (not 'form_class').
comment:13 by , 15 years ago
Cc: | 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 , 15 years ago
Cc: | added |
---|
comment:15 by , 15 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:16 by , 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?
comment:17 by , 15 years ago
Cc: | 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 , 15 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Version: | 1.0 → SVN |
comment:19 by , 15 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 , 15 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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).