Opened 16 years ago

Closed 16 years ago

Last modified 13 years ago

#7510 closed (fixed)

ModelAdmin should be able to use a non-default manager

Reported by: tom Owned by: Alex Gaynor
Component: contrib.admin Version: dev
Severity: Keywords:
Cc: gonz@…, torsten.rehn@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

At the moment the change_view and delete_view methods of
ModelAdmin in django/contrib/admin/options.py both use the
default manager of the model when retrieving an instance for
change or deletion. However, there is a "queryset" method on the
same class that is used to get the list of models to display in
the changelist. This means that to remove certain instances from
the admin one has to both update the queryset method to filter
them out and then change the 3 has_*_permission methods. This
doesn't seem very DRY to me.

I propose changing the change_view, delete_view and history_view methods so
that they simply use the queryset returned by the queryset method
instead of the default managers. This means that an filters put
on the queryset by the queryset method are honored and there is
thus only one place to change when you want to remove a subset of
objects from consideration.

This seems like a useful change to make to me and I can't see any
problems it would create but maybe I'm missing something...

Attachments (5)

use_queryset_in_delete_view_and_change_view_for_ModelAdmin.diff (1.6 KB ) - added by tom 16 years ago.
modeladmin_override_manager.diff (1.8 KB ) - added by jfw 16 years ago.
admin-qs.diff (4.0 KB ) - added by Alex Gaynor 16 years ago.
Patch with tests
options.diff (1.9 KB ) - added by jjackson 16 years ago.
Patch as of rev 8861 or Django 1.0.
admin-qs.2.diff (4.1 KB ) - added by Alex Gaynor 16 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 by jfw, 16 years ago

I also found a need to have change_view use a different queryset than the _default_manager's one. Before seeing the above patch, I had created my own. I think this one (attached) adds a little more flexibility, as it lets you override the manager used in all of the admin site. You can still override the queryset function, but it might be less necessary to do so.

by Alex Gaynor, 16 years ago

Attachment: admin-qs.diff added

Patch with tests

comment:2 by Alex Gaynor, 16 years ago

Version: newforms-adminSVN

comment:3 by Eric Holscher, 16 years ago

milestone: post-1.0
Triage Stage: UnreviewedDesign decision needed

comment:4 by Gonzalo Saavedra, 16 years ago

Cc: gonz@… added

comment:5 by jjackson, 16 years ago

Here's another patch to handle this problem. I used this to fix the problem in Practical Django Projects.

After the patch has been applied, you can then use in your model an order such as

class Entry(models.Model):
    live = LiveEntryManager()
    objects = models.Manager()

and then in your admin.py use

class EntryAdmin(admin.ModelAdmin):
    prepopulated_fields = {'slug': ('title',)}
    model_admin_manager = Entry.objects

to set the manager to be used by the admin application for the Entry class. In this case, we wanted to see only the live entries on the website, but to see all the entries in admin app. Otherwise, after setting a Entry to "draft", you would never be able to edit it again.

by jjackson, 16 years ago

Attachment: options.diff added

Patch as of rev 8861 or Django 1.0.

comment:6 by Torsten Rehn, 16 years ago

Cc: torsten.rehn@… added
Summary: Patch to have change_view and delete_view methods of ModelAdmin use queryset the queryset methodModelAdmin should be able to use a non-default manager

I changed the summary as I think the ability to use a non-default manager is more important and implies the original request to be honored.

I also want to point out that the documentation on this topic is incorrect (at least as I read it):

Django interprets this first
``Manager`` defined in a class as the "default" ``Manager``, and
several parts of Django (though not the admin application) will use
that ``Manager`` exclusively for that model.

from docs/topics/db/managers.txt:170-173 or online docs

As I interpret this, the admin is supposed to always use a "bare" Manager(), not the default manager, as it is currently doing.

Plus, this makes it impossible to use a custom manager in related lookups, but not the admin (use_for_related_fields can only be used in the default manager, I don't really know why there is no Model.Meta.related_manager option).

comment:7 by Alex Gaynor, 16 years ago

Owner: changed from nobody to Alex Gaynor

comment:9 by Chris Beaven, 16 years ago

milestone: post-1.0
Needs documentation: set
Triage Stage: Design decision neededAccepted

scel is correct that the current documentation is out of date.

brosner agrees that this is issue valid.

A small suggestion for the patch: just set model_manager = None as a ModelAdmin class attribute, then you don't have to worry about hasattr and it's a bit more of an obvious API attribute (model_admin_manager seems overly verbose - of course it's for admin ;))

comment:10 by Niels Sandholt Busch, 16 years ago

Cc: niels.busch@… added

comment:11 by Niels Sandholt Busch, 16 years ago

Cc: niels.busch@… removed

comment:12 by mrts, 16 years ago

milestone: 1.2
Needs tests: set
Patch needs improvement: set

Looks useful, tentatively pushing to 1.2.

by Alex Gaynor, 16 years ago

Attachment: admin-qs.2.diff added

comment:13 by Jacob, 16 years ago

Resolution: fixed
Status: newclosed

(In [10314]) Fixed #7510: the ModelAdmin now uses self.queryset instead of the default manager. Thanks, Alex Gaynor.

comment:14 by Jacob, 13 years ago

milestone: 1.2

Milestone 1.2 deleted

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