django.forms.BaseModelForm._post_clean updates instance even when form validation fails

[lanyjie]

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
   ...

[Luke Plant]

I'm afraid this bug report is very unclear - please give an example that shows an actual bug and re-open. Thanks!

[Vladimir Magamedov]

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.

[Bas Peschier]

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.

[Tim Graham]

Duplicate of #14885 which was fixed by a documentation update.