Opened 4 hours ago

Last modified 3 hours ago

#36251 assigned Bug

BaseInlineFormSet modifies Meta.fields of passed form class if Meta.fields is type list

Reported by: ifeomi Owned by: jengziyi
Component: Forms Version: dev
Severity: Normal Keywords: inline formset
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 Benjamin Zagorsky)

When you make an inline formset, BaseInlineFormset adds the foreign key name to the list of fields. However, it only creates a copy of the original form’s fields if the form defines its fields in a tuple; otherwise, it modifies the original reference to fields. If you pass in a form class that has defined Meta.fields as a list, BaseInlineFormset changes the fields on that form class. This means that if that form is used for other things besides the inline formset, it now has an extra field that has been erroneously added.

Here is a minimally reproducible example. We have a UserActionForm that defines Meta.fields as a list. Notice that after initializing an instance of the FormsetClass, the fields on UserActionForm have been modified to include user (the foreign key of the inline formset). Expected behavior is that UserActionForm.Meta.fields is not modified by instantiation of a formset, and instead that the formset modifies a copy of the fields.

## models.py ###
class User(AbstractUser):
    email = models.EmailField(unique=True)

class UserAction(models.Model):
    user = models.ForeignKey(User, on_delete=models.PROTECT)
    url = models.URLField(max_length=2083)
    
## forms.py ###
from django import forms

class UserActionForm(forms.ModelForm):
    class Meta:
        model = UserAction
        fields = ["url"]
                   
### Shell ###
from common.models import User, UserAction
from django import forms

FormsetClass = forms.inlineformset_factory(User, UserAction, UserActionForm)
print(UserActionForm.Meta.fields)
# ['url']
FormsetClass()
print(UserActionForm.Meta.fields)
# ['url', 'user'] --> (should just be ['url'])

Here’s the line in BaseInlineFormset's __init__ that modifies the form’s fields - in the event that the form’s fields are not a tuple, the init appends directly to fields without making a copy.

# django/forms/models.py:L1115
class BaseInlineFormSet(BaseModelFormSet):
        def __init__(...):
                ...
                if isinstance(self.form._meta.fields, tuple):
                        self.form._meta.fields = list(self.form._meta.fields)
                        self.form._meta.fields.append(self.fk.name)

The fix for this should be fairly straightforward: rather than only copying _meta.fields if it’s a tuple, BaseInlineFormset should always make a copy of _meta.fields regardless of the type of the iterable. BaseInlineFormset already works with a copy in the case that the original form’s fields are a tuple, so this change will maintain the current behavior while preventing modifications to the original form’s fields.

Proposed Patch:

  • django/forms/models.py

    diff --git a/django/forms/models.py b/django/forms/models.py
    index d220e3c90f..4e8ef39c0d 100644
    a b class BaseInlineFormSet(BaseModelFormSet):  
    11131113        # Add the generated field to form._meta.fields if it's defined to make
    11141114        # sure validation isn't skipped on that field.
    11151115        if self.form._meta.fields and self.fk.name not in self.form._meta.fields:
    1116             if isinstance(self.form._meta.fields, tuple):
    1117                 self.form._meta.fields = list(self.form._meta.fields)
     1116            self.form._meta.fields = list(self.form._meta.fields)
    11181117            self.form._meta.fields.append(self.fk.name)
    11191118
    11201119    def initial_form_count(self):

Change History (8)

comment:1 by ifeomi, 4 hours ago

Description: modified (diff)

comment:2 by Clifford Gama, 4 hours ago

Triage Stage: UnreviewedAccepted

comment:3 by Sarah Boyce, 4 hours ago

Triage Stage: AcceptedUnreviewed

Thank you for the report!
Replicated

  • tests/model_formsets/tests.py

    a b class ModelFormsetTest(TestCase):  
    16851685
    16861686            class Meta:
    16871687                model = Book
    1688                 fields = ("title",)
     1688                fields = ["title"]
    16891689
    16901690        BookFormSet = inlineformset_factory(Author, Book, form=BookForm)
     1691        self.assertEqual(BookForm.Meta.fields, ["title"])
    16911692        data = {
    16921693            "book_set-TOTAL_FORMS": "3",
    16931694            "book_set-INITIAL_FORMS": "0",
    class ModelFormsetTest(TestCase):  
    16981699        }
    16991700        author = Author.objects.create(name="test")
    17001701        formset = BookFormSet(data, instance=author)
     1702        self.assertEqual(BookForm.Meta.fields, ["title"])
    17011703        self.assertEqual(
    17021704            formset.errors,
    17031705            [{}, {"__all__": ["Please correct the duplicate values below."]}, {}],

comment:4 by Sarah Boyce, 4 hours ago

Triage Stage: UnreviewedAccepted
Version: 5.2dev

comment:5 by jengziyi, 4 hours ago

Owner: set to jengziyi
Status: newassigned

comment:6 by Benjamin Zagorsky, 3 hours ago

Description: modified (diff)

comment:7 by Benjamin Zagorsky, 3 hours ago

Description: modified (diff)

comment:8 by Benjamin Zagorsky, 3 hours ago

I've verified the bug on Django 4.2 and Django 5.1.

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