Code

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#10180 closed (wontfix)

Fixed bug saving inlineformset_factory formsets

Reported by: simon29 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: UI/UX:

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)

django-inlineformset-save-new.diff (688 bytes) - added by simon29 5 years ago.

Download all attachments as: .zip

Change History (6)

Changed 5 years ago by simon29

comment:1 Changed 5 years ago by simon29

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I also needed to pass the formset an empty fk instance.

Broken --

formset = AppMemberFormSet(data=request.POST or None)

Worked --

formset = AppMemberFormSet(instance=App(), data=request.POST or None)

Looks like formsets are still a bit broken (been trying to use em since early newforms-admin branch days).

comment:2 Changed 5 years ago by kmtracey

  • 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 Changed 5 years ago by jacob

  • milestone set to 1.1
  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 5 years ago by jkocherhans

  • Resolution set to wontfix
  • Status changed from new to 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.

comment:5 Changed 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.