Opened 16 years ago

Closed 17 months ago

Last modified 17 months ago

#9602 closed New feature (fixed)

Add admin.site._registry manipulation methods

Reported by: Rob Hudson Owned by: Mariusz Felisiak
Component: contrib.admin Version: dev
Severity: Normal Keywords: register modeladmin
Cc: rob@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This is a feature request...

Currently, one can unregister a model from the admin and register it again with a new ModelAdmin. The problem I'm anticipating is when 2 or more apps want to add ModelAdmin options to a Model and the last one wins.

Not to point out a specific 3rd party app, but this is an easily contrived example... If I have my own apps and one of them unregisters the User model's ModelAdmin and registers its own that, say, adds the is_superuser to the list_display. Then if I add the django-openid app that (currently) also unregisters the User model's ModelAdmin and registers its own that adds an inline to a ManyToManyField for the OpenID associations tied to that user. If django-openid is after my app in the INSTALLED_APPS list, I lose my list_display override. And if my app is after the django-openid app, I lose the OpenID associations inlines. (See: http://code.google.com/p/django-openid/source/browse/trunk/django_openid/admin.py)

It's possible currently to write the unregistration/registration such that this doesn't happen, but it relies on pulling the ModelAdmin out of the "private" _registry dictionary in the admin.site class. For example:

useradmin = admin.site._registry.get(User, None)
if useradmin:
    useradmin.list_display = useradmin.list_display + ('is_superuser',)
else:
    class MyUserAdmin(AuthUserAdmin):
        list_display = ('username', 'email', 'first_name', 'last_name', 'is_staff', 'is_superuser')
    admin.site.register(User, MyUserAdmin)

I can think of a few ways of fixing this, from least involved to more involved:

  1. At the very least I think it would be nice if the internal _registry dictionary didn't have the prepended underscore to signify that it is a private variable not to be touched, so one doesn't feel dirty doing something like this.
  1. I think it would be a bit cleaner if there were methods to lookup, get, and update this dict and keep it as a private dict. For example, something like admin.site.get_model_admin(User)?
  1. Along with 2, provide methods to update common ModelAdmin options, like list_display. This one is definitely venturing into debatable area, but something like useradmin.list_display.add('is_superuser') takes much away from this feeling so "monkey patchy".

Change History (8)

comment:1 by Jacob, 16 years ago

Triage Stage: UnreviewedDesign decision needed

comment:2 by Luke Plant, 14 years ago

Severity: Normal
Type: New feature

comment:3 by Julien Phalip, 13 years ago

Easy pickings: unset
Triage Stage: Design decision neededAccepted
UI/UX: unset

Accepting that there should be a cleaner API for interacting with the registry. However the third point ("provide methods to update common ModelAdmin options") doesn't seem so useful.

comment:4 by anonymous, 11 years ago

Easy pickings: set
Has patch: set
Needs documentation: set
Needs tests: set
Owner: changed from nobody to anonymous
Status: newassigned
useradmin = admin.site._registry.get(User, None)
if useradmin:
    useradmin.list_display = useradmin.list_display + ('is_superuser',)
else:
    class MyUserAdmin(AuthUserAdmin):
        list_display = ('username', 'email', 'first_name', 'last_name', 'is_staff', 'is_superuser')
    admin.site.register(User, MyUserAdmin)
Last edited 10 years ago by Tim Graham (previous) (diff)

comment:5 by Tim Graham, 11 years ago

Easy pickings: unset
Has patch: unset
Needs documentation: unset
Needs tests: unset
Version: 1.0master

comment:6 by Mariusz Felisiak, 17 months ago

Has patch: set
Owner: changed from anonymous to Mariusz Felisiak

comment:7 by GitHub <noreply@…>, 17 months ago

Resolution: fixed
Status: assignedclosed

In f64fd47:

Fixed #9602 -- Added AdminSite.get_model_admin().

This allows retrieving an admin class for the given model class without
using internal attributes.

comment:8 by GitHub <noreply@…>, 17 months ago

In 2584783:

Refs #9602 -- Moved AlreadyRegistered/NotRegistered exceptions to django.contrib.admin.exceptions.

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