If you use the newforms-admin branch and the edit inline functionality, you want to read this.
I'm planning on breaking the current syntax for defining inlines in the very near future, but I figured I'd warn people and ask for comments first. Currently it's something like this:
class MyModelAdmin(ModelAdmin): inlines = [ StackedInline(MyModel, extra=2) ]
In order to fix a few newforms-admin bugs, the inline class will take the parent model and the AdminSite object as arguments to its constructor. This means that the 'inlines' attribute will need to be a list of classes rather than a list of instances now. The most obvious way to do it looks like this:
class MyInline(StackedInline): model = MyModel extra = 2
class MyModelAdmin(ModelAdmin): inlines = [ MyInline ]
I'm still thinking about a more convenient syntax for the simple case, probably a couple of factory functions that act like the old StackedInline and TabularInline classes.
I'm thinking something like:
class MyModelAdmin(ModelAdmin): inlines = [ stacked(MyModel, extra=2) ]
where stacked is a function that makes a new class that inherits from StackedInline, applies the kwargs to that class and returns it. Other ideas are welcome.
On 9/9/07, Joseph Kocherhans <jkocherh...@gmail.com> wrote:
> If you use the newforms-admin branch and the edit inline > functionality, you want to read this.
> I'm planning on breaking the current syntax for defining inlines in > the very near future, but I figured I'd warn people and ask for > comments first. Currently it's something like this:
I don't have any partiular objections to the new syntax, but can you give a quick rundown of why these changes are necessary? The only reason that comes to my Sunday-addled brain at the moment is that it would make 'inline within an inline' forms easier to write. Is this the reason, or is there another that I'm missing?
Also - how does this affet the proposal to merge the 'inlines' clause into the 'fields' clause (and following your other proposal - the 'fieldsets' clause)? Again, I can't see any reason why it shouldn't be possible, but I wanted to make sure we weren't making a rod for our own backs.
On 9/9/07, Russell Keith-Magee <freakboy3...@gmail.com> wrote:
> On 9/9/07, Joseph Kocherhans <jkocherh...@gmail.com> wrote:
> > If you use the newforms-admin branch and the edit inline > > functionality, you want to read this.
> > I'm planning on breaking the current syntax for defining inlines in > > the very near future, but I figured I'd warn people and ask for > > comments first. Currently it's something like this:
> I don't have any partiular objections to the new syntax, but can you > give a quick rundown of why these changes are necessary? The only > reason that comes to my Sunday-addled brain at the moment is that it > would make 'inline within an inline' forms easier to write. Is this > the reason, or is there another that I'm missing?
> Also - how does this affet the proposal to merge the 'inlines' clause > into the 'fields' clause (and following your other proposal - the > 'fieldsets' clause)? Again, I can't see any reason why it shouldn't be > possible, but I wanted to make sure we weren't making a rod for our > own backs.
I would like for the inlines to be separate, the reason is that in our system, we have quite a few generic stuff that can bind to any model and I would like to be able to register these generic models to any model's admin without the model's knowledge. Like this I could keep the basic model options simple and uphold the DRY principle.
For example (I know it can be confusing, like my explanations ;) ): class Article( models.Model ): ... some_fields_go_here ... class ArticleOptions( admin.ModelAdmin ): ... some_basic_options ...
On 9/9/07, Russell Keith-Magee <freakboy3...@gmail.com> wrote:
> On 9/9/07, Joseph Kocherhans <jkocherh...@gmail.com> wrote:
> > If you use the newforms-admin branch and the edit inline > > functionality, you want to read this.
> > I'm planning on breaking the current syntax for defining inlines in > > the very near future, but I figured I'd warn people and ask for > > comments first. Currently it's something like this:
> I don't have any partiular objections to the new syntax, but can you > give a quick rundown of why these changes are necessary? The only > reason that comes to my Sunday-addled brain at the moment is that it > would make 'inline within an inline' forms easier to write. Is this > the reason, or is there another that I'm missing?
Would you believe me if I just said it makes the internals *much* more consistent and easier to understand? It's part of a refactoring that changes the wrapper classes related to inlines use a lot more of the existing admin form wrapper constructs. A desire to refactor things and http://code.djangoproject.com/ticket/4491 is where this started.
I suppose I could make the syntax backwards-compatible by naming the factory functions the same as the existing classes, but I think it would make things more confusing.
> Also - how does this affet the proposal to merge the 'inlines' clause > into the 'fields' clause (and following your other proposal - the > 'fieldsets' clause)? Again, I can't see any reason why it shouldn't be > possible, but I wanted to make sure we weren't making a rod for our > own backs.
If we decide to put inline definitions into fields or fieldsets, this change won't make it any more difficult. I'd rather cross that bridge a little later though. I have thought about both the syntax and the implementation for that and I have a few ideas, but I don't have the time to dedicate to it just yet.
On Sun, 2007-09-09 at 10:56 -0500, Joseph Kocherhans wrote:
[...]
> I suppose I could make the syntax backwards-compatible by naming the > factory functions the same as the existing classes, but I think it > would make things more confusing.
I think you're more across the subtleties of this feature a lot more than I am, Joseph, so I don't really have a strong opinion about it. However, this quoted bit caught my attention.
My gut reaction is that this would be trying to over-finesse things a bit. I mean, code related to Admin configuration already has to change; it's already a once-off disruption. Providing some backwards-compat wrapper here isn't really going to ease that burden enough to be worth the forwards maintenance hassle, so just go with the one (right) way to do things and avoid confusion.
On 9/8/07, Joseph Kocherhans <jkocherh...@gmail.com> wrote:
> If you use the newforms-admin branch and the edit inline > functionality, you want to read this.
> I'm planning on breaking the current syntax for defining inlines in > the very near future, but I figured I'd warn people and ask for > comments first.
I've checked this in as r6072. No factory functions yet. I'll probably hold off on those until the sprint, or start another thread.
I have a problem with this changeset. I have some inlines with foreign keys. By default there is one SQL query for each row - result is not cached. So I used this code in formfield_for_dbfield:
if isinstance(db_field, models.ForeignKey): kwargs['cache_choices'] = True
The argument cache_choices was used in ModelChoiceField for caching and I had only one query for one foreign key in inline.
I think that there was call of function "formset_for_model" in each request (ModelAdmin.get_inline_formsets -> inline_formset -> formset_for_model). But now this method is called from ModelAdmin.__init__ (ModelAdmin.__init__ -> InlineModelAdmin.__init__ -> inline_formset -> formset_for_model).
So if I have two users with different query, I can't cache it. But it is not possible to have 100 SQL queries (if I have 100 rows). I see only two solution - use the previous variant (and call formset_for_model in each request) or somehow intelligently bind form, its fields and formset (but it seems to be complex). But I don't know django well - maybe there are other possibilities. What do you think?
> On 9/8/07, Joseph Kocherhans <jkocherh...@gmail.com> wrote:
> > If you use the newforms-admin branch and the edit inline > > functionality, you want to read this.
> > I'm planning on breaking the current syntax for defining inlines in > > the very near future, but I figured I'd warn people and ask for > > comments first.
> I've checked this in as r6072. No factory functions yet. I'll probably > hold off on those until the sprint, or start another thread.
> I think that there was call of function "formset_for_model" in each > request (ModelAdmin.get_inline_formsets -> inline_formset -> > formset_for_model). But now this method is called from > ModelAdmin.__init__ (ModelAdmin.__init__ -> InlineModelAdmin.__init__ > -> inline_formset -> formset_for_model).
Why does that cause a problem for you?
> So if I have two users with different query, I can't cache it. But it > is not possible to have 100 SQL queries (if I have 100 rows). I see > only two solution - use the previous variant (and call > formset_for_model in each request) or somehow intelligently bind form, > its fields and formset (but it seems to be complex). But I don't know > django well - maybe there are other possibilities. What do you think?
I can't interpret what you mean by this. Could you try explaining it another way?
The patch you attached in that ticket looks like it will solve your problem, and in fact, you can just subclass TabularInline or StackedInline, override the formfield_for_dbfield on you subclass, and add the property from your patch. No patch to Django needed. What am I missing?
> The patch you attached in that ticket looks like it will solve your > problem, and in fact, you can just subclass TabularInline or > StackedInline, override the formfield_for_dbfield on you subclass, and > add the property from your patch. No patch to Django needed. What am I > missing?
> Could you explain why this snippet wouldn't work?
I am sorry. You are missing nothing - I don't use python for long time and I haven't realize yet it is possible to override attribute by property (now I see that it is logical.)
But I think anyway that there should be some kind of caching by default. It isn't only my problem - it may be quite usual to use inlines with more rows and with foreign keys. (I will try to give better description and example to the ticket - I hope tomorrow).
> I am sorry. You are missing nothing - I don't use python for long time > and I haven't realize yet it is possible to override attribute by > property (now I see that it is logical.)
It's ok. I just felt I was misunderstanding your problem. :)
> But I think anyway that there should be some kind of caching by > default. It isn't only my problem - it may be quite usual to use > inlines with more rows and with foreign keys. (I will try to give > better description and example to the ticket - I hope tomorrow).
I disagree about caching by default, but it definitely should be possible to turn caching on. I don't think there is anywhere in Django where caching is on by default.
Also, take a look at [6080]. I needed to add a couple of hooks to InlineModelAdmin that would probably be useful for you.
On 10 Zář, 02:32, "Joseph Kocherhans" <jkocherh...@gmail.com> wrote:
> > But I think anyway that there should be some kind of caching by > > default. It isn't only my problem - it may be quite usual to use > > inlines with more rows and with foreign keys. (I will try to give > > better description and example to the ticket - I hope tomorrow).
> I disagree about caching by default, but it definitely should be > possible to turn caching on. I don't think there is anywhere in Django > where caching is on by default.
Probably caching between requests is not on by default anywhere - but my ticket is about caching in one request. (I don't want one query for two request.)
I try to create example models - http://code.djangoproject.com/ticket/5372#comment:2. Yes, it is possible (after [6080]) to turn caching on - but I hope that you will see that people will expect to have caching on by default in this situation.
> Also, take a look at [6080]. I needed to add a couple of hooks to > InlineModelAdmin that would probably be useful for you.
I like it very much. But I am afraid that there is a little problem. Inlines without fields and fieldsets doesn't work after this changeset - see http://code.djangoproject.com/ticket/5383.
And thank you for your great work on newforms-admin!
> for model, model_options in admin.site._registry.items(): > if i_like_model( model ): > model_options.inlines.append( CommentOptionsInline )
This does not work with the current revision of the newforms-admin branch. I think it ends up adding the inlines to the django.contrib.sites.models.SiteAdmin class, but I don't really understand the code here.
Is there a way to extend the inlines list after the class has been defined?