#24541 closed Cleanup/optimization (fixed)
Clarify ModelFormSet's handling of initial data
Reported by: | Johannes Maron | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | dev |
Severity: | Normal | Keywords: | modelformset |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
If you create a ModelFormSet
and pass initial
data for some forms, the forms are evaluated as has_changed
False.
All in all that is true, but it has the the side effect that the forms in the formset don't get saved, tho they contain data.
The error is here:
https://github.com/django/django/blob/master/django/forms/models.py#L777-L778
It should not only be checked for has_changed
but also for initial data.
Attachments (1)
Change History (19)
comment:1 by , 10 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 10 years ago
Resolution: | invalid |
---|---|
Status: | closed → new |
That does not work for ModelFormSets
as the pop initial
to initial_extra
and only except existing objects using the Queryset.
There currently is no way to create objects using a ModelFormSet
with initial data.
I get your point, about compatibility, but you should decide if this odd behavior is a bug or a feature. Personally it feels like a bug. Especially as Model and regular form sets handle initial data so differently. There is no consistent behavior nor any documentation about this.
comment:3 by , 10 years ago
Easy pickings: | unset |
---|
I had no trouble with this view:
from django.forms.models import modelformset_factory def test_view(request): qs = Poll.objects.none() PollFormSet = modelformset_factory(Poll, fields='__all__') if request.method == 'POST': formset = PollFormSet(request.POST, queryset=qs) if formset.is_valid(): formset.save() else: formset = PollFormSet(queryset=qs, initial=[ {'question': 'initial', 'pub_date': timezone.now()}]) return render(request, 'polls/manage.html', {'formset': formset})
Can you provide a snippet that demonstrates your problem?
comment:4 by , 10 years ago
As I said:
It does not work if your submitted are not yet created and you can't pass a queryset.
from django.forms.models import modelformset_factory def test_view(request): qs = Poll.objects.none() PollFormSet = modelformset_factory(Poll, fields='__all__') if request.method == 'POST': formset = PollFormSet(request.POST, initial=[{'question': 'Foo'}, {'question': 'Bar'}]) if formset.is_valid(): obj_list = formset.save() assert len(obj_list) == 2 else: formset = PollFormSet(queryset=qs, initial=[ {'question': 'initial', 'pub_date': timezone.now()}]) return render(request, 'polls/manage.html', {'formset': formset})
This will fail.
comment:5 by , 10 years ago
Description: | modified (diff) |
---|
comment:6 by , 10 years ago
I'm using the models from the tutorial. Poll
doesn't have a vote
field also inital
is a typo in your example. Maybe you could explain your actual use case instead of modifying my example as I don't understand what the expected behavior or usefulness of that snippet is.
comment:7 by , 10 years ago
Sorry, I think I fixed the example.
Let me try to be a little more specific to what's wrong:
There are two ways to pre fill a ModelFormSet, by passing a queryset or initial data. The later is described here:
https://docs.djangoproject.com/en/dev/topics/forms/formsets/#using-initial-data-with-a-formset
Sadly a ModelFormSet only takes queryset
in account for initial_form_count
https://github.com/django/django/blob/master/django/forms/models.py#L579
Therefore initial data is used for form construction but not accounted for form saving, which is very much unexpected.
https://github.com/django/django/blob/master/django/forms/models.py#L606-L611
comment:8 by , 10 years ago
The fix would be to change
https://github.com/django/django/blob/master/django/forms/models.py#L777
to:
if not (form.has_changed() or form.initial):
comment:9 by , 10 years ago
What are you trying to accomplish by passing initial
to PollFormSet()
along with request.POST
? Do you want to override request.POST
with the initial values or use them if no value is provided in request.POST
?
comment:10 by , 10 years ago
Well it should work in a FormView right? The form view passes the initial
as well as request.POST
.
https://github.com/django/django/blob/master/django/views/generic/edit.py#L76-L90
comment:11 by , 10 years ago
If you want the initial data to be considered "changed", then don't pass it to the bound FormSet as in my example. Is there a problem with that solution?
comment:12 by , 10 years ago
I know that I can make it work, I'm just asking myself the questions if it shouldn't just work out of the box.
A ModelFormSet would be the only model, that has a different behavior regarding initial data and therefore also doesn't work with the generic FormView.
This really doesn't sound like a feature, it's a bug.
comment:13 by , 10 years ago
Component: | Forms → Documentation |
---|---|
Has patch: | set |
Summary: | save_new_objects in ModelFromsets do not handle intitial data → Clarify ModelFromsets handling of initial data |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Cleanup/optimization |
Accepting as a documentation issue. As I said before, I don't think changing the behavior is acceptable given backwards compatibility. For example, your patch assumes that the initial data includes all fields of the model. Since unchanged formsets aren't validated, saving would fail with a database IntegrityError
if a required field is omitted.
Let me know if anything is unclear or if you think something additional should be added to the documentation. I've attached a proposed patch.
by , 10 years ago
Attachment: | 24541.diff added |
---|
comment:15 by , 10 years ago
Summary: | Clarify ModelFromsets handling of initial data → Clarify ModelFormSet's handling of initial data |
---|---|
Triage Stage: | Accepted → Ready for checkin |
You should use initial forms instead of extra forms if you require those populated forms to be submitted. Changing the behavior to validate and save unchanged extra forms would be backwards incompatible. Feel free to reopen if I missed something.