Opened 13 months ago

Last modified 12 months ago

#27560 new Bug

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 (6)

comment:1 Changed 12 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 12 months ago by Lorenzo Peña

Description: modified (diff)

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

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

comment:4 Changed 12 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 12 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 12 months ago by Tim Graham

Triage Stage: UnreviewedAccepted

I haven't investigated any further but tentatively accepting.

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