[Django] #29739: BaseModelFormSet ignores excess rows in initial when extra < len(initial)
Django
--------------------------------------------+------------------------------
Reporter: S. Dole Melipone | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.1
Severity: Normal | Keywords: modelformset
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------------+------------------------------
When passing initial data through `BaseModelFormSet.__init__(...,
initial=)` and the length of the initial data exceeds the number of extra
forms defined in `modelformsetfactory(..., extra=)`, then the data in the
range `self.initial_extra[self.extra:]` will be ignored.
E.g.
{{{#!python
class Person(models.Model):
name = models.CharField(max_length=100)
initial = [{'name': 'Foo'}, {'name': 'Bar'}, {'name': 'Baz'}]
PersonFormSet = modelformsetfactory(Person, extra=2)
formset = MyModelFormSet(initial=initial)
}}}
Now, assuming that `models.Person.objects.all()` is empty,
{{{#!python
len(formset.forms) == 2
True
}}}
This is because `BaseFormSet.total_form_count()` blindly uses self.extra
and does not take into account the additional data in
`self.initial_extra`.
In order to work around this users would check `len(initial)` before
constructing the class and modify the `extra` kwarg to be
`max(len(initial), some_default_value)`. This causes issues when writing
abstract code which constructs the class separately from initialising it,
or reuses the same class with differing initial data. In these cases the
length of the initial data may not be known at the time of class
construction.
Possible solutions are the number of extra forms equals `self.extra +
len(self.initial_extra)` or `max(self.extra, len(self.initial_extra))`.
--
Ticket URL: <https://code.djangoproject.com/ticket/29739>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: S. Dole Melipone | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.1
Keywords: modelformset | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by S. Dole Melipone):
I am happy to write a patch for this. I lean towards max rather than
adding len(initial_extra) forms, it all depends on the philosophy of what
`extra` represents.
A fix could be implemented by either adding a
`BaseModelFormSet.total_form_count()` method, or adding a
`BaseFormSet.get_extra_form_count()` method analogous to
`get_initial_form_count()` and overwritten by `BaseModelFormSet`. I am
happy to do either if someone could provide guidance.
--
Ticket URL: <https://code.djangoproject.com/ticket/29739#comment:1>
Django
Reporter: S. Dole Melipone | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.1
Keywords: modelformset | 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 S. Dole Melipone:
Old description:
New description:
When passing initial data through `BaseModelFormSet.__init__(...,
initial=)` and the length of the initial data exceeds the number of extra
forms defined in `modelformsetfactory(..., extra=)`, then the data in the
range `self.initial_extra[self.extra:]` will be ignored.
E.g.
{{{#!python
class Person(models.Model):
name = models.CharField(max_length=100)
initial = [{'name': 'Foo'}, {'name': 'Bar'}, {'name': 'Baz'}]
PersonFormSet = modelformsetfactory(Person, extra=2)
formset = MyModelFormSet(initial=initial)
}}}
Now, assuming that `models.Person.objects.all()` is empty,
{{{#!python
len(formset.forms) == 2
True
}}}
This is because `BaseFormSet.total_form_count()` blindly uses self.extra
and does not take into account the additional data in
`self.initial_extra`.
In order to work around this users would check `len(initial)` before
constructing the class and modify the `extra` kwarg to be large enough.
This causes issues when writing abstract code which constructs the class
separately from initialising it, or reuses the same class with differing
initial data. In these cases the length of the initial data may not be
known at the time of class construction.
Possible solutions are the number of extra forms equals `self.extra +
len(self.initial_extra)` or `max(self.extra, len(self.initial_extra))`.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29739#comment:2>
Django
Reporter: S. Dole Melipone | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.1
Keywords: modelformset | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by S. Dole Melipone):
Sorry, I see that this was actually raised in the early stages of
discussion of #16995. I find it hard to believe that the current behaviour
is desired! So I will submit a patch...
--
Ticket URL: <https://code.djangoproject.com/ticket/29739#comment:3>
Django
Reporter: S. Dole Melipone | Owner: nobody
Component: Forms | Version: 2.1
Severity: Normal | Resolution: duplicate
Keywords: modelformset | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => duplicate
Comment:
I think this is just a duplicate of #16995 (as you say). I can't see that
the situation has changed from
[https://code.djangoproject.com/ticket/16995#comment:1 Carl Meyer's
comment there]. (But further comments on #16995 I think...) Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/29739#comment:4>