Opened 16 years ago
Last modified 10 months ago
#9025 new New feature
Nested Inline Support in Admin
Reported by: | pixelcort | Owned by: | |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | sree@…, cortland@…, vo.sinh@…, kilpatds@…, hr.bjarni+django@…, martin@…, tommi@…, vinilios@…, mmarcos@…, wnvnqxoq@…, james@…, lameiro@…, remco@…, minholi@…, cosford@…, brian@…, wouter@…, jacob@…, hajbete@…, michal.chruszcz@…, glicerinu@…, net147@…, ig0r, cg@…, skandocious, shawnkwall@…, gert314@…, danny.adair@…, kmike84@…, Hans Andersen, Chris Streeter, Carlos Palol, kitsunde@…, info@…, liokmkoil@…, jpedersen, Francis Devereux, simon@…, patrick, RichardOfWard, bjola, ckesselh, bullfish, klug.stefan@…, ZmjbS, jonathan.barratt@…, i.am@…, Gwildor Sok, akanouras, wgordonw1@…, Daniel Samuels, cviruss@…, jcpunk, Zach Borboa, bordage.bertrand@…, hugo@…, g7z-1sv@…, joost@…, olivier.dalang@…, awesome@…, Alejandro Dubrovsky, Tim Slot, Florian Demmer, Vadym, Dmytro Litvinov, ivanov17 | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | yes |
Description
Currently, admin.TabularInline and admin.StackedInline do not support inlines themselves.
This would allow nested inlines to happen.
Attachments (5)
Change History (202)
comment:2 by , 16 years ago
Cc: | added |
---|
comment:3 by , 16 years ago
Cc: | added |
---|
comment:4 by , 16 years ago
Component: | Contrib apps → django.contrib.admin |
---|
by , 16 years ago
Attachment: | django_nested_inlines.diff added |
---|
comment:5 by , 16 years ago
Has patch: | set |
---|---|
Needs tests: | set |
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Design decision needed |
I attached a patch to enable nested inlines with a depth level of 3 on the change_view of ModelAdmin
depth level of 3 means it is limited to something like:
A.inlines[ B ]
B.inlines [ C ]
C.inlines [ D ]
It's just an experiment to see if it was easily doable so the code is not that great
comment:6 by , 16 years ago
Cc: | added |
---|
comment:7 by , 16 years ago
Cc: | removed |
---|
follow-up: 9 comment:8 by , 16 years ago
Replying to pixelcort:
Currently, admin.TabularInline and admin.StackedInline do not support inlines themselves.
This would allow nested inlines to happen.
Has the attached patch been tested? Anything special in the admin.py configuration? It is not working for me, although the code with the patch was translated successfully.
Thanks for a reply.
follow-up: 10 comment:9 by , 16 years ago
Replying to anonymous:
Replying to pixelcort:
Currently, admin.TabularInline and admin.StackedInline do not support inlines themselves.
This would allow nested inlines to happen.
Has the attached patch been tested? Anything special in the admin.py configuration? It is not working for me, although the code with the patch was translated successfully.
Thanks for a reply.
I wrote it for a project with a very large number of tables (>100) and with 3 or 4 levels of relations depth between tables very often
and doing a specific admin page for every table set would be big work (20+ sets, and more could be added).
the code gives you the possibility to do multiple inlines like:
class rab_Inline(TabularInline):
model = rab
class bar_Inline(TabularInline):
model = bar
inlines = [rab_Inline]
class foo_Admin(AdminModel):
model = foo
inlines = [bar_Inline]
with code like this in your admin.py you have the three tables in the foo object admin page
is this working for you? I tested it with 1.0 and 1.0.1. I'm going to upgrade to 1.0.3 soon and test with it too, but it should work
the part that's missing from the patch is EDITING support. You are not able to edit. I took a look at it for a bit but since I dont need it i never implemented it.
I remember there's code that puts all the generated html form code in a list, but if you have multiple inlines the order is wrong and only the first added forms can work (the foo object form should work). Expanding that code should add editing support. If you are interest I can provide more details
comment:10 by , 16 years ago
Replying to vosinh:
Replying to anonymous:
Replying to pixelcort:
Currently, admin.TabularInline and admin.StackedInline do not support inlines themselves.
This would allow nested inlines to happen.
Has the attached patch been tested? Anything special in the admin.py configuration? It is not working for me, although the code with the patch was translated successfully.
Thanks for a reply.
I wrote it for a project with a very large number of tables (>100) and with 3 or 4 levels of relations depth between tables very often
and doing a specific admin page for every table set would be big work (20+ sets, and more could be added).
the code gives you the possibility to do multiple inlines like:
class rab_Inline(TabularInline):
model = rab
class bar_Inline(TabularInline):
model = bar
inlines = [rab_Inline]
class foo_Admin(AdminModel):
model = foo
inlines = [bar_Inline]
with code like this in your admin.py you have the three tables in the foo object admin page
is this working for you? I tested it with 1.0 and 1.0.1. I'm going to upgrade to 1.0.3 soon and test with it too, but it should work
the part that's missing from the patch is EDITING support. You are not able to edit. I took a look at it for a bit but since I dont need it i never implemented it.
I remember there's code that puts all the generated html form code in a list, but if you have multiple inlines the order is wrong and only the first added forms can work (the foo object form should work). Expanding that code should add editing support. If you are interest I can provide more details
tested with 1.0 and 1.0.1 but not 1.0.2
1.0.3 is typo
comment:12 by , 16 years ago
Keywords: | admin-refactor added |
---|---|
Version: | 1.0 → SVN |
To give more context why this is useful, let me illustrate this with a common use case.
Suppose you model events that can have one or more occurrences.
Each occurrence has a single location and one or more time slots (e.g. a
theatre performance that takes place in hall A on Tuesday and Wednesday and
in hall B on Saturday.)
You get the following model structure:
class Event(models.Model): name = models.CharField(max_length=255) def __unicode__(self): return self.name class Location(models.Model): name = models.CharField(max_length=255) def __unicode__(self): return self.name class Occurrence(models.Model): event = models.ForeignKey(Event) location = models.ForeignKey(Location) class TimeSlot(models.Model): occurrence = models.ForeignKey(Occurrence) start = models.DateTimeField() end = models.DateTimeField()
For the admin, you need occurrences to be displayed when you edit
the events. The occurrences themselves need to display times lots.
Thus, inline nesting is needed:
class TimeSlotInline(admin.TabularInline): model = TimeSlot extra = 1 class OccurrenceInline(admin.TabularInline): model = Occurrence inlines = (TimeSlotInline,) extra = 2 class EventAdmin(admin.ModelAdmin): inlines = (OccurrenceInline,) admin.site.register(Event, EventAdmin) admin.site.register(Location)
A design decision is also needed on how deep the nesting should be. It's
possible to support arbitrarily deep nesting with recursion, whether that's
feasible or necessary remains open. Perhaps an extension mechanism should be
provided so that API users can control the inlines and nesting.
The result could look like this:
comment:13 by , 16 years ago
Keywords: | gsoc09-admin-refactor added; admin-refactor removed |
---|
comment:14 by , 16 years ago
Keywords: | gsoc09-admin-refactor removed |
---|
comment:15 by , 15 years ago
Cc: | added |
---|
comment:16 by , 15 years ago
Cc: | added |
---|
comment:17 by , 15 years ago
Cc: | added |
---|
comment:18 by , 15 years ago
Cc: | added |
---|
comment:19 by , 15 years ago
Cc: | added; removed |
---|
comment:20 by , 15 years ago
Cc: | added |
---|
comment:21 by , 15 years ago
I've managed to partially made this patch work for version 1.2 pre-alpha (svn release).
When I go to the adding site the inlines of an inline are being displayed but unfortunately whenever I want to add or save an edit something is crashing.
Is there a working (valid) patch for later versions of django (eg. 1.1)?
Thanks and appreciate the work that you guys contribute.
If you want to me to show the changes I had to make to get it work please ask. It wont be a problem.
best regards.
mp
comment:22 by , 15 years ago
Cc: | added |
---|
comment:23 by , 15 years ago
Cc: | added |
---|
comment:24 by , 15 years ago
I've been putting a bit of effort into this today, and I have come to the conclusion that the method for doing this in the patch is flawed.
It attempts to associate the nested inlines with the base admin model. I think that there probably shouldn't be many changes to the ModelAdmin class, but that they should all be in InlineModelAdmin.
comment:25 by , 15 years ago
Cc: | added |
---|
comment:26 by , 15 years ago
I have got some code working, and attached a patch if anyone else is interested.
Basically, I have slightly modified the ModelAdmin class to include the nested inlines in the change page (I haven't worried about the add page at this stage).
Then, the InlineModelAdmin has changes to create the inline formsets. Finally, there are a couple of template changes, including a new template for the nested formsets.
I have made a couple of design decisions that reflect my use case for the project I am working on, mainly that I don't have add page support, and only include the nested inlines in a TabularInline. Each class of inline is a seperate column, and each instance of the related object is a sub-row in that column.
I have an example image:
http://s3.amazonaws.com/ember/HGETbz2gS1IXvDlOB3idRmcp0ocFL1VK_l.png
(Note that this uses the Inline properties on this class to not show more than 2, and none if the object is not saved).
comment:27 by , 15 years ago
Cc: | added |
---|
comment:28 by , 15 years ago
Cc: | added |
---|
comment:29 by , 15 years ago
Cc: | added |
---|
comment:30 by , 15 years ago
Cc: | added |
---|
comment:31 by , 15 years ago
Cc: | added |
---|
comment:32 by , 15 years ago
Cc: | added |
---|
comment:34 by , 15 years ago
Cc: | added |
---|
comment:35 by , 15 years ago
Cc: | added |
---|
comment:36 by , 15 years ago
Cc: | added |
---|
comment:37 by , 14 years ago
Cc: | added |
---|
comment:38 by , 14 years ago
Cc: | added |
---|
comment:39 by , 14 years ago
Cc: | added |
---|
comment:40 by , 14 years ago
Cc: | added |
---|
comment:41 by , 14 years ago
Cc: | added |
---|
comment:43 by , 14 years ago
#13163 provides a simpler workaround for this -- each inline provides a link to the inline object provided it is also registered in that admin site, which allows you to edit its own inlines, etc. That way you can go as deep as you like. Not quite as easy as having it all rendered on the one page, but simple, robust and makes nested/recursive inlines navigable.
comment:44 by , 14 years ago
Cc: | added |
---|
comment:45 by , 14 years ago
Cc: | added |
---|
comment:46 by , 14 years ago
Cc: | added |
---|
comment:47 by , 14 years ago
Cc: | added |
---|
comment:48 by , 14 years ago
Cc: | added |
---|
comment:49 by , 14 years ago
Cc: | added |
---|
comment:50 by , 14 years ago
Can anyone confirm that this works with 1.3 beta 1? It's not working for me.
Thanx!
comment:51 by , 14 years ago
@RoosterJuice: any reason to assume that it does?
There's no notice of any patches having been checked in nor is this ticket marked as a 1.3 milestone. But if you want this code in Django, do not let anything stop you and start nagging the core dev's as soon as 1.3 is out, and at the same time update this patch and add some unittests. We'd be amazingly thankful! ;)
comment:52 by , 14 years ago
Cc: | added |
---|
comment:53 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:54 by , 14 years ago
Cc: | added |
---|---|
Easy pickings: | unset |
comment:55 by , 13 years ago
UI/UX: | set |
---|
comment:57 by , 13 years ago
This feature is a great improvement for the Django admin! When can we use it in a stable Django release?
comment:59 by , 13 years ago
Cc: | removed |
---|
comment:61 by , 13 years ago
I just assumed I could do this! Then I tried and obviously it was a fail. DEFINITELY a required feature. +1!
comment:64 by , 13 years ago
What do I have to do to stop receiving e-mails about updates to this issue?
comment:65 by , 13 years ago
Cc: | added; removed |
---|
@ RaceCondition: removing yourself from the cc list seems to be the only way. Bugzilla at least lets you control what notifications you get, but I don't see that feature in this trace setup.
FWIW, from what I've heard this feature will not be implemented unless someone submits a very serious patch, and I imagine you'd hear about that through other channels if it were to happen.
comment:66 by , 13 years ago
@tomvons: I'm not in the CC list; I only get the option to add my e-mail to the CC, otherwise I wouldn't be asking, would I...
comment:67 by , 13 years ago
Cc: | removed |
---|
comment:68 by , 13 years ago
Cc: | removed |
---|
comment:69 by , 13 years ago
+1 but obviously not as easy as it sounds. Really improves usability though.
follow-up: 72 comment:70 by , 13 years ago
Why do I keep receiving e-mails about this ticket? I've removed myself from CC. Please fix this issue, whoever is responsible for it.
comment:71 by , 13 years ago
RaceCondition: sorry, it's a "feature" of Trac. Let me know which email you're getting emails to and I'll take care of it. You can post a comment here or email me at jacob at jacobian dot org
.
comment:72 by , 13 years ago
Replying to RaceCondition:
Why do I keep receiving e-mails about this ticket? I've removed myself from CC. Please fix this issue, whoever is responsible for it.
See #16763.
comment:73 by , 13 years ago
Cc: | added |
---|
comment:74 by , 13 years ago
it seems that "Remove from CC" doesn't work at all anymore. Is there a solution to this? Effectively, code.djangoproject.com is sending out unwanted emails.
comment:77 by , 13 years ago
Cc: | added |
---|
comment:83 by , 13 years ago
Cc: | added |
---|
comment:84 by , 13 years ago
Cc: | added |
---|
comment:86 by , 13 years ago
If you're interested in this, don't add "+1", write the code.
When you write "+1", you're broadcasting "I'm too lazy to write this code, do it for me" to all the people that could be interested in writing this feature. That's quite a motivation killer. Just a heads up.
comment:87 by , 13 years ago
Cc: | removed |
---|
comment:89 by , 13 years ago
Dear anonymous, Django is open source remember. People don't owe you, even if it has been ten years since something was reported in Trac. Do you want Nested inline support? Step up to help out, pay a developer to build it, be involved in some way to do something constructive....
comment:90 by , 13 years ago
Cc: | removed |
---|
comment:91 by , 13 years ago
I was looking for a solution to this as well
I've created a repository underwhich I started with a clone of 1.3.1
I initially applied the above patch and then modified it to extend the functionality
as a result there are 2 new classes admin.NestedTabularInline and admin.NestedStackedInline
I fixed the ability to use the add more button on the initial inline and will be doing some work to fix the add more button for the nested inlines
I've not fully tested the stacked inlines but if someone wants to give them a whirl I think they are good to go
comment:92 by , 13 years ago
Cc: | added |
---|
comment:93 by , 13 years ago
If someone is looking for a work around until this is committed, a simple method for adding links between inlines and top-level admin pages, and top level admin pages to related models, was posted Stack Overflow this week: http://stackoverflow.com/a/10011307/500584
comment:94 by , 13 years ago
I have losting the form validation, someone can help-me?
when i submit a invalid email
i have:
Caught KeyError while rendering: "Key 'processo' not found in Form"
but if a submit the same field by other user which not see the inline nested, the form show the validation message.
comment:95 by , 13 years ago
Cc: | added |
---|
comment:96 by , 13 years ago
Cc: | added |
---|
comment:97 by , 13 years ago
Cc: | added |
---|
comment:98 by , 12 years ago
Cc: | added |
---|
comment:99 by , 12 years ago
Triage Stage: | Design decision needed → Accepted |
---|
Given the huge number of people interested in this feature, I'm brazenly going to accept it.
It still needs a good patch with tests and docs. The tests should use the Selenium support introduced in Django 1.4. Whoever writes such a patch gets to make the remaining design decisions (such as max nesting depth, etc.)
comment:100 by , 12 years ago
Cc: | added |
---|
comment:101 by , 12 years ago
Cc: | added |
---|
comment:102 by , 12 years ago
Cc: | added |
---|
comment:103 by , 12 years ago
Cc: | added |
---|
comment:108 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:109 by , 12 years ago
I have started working on this a few days ago. I have a sort off working code that does whats needed, but lacks user friendliness. Currently writing tests and fixing jquery stuff
comment:110 by , 12 years ago
This is such great news! I am currently using a modified version of the solution proposed in #comment:91, but the inline formsets and admin forms code turned out to be quite complicated, and I can't wait to see a proper solution.
Please post an update when you have a beta to test - I'd be eager to jump on it.
comment:111 by , 12 years ago
I just finished fixing the jquery code, which took a bit longer than expected. I'm now testing if the saving still works as expected, and then I'll upload a diff so people can maybe try it while i'm writing the rest of the tests and the docs.
comment:112 by , 12 years ago
Cc: | added |
---|
I applied the above patch (nested_inlines_2.diff) to the latest master branch today (v1.5alpha latest commit #bfcda7781a) and it appears to work very well.
myApp.admin.py -------- from django.contrib import admin from myApp.models import Project, Parcel, Plot, Tree class TreeInline(admin.TabularInline): model = Tree fields = ('genus','species','dbh') extra = 0 class PlotInline(admin.StackedInline): model = Plot fields = ('name','shape_reported','area_reported') inlines = [TreeInline, ] extra = 0 class ParcelInline(admin.StackedInline): model = Parcel fields = ('name','area_reported') inlines = [PlotInline, ] extra = 0 class ProjectAdmin(admin.ModelAdmin): fields = ('name','owner') inlines = [ParcelInline, ]
The resulting screenshot is here: http://i46.tinypic.com/2qjzvkn.png
Implementation is as simple as adding an inlines
definition to the (inherited) TabularInline or StackedInline class (which is already supported in the admin.ModelAdmin class). This is exactly what I had originally tried to do before learning it was not supported, so I would say its a very slick enhancement. Thank you Gargamel.
I will report back any new issues if I encounter them. (Changes did save correctly for me.)
Next steps: docs and tests (?)
comment:113 by , 12 years ago
Tested also, works like a charm, nice work Gargamel, given the '+1' comments here I suspect you'll have made a bunch of people happy.
from django.contrib import admin from .models import Band, Venue, Event, EventInstance, Booking class BookingAdminInline(admin.TabularInline): model = Booking class EventInstanceAdminInline(admin.TabularInline): model = EventInstance inlines = (BookingAdminInline,) class EventAdmin(admin.ModelAdmin): inlines = (EventInstanceAdminInline,) admin.site.register(Band, admin.ModelAdmin) admin.site.register(Venue, admin.ModelAdmin) admin.site.register(Event, EventAdmin)
One slight issue is that it becomes a bit difficult to tell what field you are looking at when using a TabularInline - see the red arrows here: http://oi48.tinypic.com/4gm5w2.jpg. The stacked version looks fine and the fields are clearly labelled: http://i48.tinypic.com/rwnejt.png. Not sure what the fix would be (other than simply not using TabularInline for anything other than the innermost model), or even if it needs fixing. Perhaps an extra header row after each nested model? Maybe only bother to support StackedInline nesting? Probably a different bug for a different day.
Otherwise, great stuff, intuitive to use - like pztrick I previously tried adding an 'inlines' definition expecting it to work.
PS, I couldn't apply your patch (tried 'git am nested_inlines_2.diff', 'patch -p1 < nested_inlines_2.diff') without first poking it with a big regex. What was I doing wrong?
comment:114 by , 12 years ago
Cc: | added |
---|
comment:115 by , 12 years ago
@Gargamel: Did you ever know that you're my hero? (https://www.youtube.com/watch?v=Hu4gCIgM_SI)
comment:116 by , 12 years ago
@Gargamel: Many thanks for this patch, it's exactly what I needed. Unfortunately I think I've come across 2 bugs though...
(referring to the example in pztrick's post with Projects, Parcels, Plots and Trees)
1) In the admin page when you have a Project with no Parcels, click on "Add another Parcel" and you'll get a new Parcel to edit but it won't show any of the "Add another Plot" options for the nested inlines.
2) If you have a Project with several Parcels and you click on "Add another Parcel" you seem to get one Plot already made for each existing Parcel that you have. If I make no more changes but click "Save" now, I get the message "Parent object must be created when creating nested inlines."
It is of course possible that my models are set up incorrectly but I think they're ok - basically each nested inline has a foreign key to its immediate parent. Using the tree example again, Tree has FK to Plot, Plot has FK to Parcel, Parcel has FK to Project.
comment:117 by , 12 years ago
Hi guys
First of all, thanks for the feedback! I have been spending my time this past week on the finishing touches. I fixed some more jquery bugs, wrote all the test and wrote docs. I haven't read your feedback yet though, so maybe some of the bugs are still in the code I uploaded. I will check this this weekend and keep you up to date.
comment:118 by , 12 years ago
@RichardOfWard: Yes, I have noticed the problem with tabular inlines also. I thought about adding the field headers again after each nested inline, but this turned out to be very difficult. I would say if it bother people, just use stacked inlines :D. I'm not sure why your patch didn't work. Did you have any error messages or something? I'm not that proficient at patches/diffs/git, so maybe I did something wrong...
@hejsan: Thanks :D
@rugbyjock: I will look into these bugs. I just realize that I have done very little testing with changing existing models, so thanks for pointing this out!
comment:119 by , 12 years ago
Just found this.. so glad someone took up the challenge...
Maybe I'm just a bit slow this morning, but I've never seen a patch in that format, and I can't seem to figure out how to apply it.
Exactly what command line are you using the generate / apply this patch?
comment:120 by , 12 years ago
@FunkyBob: It shames me to tell you that I used some sort of git-gui program to generate it. I have just reinstalled my linux system, so now I can get on with learning the git syntax. Once I have fixed the last bugs I'll post a command line git diff :)
@rugbyjock: I can't reproduce the first bug you reported. Could you maybe check it again with the new code, and if it is still there give me some more details of your setup? I did reproduce the second one and am in the process of fixing it.
edit: ok, so git diff was way easier than I expected, easier than that stupid gui program at least :D, I'll update the diff file here, but no reported bugs have been fixed yet.
follow-up: 122 comment:121 by , 12 years ago
I fixed the second bug reported by rugbyjock. As of this patch, every reported bug should have been fixed. I'll monitor this page until friday, and if no new bug reports have come up, I'll make a pull request to the master branch. Happy testing, I know I will :D
comment:122 by , 12 years ago
Replying to Gargamel:
I fixed the second bug reported by rugbyjock. As of this patch, every reported bug should have been fixed. I'll monitor this page until friday, and if no new bug reports have come up, I'll make a pull request to the master branch. Happy testing, I know I will :D
Hey,
thanks for a work on this!
I didn't try the patch (only read it) and it seems there are some small issues:
https://code.djangoproject.com/attachment/ticket/9025/nested_inlines_finished.diff#L98 - this is incorrect in Python3 (remove "u");
https://code.djangoproject.com/attachment/ticket/9025/nested_inlines_finished.diff#L519 - it seems 'var' is missed and this variable is defined as global;
https://code.djangoproject.com/attachment/ticket/9025/nested_inlines_finished.diff#L525 (and further) - the same applies to border_class variable.
comment:123 by , 12 years ago
@Gargamel - I've applied the latest patch but still see the bugs I mentioned. I've attached a small project based on the trees and plots discussed earlier which demonstrates the problem.
Problem 1: If you start it up and add a new project, you'll see a link for "Add a New Parcel" but there is no way to add in the nested inlines. If I adjust my admin script to "extra = 1" for all inlines, then this seems to work.
Problem 2: If you set extra=1 and then added a project with a single parcel, single plot and single tree, that looks like it worked. Click "Save and Continue Editing" (any route into the editing repros the problem fine) and then try to add a third (you already made one plus there is the "extra=1" plot) plot to the first parcel. Notice how this plot now contains two nested tree inlines which is wrong and also breaks if you try to add data and save. The same problem occurs if you try to add a third parcel.
Let me know if this doesn't make sense! Thanks again for your work on this.
comment:124 by , 12 years ago
I forgot to mention - in problem 2 as described above, it only seems to go wrong with the inlines have got extra=1 (or greater, I guess). With extra=0 it works.
comment:125 by , 12 years ago
@rugbyjock: I was able to reproduce the first bug now, I forgot to set extra=0. The problem is, I don't really see how this is fixable... The root of the problem is that when extra is 0, the inline templates only insert the 'empty-form', but these forms do not contain the nested forms, because this would break all previous javascript code for inlines. I think to fix this bug I would have to rewrite nearly all of my code and the inline code in general... For this reason I will probably just disable the option of putting 0 in the extra field when nested inlines are used. Maybe one day I'll feel like rewriting the entire thing, but not now :).
The second bug was indeed fixed for the most, but I just uploaded the wrong diff >.<
I'll do some more fixing and then I'll upload the file again.
comment:126 by , 12 years ago
@Gargamel - fair enough, I suspected it would be something like that. I'll try working with a mixture of extra=1 at the higher level nesting and extra=0 at the lower - perhaps this way I can sidestep the issue.
Looking forward to the new diff, I'll give it a test as soon as it's up.
Thanks again!
comment:127 by , 12 years ago
Here's a list of problems with current code. (Some of these repeat the bugs reported above, some are new).
- If all extra attributes are set to 0, as mentioned above, saving an inline results in "ManagementForm data is missing or has been tampered with"
- If the first-level inline (ParcelInline) has extra=1, and the others have extra=0, I can add inlines and nested inlines, but saving or deleting existing set of inlines without filling the extra inline fields fails with this message: "Parent object must be created when creating nested inlines" (for first-level inline modifications) or "ManagementForm data is missing or has been tampered with" (for deeper levels).
- For all extra=1 (or larger number), every time I try to save, all extra fields have to be filled, otherwise the same errors appear. Deleting inlines is impossible.
- With first-level inline's extra=1, and the rest extra=0, it is impossible to save a record without at least one inline (this is a special case of 2)
comment:128 by , 12 years ago
Commenting out three lines after "Here be dragons :(" (around line 1050 in options.py) makes the "Parent object must be created when creating nested inlines" go away. This also makes it possible to delete inlines.
comment:129 by , 12 years ago
Another bug: deleting an inline that contains nested inlines does not work unless I first delete all the children.
comment:130 by , 12 years ago
Another bug: if you delete all inlines, you can't add any inlines after that - you get a "ManagementForm data is missing or has been tampered with" error.
comment:131 by , 12 years ago
Damn, I just typed a massive post about my plans with this feature, but then my firefox restarted for some reason and I lost it all :(. I'll just do a tl;dr because I don't really feel like typing it all again. Maybe later.
In short, I'm going to start from scratch with a design based on nested inlines from the ground up. This means I will need to rewrite most of the inline jquery code, so I don't expect this to be done in a week or so. I'll try to post weekly updates of my progress.
Thanks to everyone for the bug reports, though I will not fix them, they are usefull for test ideas in the future!
comment:132 by , 12 years ago
Cc: | removed |
---|
comment:133 by , 12 years ago
Cc: | added |
---|
comment:134 by , 12 years ago
Found another bug/TODO item: helpers.AdminErrorList needs to be updated, so that it iterates over nested formsets. Otherwise, a form validation error in a nested formset does not result in a general message above the admin form. This can be a problem if the form is long or uses custom styles that hide some of the fields, such as tabbed interface.
comment:135 by , 12 years ago
@bjola: Thanks, I did not think of that!
Just a short update, I haven't made much progress yet. Most of my free time is crawling into this now, but I am actually incredibly stuck on finding a proper design to start with. The problem is just that when an inline has extra=0, it's formset does not contain any forms, so the nesting stops there. I somehow need to decouple the recursive inline-nesting function from the formsets, but this proves to be quite complicated...
comment:136 by , 12 years ago
Not sure if this is the best way, but it's a way - I hacked my admin templates to include a sample form for every inline, including nested inlines. The sample form input ID's all have a '-sample' suffix. If the JS code that handles "add" link can find an existing formset, everything goes normally. Otherwise, it takes the sample one, clones it, strips away the '-sample', etc.
I only needed this for a specific project, so I just hard-coded the sample form into admin template for that particular model. But to make it generic, I'm guessing one would need to supply a "sample formset" along with the real one from the server side.
comment:137 by , 12 years ago
Another bug: in contrib/admin/static/js/inlines.js, the lines:
// Update Form Properties template.find('#id_' + formset_prefix + '-TOTAL_FORMS').val(1); update_props(template, normalized_formset_prefix, formset_prefix);
should be
// Update Form Properties update_props(template, normalized_formset_prefix, formset_prefix); template.find('#id_' + formset_prefix + '-TOTAL_FORMS').val(1); template.find('#id_' + formset_prefix + '-INITIAL_FORMS').val(0);
Because the TOTAL_FORMS and INITIAL_FORMS inputs do not have the right IDs until update_props is run. And INITIAL_FORMS needs to be 0, otherwise the server-side validation tries to save non-existing nested forms.
comment:138 by , 12 years ago
@bjola, thanks again, bugs are always helpfull to learn :). About your 'hack', I have thought about this too, but it makes the html so incredibly cluttered and very hard to handle, not to mention that jquery performance would nosedive when models get more complicated with such a massive DOM. So thats why I am now, as you said, trying to make the server side generate 1 sample formset for every inline used.
Well, I'm glad to announce I have made some progress. It's not the most pythonic code I've ever written, but its ok and it seems to work perfectly. I still need to implement the whole error checking, templates and jquery shebang, so it won't be done for a while, but suggestions are always welcome. If anyone would like a diff to check my approach and maybe offer some suggestions, just ask :)
comment:139 by , 12 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
Well, I'm going to unassign this, because I don't feel I'm progressing fast enough... I'm still gonna work on it in my spare time, but with a different approach. I just can't wrap my head around the patterns used for the InlineFormSet classes and such, and this is definitely holding me down.
On the other hand, maybe my current approach is just completely wrong... I'm just gonna take my time to really understand the codebase behind the whole inlines and then try again until I succeed.
Well, if anybody really wants nested inlines, the supplied patch works ok I guess, except for some special cases so that will have to do for now.
comment:140 by , 12 years ago
Cc: | added |
---|
comment:141 by , 12 years ago
Cc: | added |
---|
comment:142 by , 12 years ago
Needs tests: | unset |
---|
Th current version of the patch has documentation and tests. Good!
comment:143 by , 12 years ago
Cc: | added |
---|
Hi, could anyone with better knowledge summarize the status of this feature? I'd like to give it a try and potentially add some fixes. To my knowledge the status is as follows:
- There is a patch (nested_inlines_finished.diff) by Gargamel which has docs and tests but still contains some bugs described in #comment:127, #comment:134 bjola did some work on this. @bjola: Is there any public version of this work available?
- The bugs seemed so severe that Gargamel restarted from scratch, did some progress but hasn't got enough spare time to push it further. I'd love to see the status of this work as it seems a waste of time to debug the old patch when Gargamel decided to abandon it.
@Gragamel: Could you post a patch or link a git repository where the status of your new work can be seen? - Or did you dump the whole thing (#comment:140 seems like that)
It would be a lot easier to help, if there was a central place with the current state. Maybe one of the core devs could put up a git branch for this feature?
comment:144 by , 12 years ago
After several hours of stepping through the django code, I came up with a new version. It lives in the nested-inline-support-1.5.x of my django fork: https://github.com/stefanklug/django/tree/nested-inline-support-1.5.x
It is based on the current django 1.5.x branch. I applied the proposed patch (nested_inlines_finished.diff) and did some further improvements:
All bugs mentioned in #comment:127 should be fixed. I had to add some things to django.forms. These have to be discussed with a core dev.
The problem mentioned in #comment:135 is not yet fixed. This needs some more thinking and some bigger changes, but shouldn't be a show stopper. The BugRepo attached to this task works for me out of the box now.
comment:145 by , 12 years ago
Cc: | added |
---|
comment:146 by , 12 years ago
Cc: | added |
---|
comment:147 by , 12 years ago
Cc: | added |
---|
comment:148 by , 12 years ago
Hey everyone, I just aped Stefan Klug's code and produced an easy to install package that provides nested inline support in Django >=1.4. I welcome everyone to issue bug reports and pull requests on the repo: https://github.com/Soaa-/django-nested-inlines
Please test installation and functionality as this is my first ever package.
comment:149 by , 12 years ago
Cc: | added |
---|
comment:150 by , 12 years ago
Hello, Dear guys, I'm a new comer on django and encounter this inline requirement. But still don't know how to install this patch, even after i read all this comments. Can someone help me on it?
The following is the result(Django Version = 1.4.3), i also remove the --dry--run and seems the same result. Did it means the patch works?
patch --dry-run -p1 < nested_inlines_finished.diff
Hunk #1 succeeded at 1493 (offset 124 lines).
Hunk #2 succeeded at 1520 (offset 121 lines).
patching file tests/regressiontests/admin_inlines/admin.py
Reversed (or previously applied) patch detected! Assume -R? [n] y
Hunk #1 succeeded at 123 with fuzz 2.
Hunk #2 FAILED at 141.
1 out of 2 hunks FAILED -- saving rejects to file tests/regressiontests/admin_inlines/admin.py.rej
patching file tests/regressiontests/admin_inlines/models.py
Reversed (or previously applied) patch detected! Assume -R? [n] y
Hunk #1 succeeded at 191 (offset 11 lines).
patching file tests/regressiontests/admin_inlines/tests.py
Hunk #1 FAILED at 8.
Hunk #2 FAILED at 560.
Hunk #3 FAILED at 587.
3 out of 3 hunks FAILED -- saving rejects to file tests/regressiontests/admin_inlines/tests.py.rej
comment:151 by , 12 years ago
Jowett, I recommend using Alain Trinh's package from #comment:148 - https://github.com/Soaa-/django-nested-inlines (works for me)
Installation is as easy as: pip install -e git+git://github.com/Soaa-/django-nested-inlines.git#egg=django-nested-inlines
There are more instructions in the github README for actually using the module in your code.
comment:152 by , 12 years ago
If we can easily do this using an externally maintained module and we want to limit the code maintenance pressure on core devs, shouldn't we just mark this issue as a 'wontfix'?
@aaugustin: What's your take on this, having accepted the ticket some several months ago?
comment:153 by , 12 years ago
It's fine to keep it as "accepted".
After 150 comments and dozen of patches, I don't think uploading one more patch here will be noticed. If someone really wants this to happen, take it to django-developers with a solid proposal.
By "solid proposal" I mean something like https://groups.google.com/d/msg/django-developers/zwQju7hbG78/EV0tky1oMUYJ.
comment:154 by , 12 years ago
Cc: | removed |
---|
comment:155 by , 12 years ago
Just to sum up the current status, for people stumbling over this thread.
Please don't use the patches attached to this ticket. They are outdated.
My branch https://github.com/stefanklug/django/tree/nested-inline-support-1.5.x is the place where I do/did the bugfixing. It's a complete django branch and anyone can install it via pip install -e git+git@github.com:stefanklug/django.git@nested-inline-support-1.5.x#egg=django
, I will merge in the current django 1.5 release, but I don't know how much more time I'll spend on this.
Alain Trinh wrapped this in a django app. The installation is independent of django and easily done with pip install -e git+git://github.com/Soaa-/django-nested-inlines.git#egg=django-nested-inlines
. It adds some additional mixin classes and complexity, which makes contributing and debugging a bit harder.
So if you want the easy install solution go with Alain's, if you want this to land in django core at some time, fork my branch and continue with that.
Now to the code:
The basic design is the main problem. The current code is useable, but should never be applied to the core.
The current work flow is as follows:
- Every nested inline is a formset inserted after the last row of fields for a given model. This is done recursively, so after every model inside the formset new nested inlines are created. This is the main design problem.
- On the client side, as soon as a new item is added, normally the "empty" element of the formset is taken, cloned and inserted as new row. In the nested case, the empty item doesn't contain nested formsets (see 1.) and is therefore not enough to reconstruct a complete new Form including nested forms. So the current, horrible solution is: A clone of the first model in the formset is created (fingers crossed that it exists), any ids and unnecesary information is removed, the row is inserted (never change the templates or everything breaks apart) and nested inlines are recreated.
So nested inlines are constructed "beside" the ModelForm and not "inside" as they should be. The approach taken in https://github.com/yumike/django-formsetfield would be better, but requires rewriting a lot of the current nested-inlines code and mucking around with some hard-to-grasp form wrappers inside the admin app.
It should also be pushed down to the level of ModelForm and not only live inside the admin app...
follow-up: 163 comment:161 by , 11 years ago
Also, when I click on 'add .....' having put 'extra = 1' Messing up since I do not charge the nested class. When it is loaded, when I click on 'add ...', adds one to which ones previous. Instead if I put 'extra = 5', all 5 are working properly
follow-up: 164 comment:163 by , 11 years ago
Replying to anonymous:
Also, when I click on 'add .....' having put 'extra = 1' Messing up since I do not charge the nested class. When it is loaded, when I click on 'add ...', adds one to which ones previous. Instead if I put 'extra = 5', all 5 are working properly
take a look at my own repo on github (waiting pull-request by Soaa of course) which fixes that severe bug: https://github.com/silverfix/django-nested-inlines
comment:164 by , 11 years ago
Replying to silverfix:
Replying to anonymous:
Also, when I click on 'add .....' having put 'extra = 1' Messing up since I do not charge the nested class. When it is loaded, when I click on 'add ...', adds one to which ones previous. Instead if I put 'extra = 5', all 5 are working properly
take a look at my own repo on github (waiting pull-request by Soaa of course) which fixes that severe bug: https://github.com/silverfix/django-nested-inlines
I have used this one that you gave, but it still exists. There are no nested inlines for added inlines. There are only first level inline for added tag. (The inlines that are rendered initially are seen as nested. The problem is only about added inlines.)
comment:166 by , 11 years ago
Cc: | removed |
---|
comment:167 by , 11 years ago
Cc: | added |
---|
comment:168 by , 11 years ago
What is current status of this feature?
Recent changes in django 1.7 will make it somehow easier to do?
comment:169 by , 11 years ago
Recently, there were more bugs fixed in silvexfix branch https://github.com/silverfix/django-nested-inlines
comment:170 by , 11 years ago
Cc: | added |
---|
comment:171 by , 11 years ago
With the fix on version 1.6 of Django solving the MultiValueDictKeyError, the back-compatibility with Django 1.5 was lost.
We restored it with https://github.com/silverfix/django-nested-inlines/pull/8
comment:172 by , 11 years ago
Cc: | added |
---|
comment:174 by , 11 years ago
Using the silvexfix branch https://github.com/silverfix/django-nested-inlines I found a problem if I enable the 'save_as' feature in the ModelAdmin and save as new a previous stored record. I'm forced to use django 1.5.
Could someone confirm it ? If yes: some suggestion about how to fix the problem ?
comment:175 by , 11 years ago
Cc: | added |
---|
comment:177 by , 11 years ago
Cc: | removed |
---|
comment:178 by , 11 years ago
Cc: | removed |
---|
comment:180 by , 10 years ago
Cc: | added |
---|
comment:181 by , 10 years ago
up o/
currently using silverfix fork, pretty cool but has some limitations/bugs
comment:182 by , 10 years ago
Cc: | added |
---|
comment:183 by , 10 years ago
I'll confess some unfamiliarity with the django code base, but wouldn't it make more sense to extend InlineModelAdmin than to make another type of inline class definition?
Whatever receives the results from ModelAdmin.get_formsets_with_inlines() would have to be made aware of the change though.....
comment:184 by , 10 years ago
Cc: | added |
---|
comment:185 by , 10 years ago
Can someone make a kickstarter or something like this to solve this issue?
It’s really blocking. I’m ready to pay a lot to fix it.
Otherwise, that will mean I’ll have to spend at least a week on it. I think I can do it, but it would be faster if someone who already has a good knowledge of formset internals did it.
comment:186 by , 10 years ago
Cc: | added |
---|
comment:187 by , 10 years ago
Great news everyone!
I finally managed, after a few weeks of work, to make nested inlines work! It works with Django and also Grappelli.
It’s called django-super-inlines.
It should work great on all cases, except recursive inlines (when an inline mentions itself as a possible inline). It’s quite complex to handle. We may have it one day, but I’m not even sure that’s a good idea. Suppose you have a whole tree of thousands of nodes in a single inline row. Apart from being extremely heavy to load, if a user removes the root inline, it will delete the whole tree! And I’m not sure users will be easily aware of that.
It’s a full rewrite of the Javascript part using object-oriented programming. And I chose to make a minimal modification to the Python part, as it is extremely sensible and we can’t just change such a major part as formsets without a lot of discussion.
This development was made possible thanks to the financial support of the Upper Normandy region, the University of Rouen and the History Research Group (ÉA 3831) to improve user productivity on Dezède.
comment:188 by , 9 years ago
Cc: | added |
---|
comment:190 by , 9 years ago
@auvipy: You can copy a part of the work I did on django-super-inlines and integrate it in Django if you want (as soon as you mention my name). The Javascript part is especially valuable, it took me a lot of time to write it from scratch in a much more readable object-oriented way. It also fixes a few weird issues I found in the current inline code (a bug when removing an unsaved inline after its form did not validate + an invalid HTML generation). The Python part is a bit hacky, I tried to avoid rewriting formsets and other sensible parts I honestly don’t master.
comment:193 by , 9 years ago
Cc: | removed |
---|
comment:194 by , 9 years ago
Our project at The Atlantic, django-nested-admin, is another take on nested inlines, one that also allows drag-and-drop sortable inlines à la Grappelli. I don't imagine that a patch to Django would include sortable inlines, and it certainly wouldn't include Grappelli support, which would make for a lot less code. If my approach is interesting to any of the Django core developers, I would be open to reworking it into a formal pull request / patch.
comment:195 by , 8 years ago
Cc: | added |
---|
comment:196 by , 8 years ago
Cc: | added |
---|
comment:198 by , 8 years ago
Cc: | removed |
---|
comment:199 by , 8 years ago
Cc: | added |
---|
comment:200 by , 8 years ago
Cc: | added |
---|
comment:201 by , 6 years ago
Cc: | added |
---|
comment:202 by , 5 years ago
Cc: | added |
---|
comment:203 by , 5 years ago
Cc: | added |
---|
comment:204 by , 4 years ago
Cc: | added |
---|
comment:205 by , 10 months ago
Cc: | added |
---|
comment:206 by , 10 months ago
Cc: | added |
---|
comment:207 by , 10 months ago
It might be a good idea to just place nested inlines immediately after each parent. For visual distinction, nested inlines may have a slightly richer gray background at each level. It may seem a little ugly, but it's better than waiting 15 years for another solution.
I think a small number of people actually create more than two levels of inlines. So in most cases this shouldn't be a problem. What do you think?
anybody can fix this ?