On Feb 18, 8:03 pm, Honza Král <honza.k...@gmail.com> wrote:
> Hi,
> see inline text.
>
> On Wed, Feb 18, 2009 at 1:28 PM, mrts <m...@mrts.pri.ee> wrote:
>
> > The last unsolved model-validation design issue is the error message
> > protocol (my work on fields is waiting on it). Let me present the
> > approach that looks sane to me.
>
> > The old Field.default_error_messages dict and corresponding
> > logic is no longer required (and removed in my branch) as default
> > error messages live in core/validators now.
>
> What if I want to use the same error_messages on multiple instances of
> a given field?
>
> Now I would subclass the FormField and override it's default_messages,
> with your approach it would require overriding __init__. I am not
> saying that that is a bad thing, just that it still has a reason.
Yup, design decision needed:
is the need for class-wide overrides of error messages common enough
to justify leaving it as-is (considering that this can still be done
via __init__)?
> Besides not all validation will be done inside validators, some will
> remain on the Field classes.
As of now, I've managed to weed out all validation from fields into
validators. The result is not entirely stable yet and it brings in
minor backwards-compatibility issues, so it remains to be seen if this
is entirely viable.
> > In field construction, the custom error logic is plain:
>
> > CharFied.__init__(self, ..., error_messages={}):
> > ...
> > self.error_messages = error_messages
>
> > And it is used as follows:
>
> > class Form(...):
> > ...
> > def full_clean(self):
> > ...
> > for name, field in self.fields.items():
> > try:
> > ...
> > # clean() calls to_python() and validate(),
> > # raising ValidationErrors
> > value = field.clean(value)
> > ...
> > except ValidationError, e:
> > e.custom_messages = field.error_messages
> > self._errors[name] = self.error_class(e.messages)
>
> > The e.custom_messages = field.error_messages binds custom { code:
> > message } overrides to the ValidationError.
> > e.messages is a property that returns the list of error messages.
>
> > In reality, ValidationError contains only a single error message. The
> > list is required for aggregating several errors from the validator
> > collection associated with a field. However, as of now, the
> > ValidationError class tries to be both a single value and a
> > collection, resulting in quite a mess.
>
> > This is a classic case of the Composite pattern (http://
> > en.wikipedia.org/wiki/Composite_pattern ), so I propose that
> > * ValidationErrors remain single values
> > * core/validators gets an additional ValidationErrorComposite class
> > that derives from ValidationError and has the same interface
>
> What if then validators want to raise multiple messages? Of FormFields
> for that matter?
> for example:
>
> "Must be greated than 1000."
> "If field Y is supplied, X must be greater than Y."
> "I don't like your shoes!"
In core/validators there are none that need to raise multiple error
messages.
> Would they then raise ValidationErrorComposite instead? So you just
> pushed the check to a different layer. I agree that it makes more
> sense in it's new location, but the distinction still has to be made.
Yes, if that need arises, they can throw a ValidationErrorComposite.
> And how would ValidationErrorComposite.add(exception) work then? It
> would have to check the exception type to know whether to append or
> extend it's message list.
I see no problem there.
def add(e):
if isinstance(e, ValidationErrorComposite):
add the collection of ValidationErrors to self.errors
else:
add the single e to self.errors
> I agree that handling of different datat types (string, list or dict)
> in ValidationError isn't pretty, but it IMHO greatly simplifies it's
> usage and handling.
Let's just leave it to core devs to decide.
> > It will be used as follows (example for form field, model field
> > similar):
>
> > class Field(...):
> > ...
> > def clean(self, value, all_values={}):
> > if value is not None:
> > value = self.to_python(value)
> > self.validate(value, all_values)
> > return value
>
> > def validate(self, value, all_values={}, form_instance=None):
>
> unrelated to custom messages - you cannot pass all_values to field for
> validation, because at the time field validation is called, not all
> fields have been cleaned. That's why validator handling is done in
> (form/model).clean.
Ah, sorry, there was just an unintentional omission in my example. It
should have obviously been as follows:
class Form(...):
...
def full_clean(self):
...
for name, field in self.fields.items():
try:
...
# self.cleaned_data was missing
value = field.clean(value, self.cleaned_data)
...
except ValidationError, e:
...
But regarding the concern that not all fields have been cleaned --
AFAIK this has always been the constraint: you can only refer to
fields declared before the current one while using cleaned_data in
validation.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---
