form.is_valid() changes a ModelForm's associated instance now?

246 views
Skip to first unread message

Jumpfroggy

unread,
Mar 2, 2011, 2:41:54 PM3/2/11
to Django users
I discovered the recent changes to the "is_valid()" model form method
in 1.2, and I'd love to hear thoughts on the rationale behind this
change.

I have this kind of code:

item = Item.objects.get(id=1)
print 'item.value: %s' % item.value
form = ItemModelForm(request.POST, instance=item)
old_item = item
if form.is_valid():
print 'old_item.value: %s' % old_item.value
print "form.cleaned_data['value']: %s" %
form.cleaned_data['value']
new_item = form.save(commit=False)
print 'new_item.value: %s' % new_item.value

And I'd expect this output:

item.value: old_value
old_item.value: old_value
form.cleaned_data['value']: new_value
new_item.value: new_value

But instead I get this:

item.value: old_value
old_item.value: new_value
form.cleaned_data['value']: new_value
new_item.value: new_value

It seems like the "is_valid()" function not only checks for validation
errors and creates the "form.cleaned_data" members, it also modifies
the instance of the model attached to this form. I just looked over
the docs, and I can now see that this is implied but not really
explicitly referenced here:

http://docs.djangoproject.com/en/1.2/topics/forms/modelforms/

The first time you call is_valid() or access the errors attribute
of a ModelForm has always triggered form validation, but as of Django
1.2, it will also trigger model validation. This has the side-effect
of cleaning the model you pass to the ModelForm constructor. For
instance, calling is_valid() on your form will convert any date fields
on your model to actual date objects.

The problem is not that the instance is being "cleaned". It's that
the instance is being modified, then cleaned. It would help if the
docs specified this as well, so we know what the side effects of
"is_valid()" really are.

Also, doesn't this make this code obsolete?

item = form.save()

And:

item = form.save(commit=False)
item.save()

We could instead just do this:

form = ItemModelForm(request.POST, instance=item)
if form.is_valid():
item.save()

(assuming that we're modifying an existing instance, and not creating
a new one)

Since the original "item" is modified, I could just save that as well,
right? Is that what we truly want to happen? Seems like the
modification of the original instance in this case is an unintentional
side effect of needing to clean the model for validation. It seems
like it would be better if is_valid() created a copy of the instance,
and then modified & cleaned that copy instead of the original.
Otherwise it seems tricky to have a validation method secretly
modifying seemingly unrelated data.

I just want to wrap my head around these new changes, and maybe
understand the rationale behind them. Thanks!

PS. The solution to my specific problem above is to do this:

item = Item.objects.get(id=1)
print 'item.value: %s' % item.value
form = ItemModelForm(request.POST, instance=item)
# NOTE: This fixes the problem:
old_item = copy.deepcopy(item)
if form.is_valid():
print 'old_item.value: %s' % old_item.value
print "form.cleaned_data['value']: %s" %
form.cleaned_data['value']
new_item = form.save(commit=False)
print 'new_item.value: %s' % new_item.value

Vermus

unread,
Mar 24, 2011, 4:02:19 AM3/24/11
to Django users
>     item = Item.objects.get(id=1)
>     print 'item.value: %s' % item.value
>     form = ItemModelForm(request.POST, instance=item)
>     # NOTE: This fixes the problem:
>     old_item = copy.deepcopy(item)
>     if form.is_valid():
>         print 'old_item.value: %s' % old_item.value
>         print "form.cleaned_data['value']: %s" %
> form.cleaned_data['value']
>         new_item = form.save(commit=False)
>         print 'new_item.value: %s' % new_item.value

What about many-to-many fields?
Reply all
Reply to author
Forward
0 new messages