BaseForms validation and ValidationError's params

[cgenie@…]

In forms/forms.py the _clean_fields function of BaseForm waits for ValidationError exception and inserts e.messages to self._errors.
The problem is that ValidationError supports quite useful things, like params, understanding dict arguments, etc. Since forms/forms.py takes only e.messages, there's no way to access later these parameters.
Would this make a reasonable request for improvement of BaseForms: instead of using e.messages, wrap the returned exception into some object, whose unicode method returns e.messages (to preserve compatibility), with additional properties taken from ValidationError.params?
This would allow for fine-grained error reporting. Not only would we have error messages reported per field, but could also specify additional parameters in the error message.

[Anssi Kääriäinen]

I don't think __unicode__ is enough. The problem is that one can reasonably except the errors to be strings.

I am marking this as wontfix as I don't see a clean way to do this. If we got to do this from clean slate the addition would make IMO sense. So, if you can figure out a clean way to do this, then reopen this.

[ko.szymanski@…]

It can be done in a clean way.
You can do it by small rewrite of ErrorList (form Error classes in general). It have to accept additional param on init, the params from ValidationError or even whole Exception (for custom exceptions handling). Then you can change the form behaviour by passing custom error_class param to your Form, and handle this additional param in your custom ErrorList.

But it's not backward compatible for old custom classes. It could be if you would create new base class for ErrorList and check it by instanceof, keeping old behaviour too.

[Anssi Kääriäinen]

hmmh, we have documented this:

            self._errors["subject"] = self.error_class([msg])

​https://docs.djangoproject.com/en/dev/ref/forms/validation/

It seems really hard to make that code still work, yet have the ValidationError available.

We could probably add some hacks and make the ValidationError available from somewhere if it happens to be present at all. But, this needs a *really* strong use case to be worth the added complexity. I still think wontfix is the way to go.

[ko.szymanski@…]

I can think of some nice usecases.
You have some custom FormFields, with custom ValidationErrors.
Then when you implement some custom error handling for I guess an ajax calls and you wan't some nice looking errors to be returned by your form, like:

 ['field': {'type': 'validation', 'importants': 'low', 'descritption': 'Error msg', (some_other_params from exception...)}]

You can't do it without ValidationError or ValidationError.params.

It's not so big hack, just pass some additional info:

self._errors[name] = self.error_class(e.messages)

Would be like:

self._errors[name] = self.error_class(e.messages, e.code, e.params)

#and smth like this for ErrorList
class ErrorList(list, StrAndUnicode):
    def __init__(self, collection, code=None, params=None):
    	list.__init__(self, collection)

I leave decision if it's worth or not in your hands. Thanks for your time.

[Carl Meyer]

I think a more reasonable change here would be to add a documented public add_error API on form classes, and have _clean_fields call that instead of doing self._errors[name] = self.error_class(e.messages) (which would be the default implementation of add_error.

It surprises me that we have all that documentation on stuffing things into form._errors directly; it would be a better if we changed that documentation to use a real API. And the side benefit would be that by overriding add_error and specifying a custom error_class, you could do everything that's been discussed here, without having to make ugly changes in core Django with potential backwards-compatibility implications.

I'm going to re-close this as wontfix (because I am also -1 on the change as it has been presented in this ticket) rather than totally hacking up the title and description to reflect my alternative approach. If anyone following this ticket is motivated to make the change as I've described it, please open a new ticket for that and link it from here for posterity's sake.