Feature request: ModelForm validation error handling after commit=False

[Oroku Saki]

For lack of a good explanation, I'll include an example first:

#First, the models and forms used in example:

class Account(models.Model):
    subdomain = SubdomainField(max_length=25, unique=True)


class Administrator(models.Model):
    account = models.ForeignKey(Account, related_name='administrators')
    username = LowerAlphanumericField(max_length=50)
    email = models.EmailField()
    
    class Meta:
        unique_together = (('account', 'username'), ('account', 'email'))


class AdministratorForm(ModelForm):
    class Meta:
        model = Administrator
        exclude = ('account',)

And, here's the example usage that breaks:

#views.py

def create_administrator(request):
    from django.db.utils import IntegrityError
    from myproject.apps.account.forms import AdministratorForm
    if request.method == 'POST':
        form = AdministratorForm(request.POST)
        if form.is_valid():
            instance = form.save(commit=False)
            instance.account = request.account  # ``account`` is attached to ``request`` in custom middleware.
            instance.save()
            # What happens if there is an error here such as IntegrityError with one of the 2 unique_together fields ('username', and 'email').
            # You cannot tell which field failed the ``unique_together`` test. validate_unique skips over the excluded fields in the form.
            #
            # You could run instance.full_clean() first and catch ValidationErrors and handle them manually, but this would not be very DRY after about 3 views.
            # I don't know what the new version would be like, but I can imagine a way to validate the instance and then add the errors into the form again or something.

I probably made this very confusing, but basically there is no way to use commit=False and still use ModelForm's built-in validation conveniences.

[Oroku Saki]

Here is one case during the model validation talks in January where somebody talks about precisely what I'm dealing with, although he puts it better:

​http://osdir.com/ml/django-developers/2010-01/msg00169.html

[Oroku Saki]

One solution I could propose as an abstract idea is to allow for the inclusion of extra data during the creation of a ModelForm instance:

form = MySpamModelForm(request.POST, data_override={'account': request.account})
if form.is_valid():
    instance = form.save()

This doesn't allow for the use of instance.pk, but it at lease allows you to add request related data to the instance before creation.

[Russell Keith-Magee]

Please don't triage your own tickets. The DDN marker indicates that the idea has been accepted prima facie, but a final decision is required.

Firstly, the assertion in the problem statement ("there is no way to use commit=False and still use ModelForm?'s built-in validation conveniences") is false. When you call 'form.save(clean=False)', you're saying that you're taking control of making sure the instance is saved properly. If you're using model validation, this means taking control of the validation process, including calls to full_clean(), and the handling of the resultant errors. The docs added in r12206 pretty much say this explicitly, and give examples of how it should be handled.

As for potential fixes - the "data override" idea you suggest isn't required- it's already possible to do this by specifying an instance.

    if request.method == 'POST':
        new_obj = Administrator()
        new_obj.account = request.account
        form = AdministratorForm(data=request.POST, instance=new_obj)
        if form.is_valid():
            form.save()
        ...

[Oroku Saki]

@Russellm

There's still one problem with doing what you did above.

Thanks for the code snippet BTW first of all. I ended up using form.instance.account = request.account before my call to form.is_valid() , but in the case of adding data to an instance or a form during all of this is usually due to the fact that the field is not included in the form because of editable=False on the model or exclude=('field',) in the form. When this is the case the fields that are excluded still don't go through validation with form.is_valid() . Is it reasonable to request the addition of either (A) If a field's value is set via form.instance.account = request.account , then it is included in validation again, or (B) A way to add the field back into validation again manually such as form.include_in_validation = 'account' .

If you'd like, I can create a new ticket, or is it relevant enough to this ticket, or do you think it's even worth the time for what I mentioned?

[legutierr]

I agree with orokusaki that although there is a better approach to that described in the original ticket (i.e. the use of form.save(commit=False)), there is, nonetheless, a problem in the way that form.is_valid() and model validation validates unique_together. Currently, model validation ignores all unique_together tuples where *any* of the referenced fields are excluded; instead, model validation should only ignore those unique_together tuples where *all* of the referenced fields are excluded.

I am reopening this ticket with the purpose of marking it as a duplicate of #13091.