-rob
Sure, now is as good a time as any!
In the newforms-admin branch, I've been doing a bunch of cleaning up
of the admin code and adding more hooks for customization. Until now,
all admin options have been static data structures, such as
list_display and list_filter, but now I've started to add hooks that
are *methods* on the "class Admin." A good example is the
has_change_permission() hook. You can specify that method on your
"class Admin" to implement more finely-grained permission schemes on a
per-object or per-user basis -- the method is passed both the request
object and ID of object you're changing.
In order to get this to work, I've refactored the way "class Admin"
works so that it magically becomes a subclass of
django.contrib.admin.options.ModelAdmin. We should decide whether this
magic subclassing should be removed in favor of an explicit
subclassing -- I'm in favor of being explicit here.
But the bigger question is, should the "class Admin" even live in the
model at all, given that it's gaining many more hooks, not all of
which are simple data structures. With each hook I add, it feels more
and more messy to have that functionality within the model. Of course,
having it in the model requires very little typing and no extra Python
modules to manage, so if we were to move "class Admin" somewhere else,
we'd lose that convenience.
The goal of this thread, then, should be to come up with a design that
satisfies both requirements: "class Admin" shouldn't feel out of
place, yet it shouldn't be a tremendous pain to declare admin
options/functionality.
Adrian
--
Adrian Holovaty
holovaty.com | djangoproject.com
-rob
It feels right to me to have admin and the model separate. The way I think of it
is this: Suppose I distribute an app and a user decides to use the app, but
doesn't run the auto-admin. With my admin information factored out in a separate
file: admin.py, she's not loading code that she'll never use.
It also makes it feel like admin is just another (albeit kickarse) Django app,
just like any other. It's not magically intertwined with model classes unlike
other apps.
Presumably if I was writing other apps that hooked into existing apps, they'd
have their own separate file for doing that, like an admin.py.
--
Lach
Personal: http://illuminosity.net/
The newforms-admin branch already has some of those extra hooks, so
that should make you very happy. :-)
> The only part that I'm still hung on is how to specify the URL's cleanly so
> that both user specified and the defaults work smoothly together. But I
> think it should not be too hard to do.
This is exactly what I'm trying to solve. How does the URLconf
interact with the "class Admin" so that not too much extra code is
required?
why not create a function to do that for you..
urls.py is after all a python module, so you could just introduce a function
urlpatterns = admin_urls_for_model(
MyModel,
field_list=[ 'field' ],
exclude_actions=[ admin.EDIT ],
perms_required={
admin.EDIT : 'can_change_MyModel',
admin.LIST : 'something_else'
}
)
or mimic the patterns:
urlpatterns = admin_urls( 'MyModel',
(r'path/', 'add', option_dict ),
(r'path/list/', 'list', option_dict ),
(r'path/', 'edit', option_dict ),
)
where MyModel could be omitted and specified in url...
the default values could be taken from the model (for backwards
compatibility) and/or anyplace else...
just my 2c's
>
> Adrian
>
> --
> Adrian Holovaty
> holovaty.com | djangoproject.com
>
> >
>
--
Honza Král
E-Mail: Honza...@gmail.com
ICQ#: 107471613
Phone: +420 606 678585
I must not have explained this correctly. In the newforms-admin
branch, all of those options are specified in a class, ModelAdmin. The
goal here is to figure out how those classes interact with the
URLconf. There's no need to pass dictionaries around -- all of that
configuration is in the class.
Something like this is what I had in mind:
"""
from myapp.models import PollAdmin, ChoiceAdmin
urlpatterns = patterns('',
(r'^admin/', include('django.contrib.admin.urls'), {'models':
(PollAdmin, ChoiceAdmin)})
)
"""
The problem here is that each time you add an admin interface to a
model, you have to remember to add it to your URLconf. Maybe there can
be a helper function that looks for all Admin classes, as long as you
save them in a file called admin.py within the app, but that's kind of
magic.
I generally like the simplicity and explicitness of your suggestion,
but having to manually add & remove models is too fragile, I think.
> Maybe there can be a helper function that looks for all Admin classes, as long as you save them in a file called admin.py within the app, but that's kind of magic.
If you explicitly reference the module in the urls.py file, it is
simple without feeling magic. e.g.
urlpatterns = patterns('',
(r'^admin/', include('django.contrib.admin.urls'), {'model_loc':
('mysite.polls.models.admin',)})
)
This would rely on introspection to find out which ModelAdmin classes
exist in the module, but it makes it pretty simple to turn it on/off or
add/remove modules (assuming multiple modules can be specified via the
tuple argument).
-rob
I'm -1 on adding it to URLconf and would rather see it in settings.py.
Perhaps like:
ADMIN_FIND_ADMIN_CLASSES = True
ADMIN_MODELS = (
'myproj.myapp.admin.PollAdmin',
'someotherproj.someapp.admin.FooAdmin',
)
Where ADMIN_FIND_ADMIN_CLASSES (or whatever we want to call it)
permits autodiscovery of the ModelAdmin classes in the app/admin.py
files. The ADMIN_MODELS settings are explicit ModelAdmins that would
override anything autodiscovered or mbe the only ModelAdmins if
ADMIN_FIND_ADMIN_CLASSES = False.
No Magic there (I think). This would allow developers to have multiple
ModelAdmins for a model and switch between them depending on the
application's deployment requirements.
regards
matthew
ps. if the ModelAdmin class moves out of the inner class is it going
to gain a 'model' attribute?
pps. I'm +1 on the inner Admin class explicitly subclassing ModelAdmin
as opposed to the current magic.
I'm -1 on adding it to URLconf and would rather see it in settings.py.
Perhaps like:
ADMIN_FIND_ADMIN_CLASSES = True
ADMIN_MODELS = (
'myproj.myapp.admin.PollAdmin',
'someotherproj.someapp.admin.FooAdmin',
)
Where ADMIN_FIND_ADMIN_CLASSES (or whatever we want to call it)
permits autodiscovery of the ModelAdmin classes in the app/admin.py
files. The ADMIN_MODELS settings are explicit ModelAdmins that would
override anything autodiscovered or mbe the only ModelAdmins if
ADMIN_FIND_ADMIN_CLASSES = False.
pps. I'm +1 on the inner Admin class explicitly subclassing ModelAdmin
as opposed to the current magic.
I'd like to second this notion. +1.
Will.
My 2 (belated) cents on the issue: the settings.py idea is the best
I've seen in the thread so far. Let me throw in another option: maybe
it's time to return to Meta? In the dawn before time
(pre-magic-removal) Meta housed all of the Admin options until it was
determined which merely served the Admin itself and which were more
generally useful, breaking things into two inner-classes, Meta and
Admin. If we want to deprecate the inner-Admin class it makes sense to
me that we can use the Meta class we still have lying around for odd
jobs (like ordering and verbose_name_plural). For instance::
class Meta:
show_in_admin = True
admin_model = ModelAdmin({
'list_display': ... })
Certainly that should look eerily familiar to pre-0.95 developers. You
could easily then do something like::
admin_model = ModelAdmin(ModelName.Admin.__dict__)
This becomes the easy to add explicit "magic" sub-typing of the inner
Admin class, and you just have the Admin assume until deprecated (ie,
one version later when oldforms is deprecated) that the above line
already exists in Meta if it doesn't exist. For simple classes
up-conversion is simply add two lines to Meta (show_in_admin and
admin_model).
This keeps Admin information inside the model (which I'm still
uncertain if that is a Bad Thing just yet) and yet still adds a good
amount of explicitness. Those worried about "admin contamination" of
apps for non-admin users could handle that difficulty using something
akin to::
if use_admin_flag:
from django.contrib.admin import ModelAdmin
else:
def ModelAdmin(dict):
pass
>>ADMIN_FIND_ADMIN_CLASSES = True
>>ADMIN_MODELS = (
>> 'myproj.myapp.admin.PollAdmin',
>> 'someotherproj.someapp.admin.FooAdmin',
>>)
>
> [...] Let me throw in another option: maybe
> it's time to return to Meta? [...] For instance::
>
> class Meta:
> show_in_admin = True
> admin_model = ModelAdmin({
> 'list_display': ... })
The reason I don't like this is it still leaves models tied to admin, rather
than the other way around. If someone wants to not have the admin displayed for
a particular app, it should be as easy as deleting or renaming a file, imo, you
shouldn't need to change it in other places.
I don't think there's a clean way out of this without resorting to another
setting, after thinking on it for a while.
My suggestion?
ADMIN_MODEL_FILE = 'admin.py'; set to this by default so most people won't even
need to think about setting it themselves -- but is is settable if someone
really wants to change it.
That's not enough, though. It should be specifiable per app, so that if you have
good reasons for wanting to change it per app -- or want to integrate an app
from someone else using a different name it's possible.
So I think an app's __init__.py should have the *option* of overriding the
ADMIN_MODEL_FILE setting, by settings its own variable called ADMIN_MODEL_FILE,
and pointing to the specific module for this app.
Theoretically, you could do this for all apps, and have that added to manage.py
's auto-generation, but I don't like that as much, since someone who does like
something different would have to change it for every app they create. Not that
that's a huge deal, either.
My suggestion is that admin looks at every installed app. For every installed
app, it gets settings's ADMIN_MODEL_FILE. Then, if the app itself sets one, it
replaces the current ADMIN_MODEL_FILE value with the app's one.
If the file indicated by one of these settings exists, then its admin-models are
loaded and used. If the file doesn't exist, admin assumes that this app has no
admin interface -- that is, it's not an error to point to an admin file that
isn't there.
How does this sound?
-
On a slightly related note, this would set a precedent for settings files.
Should settings files be used to store app specific settings? This would
indicate a solid yes. Maybe, though, each app should have its own settings file,
instead, and admin could point the way by doing things that way? The advantage
being that it keeps the main settings file relatively uncluttered the
disadvantage being that suddenly we could get a proliferation of relatively
empty files.
The problem with that is that there is a good amount of description
per model (list_display, et al in the current inner class) that I
would hate to have to move to other files because I think it mentally
makes sense to keep with the model because: a) It's "meta-information"
on the model itself and b) it's "meta-information" that yes is
currently 95% used by the Admin but can be just as useful to other
applications. I've certainly felt that smarter versions of the
list_detail generic views might take into account list_display and
list_filter.
Perhaps I'm just an old fogey on this issue, but I'm certainly not yet
convinced that entirely moving what's currently in the Admin inner
class is a good idea. Better theoretically perhaps, but from a
practical standpoint I'd certainly rather not have this information in
two separate files when it 95%+ of it is descriptive model-level
information. I can understand, at a project level, if I think I'm
becoming too dependent on the Admin refactoring that data into
separate files, but forcing it from the start gets us closer to the
slippery slope of scaffolding magic...
> My suggestion?
>
> ADMIN_MODEL_FILE = 'admin.py'; set to this by default so most people won't even
> need to think about setting it themselves -- but is is settable if someone
> really wants to change it.
[snip]
> How does this sound?
Yuck. -1. Too much magic. Create an admin.py for some other purpose
and watch as errors crop up or things start working differently in
other applications. Worse, this should solely be Python relative dot
notation so the above would become just ADMIN_MODEL_MODULE = 'admin',
which then adds the possibility, until everyone standardizes on the
new relative dot notation, of huge blow ups if you choose to name a
project, application or already happen to have an existing Python
library named admin.
For new users trying to use the Admin app you are effectively either
going to have to hand-hold them hugely through the process of "create
the magic named file, add these incantations, pray no errors are
returned" or you scaffold these files for them and then hand-wave
around it like those other guys do when it comes time to do a screen
cast.
Magic-named inner class is more than enough as it is.
--
--Max Battcher--
http://www.worldmaker.net/
All progress is based upon a universal innate desire on the part of
every organism to live beyond its income. --Samuel Butler
There's no reason values useful for more than admin can't be stored in meta.
There's nothing to stop admin reusing model wide metadata, but it shouldn't be
polluting the main model file with its own specific settings.
> Yuck. -1. Too much magic. Create an admin.py for some other purpose
> and watch as errors crop up or things start working differently in
> other applications.
Obviously I haven't explained this well enough. Admin would look for the
admin.py file in the app, and then it would use any classes in the file that
inherit from AdminBase.. as Adrian is doing things already, if I understand
correctly. You could do whatever else with the file you wanted; you could have
an admin.py file without any adminbase inheriting classes, and it'd all be left
alone by admin.
> Worse, this should solely be Python relative dot
> notation so the above would become just ADMIN_MODEL_MODULE = 'admin',
> which then adds the possibility, until everyone standardizes on the
> new relative dot notation, of huge blow ups if you choose to name a
> project, application or already happen to have an existing Python
> library named admin.
It's only looking for these admin.py files inside installed apps -- in other
words, if your installed app is registered as 'contact' it'd try to import
'contact.admin'. If your installed app is 'myproj.big.app.hierarchy' it'd import
'myproj.big.app.hierarchy.admin'. If you've got local files that are clashing
with your installed apps' names, then you have problems independent of admin
that you need to resolve.
> For new users trying to use the Admin app you are effectively either
> going to have to hand-hold them hugely through the process of "create
> the magic named file, add these incantations, pray no errors are
> returned" or you scaffold these files for them and then hand-wave
> around it like those other guys do when it comes time to do a screen
> cast.
I don't see how this is 'magic'. Magic, as discussed here, is all about doing
things implicitly.. assuming what the user wants. Allowing the admin file name
to be set and overridden is the exact opposite of that -- it's setting things
explicitly, with sensible defaults.
Again, no errors because admin only cares about its own classes. And creating an
admin.py file is surely no harder than creating a urls.py, etc., for your app.
> Magic-named inner class is more than enough as it is.
This'd remove the magic from it. It's explicitly saying what to look for, not
'name your class admin, and we'll use it for the admin'.
--
Lach
Personal: http://illuminosity.net/
Business: http://taravar.com/
No problem. I agree that decoupling is a worthy *goal*. But I'm
certainly going to need convincing to drop the current, practical,
declarative approach for another purely on theoretical beauty. What
effort does moving admin stuff into a different file gain me? How
much effort am I going to need to upgrade just to fit with your
"better academic approach"? Those are my arguments here. I certainly
see no reason that many current uses of the inner-Admin class need
deprecate at all. It's certainly OK in Python for a duck-test... if
there is something resembling (looks, smells, quacks like it) an old
"static inner Admin declaration", the Admin could do its best to
follow it.
Basically, someone needs to play advocate for all the poor people that
might look at the migration path for this change and think "this is
Magic-Removal all over again". Particularly with the Meta -> Admin
switcharoo that's still fresh in some people's minds.
> Obviously I haven't explained this well enough. Admin would look for the
> admin.py file in the app, and then it would use any classes in the file that
> inherit from AdminBase.. as Adrian is doing things already, if I understand
> correctly. You could do whatever else with the file you wanted; you could have
> an admin.py file without any adminbase inheriting classes, and it'd all be left
> alone by admin.
Unfortunately Python doesn't work that way. Imports and import
attempts have side effects in current versions of Python. You cannot
just iterate through every "admin.py" file in a project and hope that
there aren't any errors or conflicts you just opened up. Not to
mention that in mod_python and similar setups every import you make
has a long term impact on memory usage.
Model loading is the only thing I can think of that "vacuums" in a
large number Python modules in that way and adding a second solely for
the Admin application quite possibly could anger a lot of potential
Admin users.
> I don't see how this is 'magic'. Magic, as discussed here, is all about doing
> things implicitly.. assuming what the user wants. Allowing the admin file name
> to be set and overridden is the exact opposite of that -- it's setting things
> explicitly, with sensible defaults.
No, it's not. Sure you have 1 explicit setting that you might
remember in a year has an effect on the process when you are trying to
debug that subtle but unfortunate bug, but you are sucking in a lot of
excess data from a single setting, and there is very little precedent
for that. In most cases Django settings are precisely focused:
DJANGO_SETTINGS_MODULE points to a single module and so does
ROOT_URLCONF.
Keep in mind, I pointed out that the least vile approach I've seen so
far is using an setting that defines Admin models on a per-model
basis. It's not the cleanest, but it's certainly precisely explicit
and I can think of several utility functions I might write to remove a
majority of the tedium from editing that list. Any ADMIN_MODELS
setting should follow the example of ROOT_URLCONF or the
aforementioned plan and point to a either a list of specific classes
or a single specific module. I still don't think either is the best
possible approach, and I certainly think that there is still good
enough reason to leave the majority of my projects' admin information
where it currently resides, but I'm willing to be convinced.
> Again, no errors because admin only cares about its own classes. And creating an
> admin.py file is surely no harder than creating a urls.py, etc., for your app.
As I said, in the case of Python as it currently exists it is
unbelievably easy to trigger odd bugs and weird errors solely due to
importing one more module than you expected. It's unfortunate, but it
is simply the case that Python runs code in a module on import whether
you "only care about certain subclasses within the module" or not.
Module import and module introspection have prices, even if as a
dynamic language there is certain ease of doing such things.
What if we made the admin app scan all installed applications in its
urls.py file for admin classes in models or admin modules?
Currently we have this in django.contrib.admin.urls:
# Model-specific admin pages.
('^([^/]+)/([^/]+)/(?:(.+)/)?$', 'django.contrib.admin.views.main.model_admin_view'),
This pulls us out of urlconf into a view to decide what the real view
is. Instead we could continue adding to urlpatterns for any admin
classes we find.
# This is pseudo-python
def find_admin_urlpatterns:
for app in settings.INSTALLED_APPS:
if app.admin:
urlpatterns += app.admin.urlpatterns
else:
for model in app.models:
if model.Admin:
urlpatterns += model.Admin.urlpatterns
With this method we could support the old style of Admin class in
models and whatever new way we want for extending the ModelAdmin class
using admin.py in each app.
Nate
On 31 tammi, 06:24, Nate Straz <nate-dja...@refried.org> wrote:
> With this method we could support the old style of Admin class in
> models and whatever new way we want for extending the ModelAdmin class
> using admin.py in each app.
I like the idea that you have simple or none admin in model and more
complicated or limited admin views in separate files.
As far as I can see, I think most common situation is that you have
multiple admins for same model in same app, like admin_full.py,
admin_limited.py and similar, so to be able to choose admin view by
different type of testing could be needed. But cross site/application
selecting of admin should be possible too.
Like for example you probably want to show for certain user group only
"admin_limited" and for all master users the "admin_full", in this
case the testing method is permission, but you could use any type of
testing.