ValidationError for fields

282 views
Skip to first unread message

Simon Litchfield

unread,
Aug 19, 2013, 8:58:25 PM8/19/13
to django-d...@googlegroups.com
Lack of clean control over field-specific form errors is an issue that has been raised and discussed many times over the years, but still the solution seems pretty inadequate. We're even directing people hack around with _errors and making excuses for it in the documentation.

What about an optional kwarg on ValidationError? Eg, raise forms.ValidationError('This field is seriously not cool', field='myfield'). Current behaviour as-is.

Loic Bistuer

unread,
Aug 20, 2013, 12:11:44 AM8/20/13
to django-d...@googlegroups.com
There is a ticket with a PR to address the issue of targeting specific fields when raising errors; I'm awaiting feedback on what should be the documented API.

https://code.djangoproject.com/ticket/20867.

This patch enables two patterns:

- Raising ValidationError({'field': 'error'}) from `Form.clean()`.

- Calling Form.add_errors('field', 'error') from anywhere.

The former is actually something that existed for a long time; only it couldn't be used from `Form.clean()`. This pattern allows targeting specific fields from the `Model` layer (see #16986).

The later has been proposed by @akaariai and @mjtamlyn, it's easier to use for the simple cases and it's accessible from outside the form, from a view for example.

The current patch only documents the dict construct for `ValidationError` since `Form.add_errors` was a private method in my original patch; should both be documented or only `Form.add_errors`?

--
Loic

Aaron Merriam

unread,
Aug 20, 2013, 12:15:58 AM8/20/13
to django-d...@googlegroups.com
+1 on a better way to do field errors.

An alternate would be to implement a method that does this.

def field_error(self, name, error):
    self._errors = self._errors or ErrorDict()
    self._errors.setdefault(name, ErrorList())
    self._errors[name].append(error)

Having to raise a ValidationError would prevent you from creating multiple field errors from within `clean`. 

Loic Bistuer

unread,
Aug 20, 2013, 1:26:18 AM8/20/13
to django-d...@googlegroups.com
> An alternate would be to implement a method that does this.
>
> def field_error(self, name, error):
> self._errors = self._errors or ErrorDict()
> self._errors.setdefault(name, ErrorList())
> self._errors[name].append(error)

I also have a pending PR for that: https://code.djangoproject.com/ticket/5335.

So you could do `self._errors[name].append(error)` from within the form and `self.errors[name].append(error)` from outside.

> Having to raise a ValidationError would prevent you from creating multiple field errors from within `clean`.


Not necessarily, the `ValidationError` constructor accepts a variety of scenarios:

def clean(self):
errors = {}
if condition1:
errors['field1'] = ValidationError('message1', code='code1')
if condition2:
errors['field2'] = ValidationError('message2', code='code2')
raise ValidationError(errors)

Note that in the example above, `ValidationError('message1', code='code1')` can also be a simple string, but if https://github.com/django/django/pull/1483 (yet another PR) goes in, passing an error code will become quite useful.

--
Loic

Anssi Kääriäinen

unread,
Aug 20, 2013, 2:07:33 AM8/20/13
to django-d...@googlegroups.com, Loic Bistuer
I really like the Form.add_error(field, error_message) approach. I have
used it as custom addition in some of my projects, and it just feels
correct.

- Anssi

Marc Tamlyn

unread,
Aug 20, 2013, 4:35:54 AM8/20/13
to django-d...@googlegroups.com
Yeah I'm definitely on the "document add_error first, ValidationError(dict) as a more powerful alternative". The second option is necessary to do clever things directly from Model.clean()

Marc




--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.

Simon Litchfield

unread,
Aug 20, 2013, 8:31:07 PM8/20/13
to django-d...@googlegroups.com
An improvement for sure. Any reason it can't be a little more pythonic, ie using optional kwargs etc?

My only concern is in having two ways of achieving the same thing. If the latter is simpler and more flexible, does this place our entire ValidationError approach to handling form and model errors in question? Hmmm. Maybe we can come back to that later :-/

Wim Feijen

unread,
Aug 21, 2013, 5:29:08 PM8/21/13
to django-d...@googlegroups.com
Hi Loic,

That's nice! It looks very powerful, saving lines of code and being more clear. 

How about:

form.add_errors(dictionary)

? Because then we can either add one or more errors at the same time.

Wim

Loic Bistuer

unread,
Aug 21, 2013, 11:01:26 PM8/21/13
to django-d...@googlegroups.com
On Aug 22, 2013, at 4:29 AM, Wim Feijen <w...@go2people.nl> wrote:

> How about:
>
> form.add_errors(dictionary)

That's actually supported, although you need to nullify `field` explicitly: `form.add_errors(None, dictionary)`.

It's mostly used internally; I'm not sure it's really useful to document, since most likely you will need to construct that dictionary by iterating through a series of conditions and you could call `add_errors` each time.

I did a quick poll on IRC and it looks like the singular name `Form.add_error(self, field, error)` is more popular because that should be the most common case, although `error` will still accept all kind of constructs: single error, list of errors, dict of errors, simple strings and `ValidationError` instances.

On Aug 21, 2013, at 7:31 AM, Simon Litchfield <litch...@gmail.com> wrote:

> An improvement for sure. Any reason it can't be a little more pythonic, ie using optional kwargs etc?

I'm not too sure what you mean, could you paste some pseudocode?

> My only concern is in having two ways of achieving the same thing. If the latter is simpler and more flexible, does this place our entire ValidationError approach to handling form and model errors in question? Hmmm. Maybe we can come back to that later :-/

The two approaches complement each other.

By design (loose coupling) you can't access `Form._errors` through most of the validation process: `Field.clean()`, `Validator` or validation at the `Model` layer for `ModelForm`. Raising `ValidationError` is your only option here.

`Form.add_error()` is just a more elegant solution to fiddling with `Form._errors`. It comes in handy when you want to validate "after the fact", like adding errors from a view or adding errors to specific fields after their dedicated validation cycle is done (in other words, from `Form.clean()`). Raising an exception was never an option here hence why we documented all the shortcomings of modifying `_errors` directly.

The only overlap is `Form.clean_<fieldname>()`, but even here `Form.add_error()` wouldn't be too practical compared to raising an exception since you would have to manually provide the field name.

Also, as I mentioned in a previous post, if https://github.com/django/django/pull/1483 pass, it will become rewarding to provide metadata to errors (codes, and params), and the carrier for such metadata is `ValidationError`. Ideally you would do `self.add_error('fieldname', ValidationError('message', code='code'))`; so if you are in `Form.clean_<fieldname>()` you might as well raise the exception...

--
Loic

Reply all
Reply to author
Forward
0 new messages