#14885 closed Cleanup/optimization (fixed)
is_valid for ModelForm changes instance if instance is provided
| Reported by: | italomaia | Owned by: | stumbles |
|---|---|---|---|
| Component: | Documentation | Version: | 1.2 |
| Severity: | Normal | Keywords: | forms, is_valid, bad data |
| Cc: | Torsten Bronger | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description
See this example:
def val_check(value):
from django.core.exceptions import ValidationError
raise ValidationError('Some reason')
class Model(models.Model):
image = models.ImageField(upload_to='some/place', validators=[val_check(])
class ModelForm(forms.ModelForm):
pass
def view(request, model_id):
instance = Model.objects.get(pk=model_id)
if request.method=='POST':
form = ModelForm(request.POST, instance=instance)
else: form = ModelForm(instance=instance)
if form.is_bound and form.is_valid():
form.save()
# do something else
return simple.direct_to_template(request, {'template':'some.html', 'extra_context':{'model':instance}})
In the example above, if i send a post request with some data for the field image, instance will be modified. If i use the value of instance in my 'some.html' i'll get invalid data in my template. This seems wrong.
Attachments (1)
Change History (8)
comment:1 by , 15 years ago
| Resolution: | → invalid |
|---|---|
| Status: | new → closed |
comment:3 by , 14 years ago
| Cc: | added |
|---|---|
| Component: | Uncategorized → Forms |
| Easy pickings: | unset |
| Resolution: | invalid |
| Severity: | → Normal |
| Status: | closed → reopened |
| Type: | → Bug |
| UI/UX: | unset |
If necessary I will create a snippet for reproducing this, but actually the summary is complete: If you pass an "instance" to a model form constructor and later on call ".is_valid()" on that form, that given instance is changed in place. Very often you don't care about this object. But for example, if you want to compare the given instance with the new one returned by my_model_form.save(), changes are never detected.
I cannot read this behaviour from the docs. It is not about model validation here: The fields are really updated and not just normalised.
See also http://permalink.gmane.org/gmane.comp.python.django.user/132931 and http://groups.google.com/group/django-users/msg/2e31bfaefd8351cf.
comment:4 by , 14 years ago
| Component: | Forms → Documentation |
|---|---|
| Easy pickings: | set |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Bug → Cleanup/optimization |
Accepting as a documentation issue. We could make it clearer that calling is_valid() will update the instance passed to the ModelForm.
by , 13 years ago
| Attachment: | trac#14885-is_valid.diff added |
|---|
Explain that is_valid() changes the model instance in place.
comment:5 by , 13 years ago
| Has patch: | set |
|---|---|
| Owner: | changed from to |
| Status: | reopened → new |
I've added a patch to point out both that is_valid() causes the model instance to be modified in place, and also that this modification can be partial.
comment:6 by , 13 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
It is not clear from the report what this ticket is reporting, it doesn't help at all that the example code isn't correct/complete:
simple.direct_to_template()function.If the OP or anyone wants to reopen this, please provide a working test case and a clear description of the issue at hand.
Remember Since Django 1.2 ModelFom validation also triggers Model validation