Opened 14 years ago
Closed 11 years ago
#15587 closed Bug (duplicate)
django.forms.BaseModelForm._post_clean updates instance even when form validation fails
Reported by: | lanyjie | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | 1.3 |
Severity: | Normal | Keywords: | data validation |
Cc: | vmagamedov@…, Sergey Maranchuk | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
This method will update the model instance with self.cleaned_data.
self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
however, the cleaned_data might not be good. It is probably good to have a single line inserted before the first line of code of this method:
def _post_clean(self): if self._errors: return ...
Change History (7)
comment:1 by , 14 years ago
Resolution: | → needsinfo |
---|---|
Severity: | → Normal |
Status: | new → closed |
Type: | → Uncategorized |
comment:2 by , 14 years ago
Type: | Uncategorized → Bug |
---|
comment:3 by , 13 years ago
Cc: | added |
---|---|
Easy pickings: | unset |
Has patch: | unset |
Resolution: | needsinfo |
Status: | closed → reopened |
UI/UX: | unset |
Version: | 1.2 → 1.3 |
I have the same problem with model forms:
>>> from django.contrib.auth.models import User >>> from django.contrib.auth.forms import UserChangeForm >>> user = User.objects.all()[0] >>> form = UserChangeForm({'first_name': 'Vladimir', 'email': 'invalid'}, instance=user) >>> user.first_name u'' >>> form.is_valid() False >>> user.first_name u'Vladimir'
As you can see, this form changes provided instance after validation. This behavior is not expected and I'm considering this as a bug.
Or this is not a bug? Maybe this is a design decision to make it possible to validate form data in the BaseModel.clean_fields
method? This is odd for me.
Provided single-line patch in the ticket description isn't taking into account that validation error can be raised later by the model validation, so this is wouldn't fix this problem.
comment:4 by , 13 years ago
Cc: | added |
---|
comment:5 by , 13 years ago
Needs documentation: | set |
---|---|
Summary: | problem in django.forms.BaseModelForm._post_clean → django.forms.BaseModelForm._post_clean updates instance even when form validation fails |
Triage Stage: | Unreviewed → Accepted |
The question here is at which point the form is allowed to update the instance. Other tickets are also having problems here for their own reasons (#15995, #16423).
There is a strong argument for doing this while validating since ModelForm actually promises to fully validate the data (form and model). For this the instance needs to be updated.
However, there is case for reminding people that in current workflow validation implies updating the model to independently validate the model; so if anybody can create a patch for the docs explaining this.
Otherwise this will become a DDN and we need to discuss whether a form validation error means the field should not be updated on the model.
comment:6 by , 12 years ago
Status: | reopened → new |
---|
comment:7 by , 11 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Duplicate of #14885 which was fixed by a documentation update.
I'm afraid this bug report is very unclear - please give an example that shows an actual bug and re-open. Thanks!