#36251: BaseInlineFormSet modifies Meta.fields of passed form class if
Meta.fields
is type list
--------------------------------+--------------------------------------
Reporter: ifeomi | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 5.2
Severity: Normal | Resolution:
Keywords: inline formset | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------
Description changed by ifeomi:
Old description:
> 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.
>
> {{{#!python
> ## models.py ###
> class User(AbstractUser):
> email = models.EmailField(unique=True)
>
> class UserAction(models.Model):
> user = models.ForeignKey(User)
> url = models.URLField(max_length=2083)
>
> ## forms.py ###
> 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.
>
> {{{#!python
> # 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:**
>
> {{{#!diff
> diff --git a/django/forms/models.py b/django/forms/models.py
> index d220e3c90f..4e8ef39c0d 100644
> --- a/django/forms/models.py
> +++ b/django/forms/models.py
> @@ -1113,8 +1113,7 @@ class BaseInlineFormSet(BaseModelFormSet):
> # Add the generated field to form._meta.fields if it's defined
> to make
> # sure validation isn't skipped on that field.
> if self.form._meta.fields and self.fk.name not in
> self.form._meta.fields:
> - if isinstance(self.form._meta.fields, tuple):
> - self.form._meta.fields = list(self.form._meta.fields)
> + self.form._meta.fields = list(self.form._meta.fields)
> self.form._meta.fields.append(self.fk.name)
>
> def initial_form_count(self):
> }}}
New description:
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.
{{{#!python
## models.py ###
class User(AbstractUser):
email = models.EmailField(unique=True)
class UserAction(models.Model):
user = models.ForeignKey(User)
url = models.URLField(max_length=2083)
## forms.py ###
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.
{{{#!python
# 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:**
{{{#!diff
diff --git a/django/forms/models.py b/django/forms/models.py
index d220e3c90f..4e8ef39c0d 100644
--- a/django/forms/models.py
+++ b/django/forms/models.py
@@ -1113,8 +1113,7 @@ class BaseInlineFormSet(BaseModelFormSet):
# Add the generated field to form._meta.fields if it's defined to
make
# sure validation isn't skipped on that field.
if self.form._meta.fields and self.fk.name not in
self.form._meta.fields:
- if isinstance(self.form._meta.fields, tuple):
- self.form._meta.fields = list(self.form._meta.fields)
+ self.form._meta.fields = list(self.form._meta.fields)
self.form._meta.fields.append(self.fk.name)
def initial_form_count(self):
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/36251#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/010701958b13fa1d-1977ba8b-adad-47f5-9b67-244fba75f161-000000%40eu-central-1.amazonses.com.
