#10180 closed (wontfix)
Fixed bug saving inlineformset_factory formsets
Reported by: | Simon Litchfield | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | 1.0 |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The inlineformset_factory formset save() method was setting it's fk using the 'myfk_id' no problem, but then overwriting it with form data that used 'myfk'. For new records, the form's 'myfk' is empty, so up comes an IntegrityError since null fk's were attempting to be saved. This same code used to work fine, so I assume something has fallen inconsistent around ModelForm field naming, or fk.name/fk.get_attname().
Example --
class App(models.Model): company = models.CharField(max_length=100, verbose_name='company name') ... class AppMember(models.Model): app = models.ForeignKey(App, related_name='members') name = models.CharField(max_length=50) ... AppMemberFormSet = inlineformset_factory(App, AppMember, extra=10) ... formset = AppMemberFormSet(data=request.POST or None) ... formset.instance = app # A saved app instance with an ID formset.save() # Error
Attachments (1)
Change History (6)
by , 16 years ago
Attachment: | django-inlineformset-save-new.diff added |
---|
comment:1 by , 16 years ago
comment:2 by , 16 years ago
Needs tests: | set |
---|
For the initial problem description, a complete test, preferably one integrated into the existing test suite, would help immensely. See the commits associated with #9865 #10075 for examples. As this code is modified to handle each new corner case that wasn't previously explicitly tested for, we really need to add a test to ensure that it is not broken by any subsequent changes to this code. If an integrated test is more than you are able to do, it isn't entirely clear to me how the case you are reporting differs from ones we already test, so please spell it out. Is it that you are setting the instance for the formset after creating the formset? I'm not sure that is valid...why is the instance not passed in on the call to create the formset? Changing the instance after-the-fact strikes me as something likely to cause problems, though I can't say that it will for sure without delving more into the code than I have time for at the moment.
For the added comment, is this really the same or a different problem? It looks like it may be related to #9462 but I am not sure -- you don't say what "Broken" means. It looks to me like the fix for #9462 was intended to implicitly do what you are doing in the "Worked" version, perhaps you are reporting the same thing as the comment by tobias but since neither of you have given any specifics on the nature of any failures resulting from the current code it is very hard to be sure what either of you are reporting.
comment:3 by , 16 years ago
milestone: | → 1.1 |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:4 by , 16 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
If I understand your example correctly, you should be doing this instead:
formset = AppMemberFormSet(request.POST, instance=app)
If you have a reason for assigning formset.instance, or if I'm misunderstanding your problem, please reopen this ticket with more details.
I also needed to pass the formset an empty fk instance.
Broken --
Worked --
Looks like formsets are still a bit broken (been trying to use em since early newforms-admin branch days).