Opened 6 years ago

Closed 6 years ago

#29739 closed Bug (duplicate)

BaseModelFormSet ignores excess rows in initial when extra < len(initial)

Reported by: Mark Gensler Owned by: nobody
Component: Forms Version: 2.1
Severity: Normal Keywords: modelformset
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Mark Gensler)

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.

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,

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)).

Change History (4)

comment:1 by Mark Gensler, 6 years ago

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 a preference.

Last edited 6 years ago by Mark Gensler (previous) (diff)

comment:2 by Mark Gensler, 6 years ago

Description: modified (diff)

comment:3 by Mark Gensler, 6 years ago

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 leave this here for further comment...

Last edited 6 years ago by Mark Gensler (previous) (diff)

comment:4 by Carlton Gibson, 6 years ago

Resolution: duplicate
Status: newclosed

I think this is just a duplicate of #16995 (as you say). I can't see that the situation has changed from Carl Meyer's comment there. (But further comments on #16995 I think...) Thanks.

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