[Django] #21332: BaseInlineFormSet.add_fields adds multiple InlineForeignKeyFields
Django
-------------------------------+------------------------
Reporter: Raumkraut | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: 1.6-beta-1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------
When a formset object has its forms iterated over, the
`_construct_form(...)` method calls `self.add_fields(...)` against each
form in the set.
However, for a `BaseInlineFormSet`, the add_fields implementation creates
an `InlineForeignKeyField`, and appends it to `form._meta.fields`. It does
this for each form, and since `form._meta` is a class attribute, it ends
up with multiple duplicate InlineForeignKeyField instances.
This doesn't appear to be normally evidenced in any usual output, since
all the generated FK fields have the same name, so overwrite each other in
name-keyed dictionaries of the fields.
Tested on the latest stable/1.6.x branch
--
Ticket URL: <https://code.djangoproject.com/ticket/21332>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Raumkraut | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: 1.6-beta-1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Raumkraut):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Also note that, unless I'm mistaken, the `InlineForeignKeyField` instances
will continue to be appended to `_meta.fields` for as long as the server
is running, and formsets are being iterated over. Which would make this
something of a memory leak.
--
Ticket URL: <https://code.djangoproject.com/ticket/21332#comment:1>
Django
Reporter: Raumkraut | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: 1.6-beta-1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
Do you have any ideas to address this? Could you provide a sample project
that demonstrates the problem?
--
Ticket URL: <https://code.djangoproject.com/ticket/21332#comment:2>
Django
Reporter: Raumkraut | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: 1.6-beta-1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Raumkraut):
Okay, so I was wrong about what was getting appended to _meta.fields: It's
not InlineForeignKeyField instances which get added, it's only the
//name// of the foreign-key field.
I've cooked up a basic project to demonstrate the issue:
https://github.com/Raumkraut/test_formsets
It should be enough to clone, syncdb and runserver. The root page
demonstrates the issue in question.
Since it is just the name of the field which gets appended repeatedly,
unless I've missed something, it seems a fix could be fairly simple:
{{{
diff --git a/django/forms/models.py b/django/forms/models.py
index 71da433..20a3d54 100644
--- a/django/forms/models.py
+++ b/django/forms/models.py
@@ -893,7 +893,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 form._meta.fields:
+ if form._meta.fields and self.fk.name not in form._meta.fields:
if isinstance(form._meta.fields, tuple):
form._meta.fields = list(form._meta.fields)
form._meta.fields.append(self.fk.name)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21332#comment:3>
Django
Reporter: Raumkraut | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: 1.6-beta-1
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akaariai):
* stage: Unreviewed => Accepted
Comment:
I can reproduce, on 1.5.x, 1.6.x and master. The project doesn't seem to
be doing anything special.
--
Ticket URL: <https://code.djangoproject.com/ticket/21332#comment:4>
Django
Reporter: Raumkraut | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: Forms | 1.6-beta-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by carljm):
* type: Uncategorized => Cleanup/optimization
--
Ticket URL: <https://code.djangoproject.com/ticket/21332#comment:5>
Django
Reporter: Raumkraut | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: Forms | 1.6-beta-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
Comment:
#26537 is a duplicate with a [https://github.com/django/django/pull/6503
PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/21332#comment:6>
Django
Reporter: Raumkraut | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: Forms | 1.6-beta-1
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/21332#comment:7>
Django
Reporter: Raumkraut | Owner: nobody
Type: | Status: closed
Cleanup/optimization | Version:
Component: Forms | 1.6-beta-1
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"a5c8a6ce19eabd96bf3b8c7bb4fb487f53928f3b" a5c8a6ce]:
{{{
#!CommitTicketReference repository=""
revision="a5c8a6ce19eabd96bf3b8c7bb4fb487f53928f3b"
Fixed #21332, #26538 -- Fixed inconsistent and duplicate form fields on
inline formsets.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21332#comment:8>