Thanks for a great reply Russ. I agree 100% about interpretation 3
being the most useful and beneficial.
The first concern about the original use case is something I'd
considered, but I don't think it can be helped without creating an
ugly hack (like my original patch on the ticket). The change should be
noted in the docs, but thankfully it's an incredibly easy fix for
anyone who wants to keep using that "feature".
I think we both agree about the second concern. All existing inlines
should be displayed. Since max_num and extra have no actual DB
constraints backing them up, I've always seen them as display
guidelines.
With that, I'll get to work on a proper solution over the weekend
(code changes, docs, and tests) and will have it done with time to
spare for 1.2.
Thanks so much!
- Gabriel
On Mar 18, 7:45 pm, Russell Keith-Magee <[email protected]>
wrote:
> Hi Gabriel
>
> Sorry for taking so long to get back to you on this. I certainly
> appreciate the effort you've put into the analysis.
>
> On Fri, Mar 19, 2010 at 6:03 AM, Gabriel Hurley <[email protected]> wrote:
> > There are two possible solutions, as I see it:
>
> > 1. Change the default value of max_num from 0 to -1. That's a pretty
> > standard solution to indicating an "off" value for something with a
> > range of [0, ∞]. That way when max_num and extra are *both* explicitly
> > set to 0, the "Add Another" link could be removed. For the vast
> > majority of people who *don't* explicitly set max_num to 0, it would
> > be functionally identical to the current status of things in 1.2-beta.
> > It would also bring the javascript-enabled and noscript behaviors
> > closer into line with each other. Even better it means that the
> > behavior of extra is completely untouched. This solution would also
> > mean that max_num = 0 (which should be a valid value even if it's not
> > a common usage) is no longer ignored. Obviously the docs and tests
> > would be updated in conjunction with all that.
>
> This seems like the better approach, although I would use None rather
> than -1 as a more Pythonic alternative i.e., "max_num = None" says
> "there is no max number"; max_num=0 says "there is a maximum of 0".
>
> However, I can see two downsides.
>
> Firstly, the original use case that raised this problem (i.e., someone
> that wants to show exactly 2 inlines, with no extras allowed) will
> require code changes. I'm not convinced this is a show-stopper, but it
> is something that we should mention in the release notes as an edge
> case backwards incompatibility if you were relying on one particular
> interpretation of the docs.
>
> Secondly -- and more concerning -- what happens if you set max_num = X
> (where X is any non-null value), and you already have > X inlines? For
> example, if you have 4 inline related objects, and you set max_num =
> 2, what happens to the extra 2 objects?
>
> This is actually already a problem with 1.1, but if we're going to
> start getting esoteric on the interpretation of max_num, it's doubly
> important we get this right.
>
> So, is the interpretation of max_num:
> 1) Only display max_num inlines
> 2) Always display all inlines, but only allow max_num inlines to be
> saved (and raise a validation error for the remainder)
> 3) Don't allow extra inlines to be added past the limit of max_num;
> if a form already has more than max_num inlines, allow them to remain,
> but prevent any extra inlines being added.
>
> The current implementation (1) is clearly wrong, if only because it's
> possible to display an object and *not* see all the inlines.
>
> Whether (2) or (3) is the right response depends on whether we
> consider the ModelAdmin to be a validation condition or a display
> guideline. (2) enforces ModelAdmin as a validation condition; (3)
> treats it as display guidelines. My initial response is that (3) is
> correct (since you could easily add other related objects
> programatically, or using a different form), but I'm willing to be
> convinced otherwise.
>
> The interpretation here also reflects back on the original problem. In
> the original problem, B had a FK on A, so A allowed inline B's in the
> admin. B was a complex object, so not all the fields of B were
> displayed in the inline. The inlines on A were made available as a
> convenience to edit the most notable properties of B. If you wanted to
> add a new B, you used the full admin interface for B, and set the FK
> to point to a specific instance of A. As a consequence of this, you
> can't add new B's via the inline -- but you also don't know a prioiri
> how many B's will actually be there. The constraint required here is
> "Don't allow any more B's to be added", not "don't allow more than N
> related B's". Again, (3) is the useful interpretation here, since
> ModelAdmin isn't a validation condition, it's display guidelines.
>
> Yours,
> Russ Magee %-)
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
