Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#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 Johannes Maron)

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)

24541.diff (714 bytes ) - added by Tim Graham 9 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 by Tim Graham, 9 years ago

Resolution: invalid
Status: newclosed

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.

comment:2 by Johannes Maron, 9 years ago

Resolution: invalid
Status: closednew

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 Tim Graham, 9 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 Johannes Maron, 9 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.

Last edited 9 years ago by Johannes Maron (previous) (diff)

comment:5 by Johannes Maron, 9 years ago

Description: modified (diff)

comment:6 by Tim Graham, 9 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 Johannes Maron, 9 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 Johannes Maron, 9 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 Tim Graham, 9 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 Johannes Maron, 9 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 Tim Graham, 9 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 Johannes Maron, 9 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 Tim Graham, 9 years ago

Component: FormsDocumentation
Has patch: set
Summary: save_new_objects in ModelFromsets do not handle intitial dataClarify ModelFromsets handling of initial data
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/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 Tim Graham, 9 years ago

Attachment: 24541.diff added

comment:14 by Johannes Maron, 9 years ago

Fair enough, thanks.

comment:15 by Tim Graham, 9 years ago

Summary: Clarify ModelFromsets handling of initial dataClarify ModelFormSet's handling of initial data
Triage Stage: AcceptedReady for checkin

comment:16 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: newclosed

In 6de3a1e2:

Fixed #24541 -- Clarified ModelFormSet's handling of initial data.

comment:17 by Tim Graham <timograham@…>, 9 years ago

In 7c7d29a:

[1.7.x] Fixed #24541 -- Clarified ModelFormSet's handling of initial data.

Backport of 6de3a1e2c34ae5bfcdec3ebbf3d682aa578ecae0 from master

comment:18 by Tim Graham <timograham@…>, 9 years ago

In a70dea2c:

[1.8.x] Fixed #24541 -- Clarified ModelFormSet's handling of initial data.

Backport of 6de3a1e2c34ae5bfcdec3ebbf3d682aa578ecae0 from master

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