Opened 8 years ago

Closed 7 years ago

Last modified 5 years ago

#7510 closed (fixed)

ModelAdmin should be able to use a non-default manager

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


At the moment the change_view and delete_view methods of
ModelAdmin in django/contrib/admin/ 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 8 years ago.
modeladmin_override_manager.diff (1.8 KB) - added by jfw 8 years ago.
admin-qs.diff (4.0 KB) - added by Alex 8 years ago.
Patch with tests
options.diff (1.9 KB) - added by jjackson 8 years ago.
Patch as of rev 8861 or Django 1.0.
admin-qs.2.diff (4.1 KB) - added by Alex 7 years ago.

Download all attachments as: .zip

Change History (19)

Changed 8 years ago by jfw

comment:1 Changed 8 years ago by jfw

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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.

Changed 8 years ago by Alex

Patch with tests

comment:2 Changed 8 years ago by Alex

  • Version changed from newforms-admin to SVN

comment:3 Changed 8 years ago by ericholscher

  • milestone set to post-1.0
  • Triage Stage changed from Unreviewed to Design decision needed

comment:4 Changed 8 years ago by gonz

  • Cc gonz@… added

comment:5 Changed 8 years ago by jjackson

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

Changed 8 years ago by jjackson

Patch as of rev 8861 or Django 1.0.

comment:6 Changed 8 years ago by scel

  • Cc torsten.rehn@… added
  • Summary changed from Patch to have change_view and delete_view methods of ModelAdmin use queryset the queryset method to ModelAdmin 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 Changed 8 years ago by Alex

  • Owner changed from nobody to Alex

comment:9 Changed 8 years ago by SmileyChris

  • milestone post-1.0 deleted
  • Needs documentation set
  • Triage Stage changed from Design decision needed to Accepted

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

  • Cc niels.busch@… added

comment:11 Changed 8 years ago by niels

  • Cc niels.busch@… removed

comment:12 Changed 7 years ago by mrts

  • milestone set to 1.2
  • Needs tests set
  • Patch needs improvement set

Looks useful, tentatively pushing to 1.2.

Changed 7 years ago by Alex

comment:13 Changed 7 years ago by jacob

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:14 Changed 5 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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