Opened 7 years ago

Closed 7 years ago

Last modified 4 years ago

#6003 closed (fixed)

Registration for newforms admin

Reported by: Petr Marhoun <petr.marhoun@…> Owned by: nobody
Component: contrib.admin Version: newforms-admin
Severity: Keywords: nfa-blocker
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Some points to registration for newforms admin:

  • One project can have many application, one application can be part of many projects.
  • One project can have several admins.
  • One application can be used in several admins from one project.
  • One application can be registered in one project and not registered in another project.
  • One application can be used with default ModelAdmin in one project but with unknown customatization in another project.
  • Explicit is better than implicit, magic is evil.

I would like to propose new conventions. There would be new files admin.py in some applications with method register. Example:

from django.contrib import admin
from django.contrib.sites import models

class SiteAdmin(admin.ModelAdmin):
    list_display = ('domain', 'name')
    search_fields = ('domain', 'name')

def register(site):
    site.register(models.Site, SiteAdmin)

There would be admin.py in each project. Example:

from django.contrib import admin

# Import admins from your or contrib applications.
from django.contrib.auth import admin as auth_admin
from django.contrib.sites import admin as sites_admin

# Register admins from your or contrib applications.
admin_site = admin.AdminSite()
auth_admin.register(admin_site)
sites_admin.register(admin_site)

And it would be called from urls.py. Example:

from django.conf.urls.defaults import *

import admin

urlpatterns = patterns('',
    ('^admin/(.*)', admin.admin_site.root),
)

I say "is" but I mean "convention is".

Attachments (2)

00-admin-registration.diff (11.9 KB) - added by Petr Marhoun <petr.marhoun@…> 7 years ago.
00-admin-registration.2.diff (10.0 KB) - added by Petr Marhoun <petr.marhoun@…> 7 years ago.

Download all attachments as: .zip

Change History (10)

Changed 7 years ago by Petr Marhoun <petr.marhoun@…>

comment:1 Changed 7 years ago by Karen Tracey <kmtracey@…>

  • Keywords nfa-blocker added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

I got rather lost in the bulleted list of "one can have/be in many" but from the patch I gather this does things like adapt manage.py startapp/startproject to handle the new admin registration requirement, and modifies the registration for Django-provided apps to follow the proposed new convention. Someone with a better understanding of the issues than I needs to make a design decision on the proposed convention, and it needs to be settled before merge since guidance must be provided (once, hopefully) to everyone migrating as to how to do it when the merge happens.

Changed 7 years ago by Petr Marhoun <petr.marhoun@…>

comment:2 Changed 7 years ago by garcia_marc

  • milestone set to 1.0 alpha

comment:3 Changed 7 years ago by russellm

  • Triage Stage changed from Design decision needed to Accepted

Accepted on the basis that conventions for admin registration are a good idea; however, the exact format of those conventions needs to be finalized.

Personally, I'm agreed that:

  • admin.py should be the convention for admin registrations in an app.
  • startapp should create an empty admin.py file for each app

The register(site) method in app/admin.py is an interesting convention, although I'm not convinced it's required. If the registration calls are at the module level, importing the module will do all the registrations.

I'm also not convinced that there is a need for project-level admin.py. Ideally, the admin registrations would be picked up automatically by virtue of importing an application plus defining and deploying an admin site. As a fallback, I have no problem with expecting the root urls.py to import all the admin modules that are required. Introducing a project level admin.py just seems like overkill to me.

comment:4 follow-up: Changed 7 years ago by anonymous

The register(site) method in app/admin.py is an interesting convention, although I'm not convinced it's required. If the registration calls are at the module level, importing the module will do all the registrations.

I disagree, I want full control over what site apps register themselves, and it's a lot more DRY-ful to specify what site you want in one place (The register(site) call) than it is to have it defined 10 different times by calling CustomAdminSite.register(model,modeladmin).

I also want full control over where apps I haven't created myself get registered. Specifically, I wanted to alter the way Users are displayed in the admin site, but the auth app registers the user admin to the root autmatically, meaning I had to create my own admin site and register everything there. I am picturing frustrations with importing third-party apps only to find they've registered themselves to an adminsite I don't want them to be in, and that could be solved by the app/admin.py register(site) idea. Ultimately the decision of what admin stuff to put in what model in what site should be at the project level, so why not standardize it to be decided at the project level?

comment:5 in reply to: ↑ 4 Changed 7 years ago by russellm

Replying to anonymous:

I disagree, I want full control over what site apps register themselves, and it's a lot more DRY-ful to specify what site you want in one place (The register(site) call) than it is to have it defined 10 different times by calling CustomAdminSite.register(model,modeladmin).

This presumes that each site will display the model the same way in admin. This will certainly be a common arrangement, but it is by no means required. I'd be opposed to hard coding this limitation into the convention.

On the mailing list, Brian and Joseph have indicated that they have given this topic some thought; lets wait until they get their thoughts together before we get too involved in discussion on this ticket.

comment:6 Changed 7 years ago by mrts

#7497 and Usability improvements in application handling are related and would enhance this. It would be nice to merge relevant bits from there and close #7497 as duplicate if it's OK with everyone.

comment:7 Changed 7 years ago by brosner

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

(In [7872]) newforms-admin: Added autodiscover functionality to django.contrib.admin. This makes the admin aware of per-app admin.py modules and does an import on them when explicitly called. Docs show how this is used. Fixed #6003, #6776, #6776.

comment:11 Changed 4 years ago by jacob

  • milestone 1.0 alpha deleted

Milestone 1.0 alpha deleted

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