Opened 21 months ago

Closed 3 months ago

#27560 closed Bug (invalid)

Formset.save() crashes for model with foreign key to concrete base model

Reported by: Lorenzo Peña Owned by: nobody
Component: Forms Version: 1.9
Severity: Normal Keywords: formset inline_formset inline_formsetfactory
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Lorenzo Peña)

Model hierharchy

class ConcreteBase(models.Model):
    ...


class Descendant(ConcreteBase):
    ...

Master / Detail type models (e.g. Order and OrderDetail)

class MasterModel(models.Model):
    ...


class DetailModel(models.Model):
    
    master = models.ForeignKey(MasterModel, ...)
    concrete_base = models.ForeignKey(ConcreteBase, ...)

Form definition

class MasterModelForm(forms.ModelForm):
    ...


class DetailModelForm(forms.ModelForm):
    ...
    # master excluded from fields as will be manually added in view
    ...

Formset definition

DetailModelFormset = inlineformset_factory(MasterModel, DetailModel, form=DetailModelForm)

View definition

from django.views.generic.edit import CreateView #### <---- the error only happens on create view of MasterModel

class MasterModelAdd(CreateView):
    model = MasterModel
    form_class = MasterModelForm
    ...

    def form_valid(self, form):
        master_instance = form.save()
        detail_formset = DetailModelFormset()(self.request.POST)
        detail_instances = detail_formset.save(commit=False) ### <------ this line raises the error
        for detail_instance in detail_instances: ### <----- it never gets here
            detail_instance.master = master_instance
            detail_instance.save()

When calling formset.save() I'm getting an error from django/forms/models.py (line 910) regardless of the value of commit passed.

910:   pk_value = getattr(self.instance, self.fk.remote_field.field_name)
911:   setattr(obj, self.fk.get_attname(), getattr(pk_value, 'pk', pk_value))

The getattr call fails

if I wrap both lines in: if hasattr(self.instance, self.fk.remote_field.field_name):
it works good.

Not sure if the condition is the way to fix the bug or it's just preventing the original cause from happening.

Change History (8)

comment:1 Changed 21 months ago by Tim Graham

Summary: Formset.save for model with foreign key to concrete base modelFormset.save() crashes for model with foreign key to concrete base model
Type: UncategorizedBug

Could you include more complete details about how to reproduce the issue? My try to reproduce gives save() prohibited to prevent data loss due to unsaved related object 'some_model'. at formset.save().

comment:2 Changed 21 months ago by Lorenzo Peña

Description: modified (diff)

comment:3 Changed 21 months ago by Lorenzo Peña

Updated ticket to expand context so that bug may be (hopefully) reproduced.

comment:4 Changed 21 months ago by Tim Graham

This report is awfully complicated and without spending more than a few minutes investigating I can't say whether or not this is a bug in Django or in your code. One thing to try might be: DetailModelFormset(self.request.POST, instance=master_instance).

It seems like excluding master from DetailModelForm may also be related. (How can you have an inline formset without the field linking the subforms to the parent?)

It would be nice if you do more to confirm that this is actually a bug, such as by using our support channels. Putting together a sample project with a test case that someone could download to quickly reproduce the issue would also help.

comment:5 Changed 21 months ago by Lorenzo Peña

I'll have to investigate, but for the record:

  1. In this view, I'm creating master and details at the same time, so I don't have really anything to pass because nothing exists yet.
  2. Regarding the fact that I'm omitting the link to master, do you suggest a hidden field?
  3. This exact pattern works in at least one more place in my codebase, only crashing when the abstract base is involved in the child formset.

comment:6 Changed 20 months ago by Tim Graham

Triage Stage: UnreviewedAccepted

I haven't investigated any further but tentatively accepting.

comment:7 Changed 3 months ago by Bartosz Grabski

I've tested this and found two issues with your code in form_valid:

        ...
        detail_formset = DetailModelFormset(self.request.POST)  <---- there was a bug in your previous code: DetailModelFormset()(self.request.POST) 
        if detail_formset.is_valid():  <--- you were missing formset validation
            detail_instances = detail_formset.save(commit=False)
            ...

After fixing that, the code works fine.

comment:8 Changed 3 months ago by Lorenzo Peña

Resolution: invalid
Status: newclosed

Looks like it was a bug in my code.

Note: See TracTickets for help on using tickets.
Back to Top