Opened 18 years ago
Closed 12 years ago
#5335 closed New feature (duplicate)
Add an append method to ErrorDict
| Reported by: | Owned by: | jkocherhans | |
|---|---|---|---|
| Component: | Forms | Version: | dev |
| Severity: | Normal | Keywords: | append errodict |
| Cc: | hv@…, simon@…, cahenan@…, miracle2k, Torsten Bronger, alasdair | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | yes |
| Easy pickings: | no | UI/UX: | no |
Description
Custom Validation: If you need to access other fields, you
need to overwrite form.clean(). If you want to have
the error message near a widget (not all), you need to
modify the _error dict of the form.
To make this more user friendly I wrote a small patch:
ErrorDict.append(fieldname, message)
Doc patch included.
Attachments (6)
Change History (39)
by , 18 years ago
| Attachment: | form_error_append.diff added |
|---|
comment:1 by , 18 years ago
| Resolution: | → invalid |
|---|---|
| Status: | new → closed |
comment:2 by , 18 years ago
| Resolution: | invalid |
|---|---|
| Status: | closed → reopened |
Thank you brosner, for looking at this patch. But I can't use clean_FIELD():
if clean_FIELD() requires to look at FIELD2, the dict self.cleaned_data does not contain
FIELD2. If you need the cleaned values of two fields for validation, you can not use clean_FIELD().
Nevertheless I want the error message to be near the field FIELD.
Here is how I use the patch:
def clean(self):
parent=self.cleaned_data.get("parent")
name=self.cleaned_data.get("name")
if ....:
# Needs Django Patch #5335
self.errors.append("name", u'...')
return self.cleaned_data
Of course I could do this without the patch, too:
errorlist=self.errors.get(name)
if errorlist==None:
errorlist=ErrorList()
self.errors[name]=errorlist
errorlist.append(message)
But that's too much code, I don't want to repeat. Maybe the documentation should
be updated, too.
comment:3 by , 18 years ago
| Needs tests: | set |
|---|---|
| Triage Stage: | Unreviewed → Design decision needed |
by , 18 years ago
| Attachment: | form_error_append2.diff added |
|---|
comment:5 by , 18 years ago
| Cc: | added |
|---|
comment:6 by , 18 years ago
| Cc: | added |
|---|
comment:7 by , 17 years ago
| Cc: | added |
|---|
comment:9 by , 17 years ago
Perhaps a better method would be to have the ErrorDict be a defaultdict, that way users can do form._errorsfield.append('error message') without worrying about whether or not field is in the dict or not.
comment:10 by , 17 years ago
| Keywords: | append errodict added |
|---|---|
| Summary: | newforms: Custom validation. → Add an append method to ErrorDict |
Attached patch implements ErrorDict as a defaultdict (can't inherit directly from defaultdict since it's new in 2.5 or 2.4, can't remember, but definitely not in 2.3). It would be helpful if someone could review the documentation part of the patch since I'm not a native english speaker.
comment:11 by , 17 years ago
The docs look ok by me, 2 small things though, instead of self.has_key(field) do field in self . Also there's no need to use super() for setitem since it isn't overode in the subclass, you can actually just use setattr(self, field, []).
comment:13 by , 17 years ago
There's actually a slight disconnect between the docs and the code, the docs say an ErrorList is created, but getitem adds a list([]), I think an ErrorList would be the correct behavior.
by , 17 years ago
| Attachment: | append-take3-r9698.diff added |
|---|
Fixed patch, create an ErrorList instead of a simple list
comment:14 by , 17 years ago
| milestone: | → 1.1 |
|---|
According to Version1.1Features seems to be in scope for 1.1?
comment:15 by , 17 years ago
| milestone: | 1.1 → 1.2 |
|---|
Bumping to 1.2. For the record, I have to do this all the time and I'd like to see something like this, but I'm not entirely sold on this way yet.
comment:16 by , 17 years ago
Definitely. Often views need to perform more involved steps *after* the form is validated; which may fail, and ultimately mean there was a problem with what was entered in the form.
There needs to be an easy, documented, encouraged way of appending errors to NON_FIELD_ERRORS as well as to specific fields; post-validation (ie from the view).
comment:17 by , 16 years ago
This approach is in conflict with #4752 ([6142]).
The method would either have to be on the Form class (not entirely insane). Another way how to propagate your error onto a field is using ComplexValidators present in model-validation branch, that way you don't have to worry about the internals of ErrorDict...
comment:18 by , 16 years ago
| Cc: | added |
|---|
comment:19 by , 16 years ago
| milestone: | 1.2 → 1.3 |
|---|---|
| Owner: | changed from to |
| Status: | reopened → new |
This a a feature, so I'm bumping it to 1.3. Sorry. I'm assigning it to myself it to increase the chances that I'll remember it for the next cycle.
comment:20 by , 15 years ago
| Triage Stage: | Design decision needed → Accepted |
|---|
comment:21 by , 15 years ago
| milestone: | 1.3 → 1.4 |
|---|---|
| Patch needs improvement: | set |
Looks like Joseph didn't remember :-( This is a feature, so will have to wait again. Patch also needs updating (especially to use unittest rather than doctests).
comment:22 by , 15 years ago
| Severity: | → Normal |
|---|---|
| Type: | → New feature |
comment:23 by , 14 years ago
| Easy pickings: | unset |
|---|---|
| UI/UX: | unset |
Thanks a lot for updating the patch. I showed it to Alex and he wondered if you could write a test that verifies that doing errors['foo'] without actually altering it doesn't cause any problem?
comment:24 by , 14 years ago
| Cc: | added |
|---|
comment:25 by , 14 years ago
Current patch (form_error_append_r16741.diff) will auto-create an ErrorList() object for a field's error list even if the including form has over-ridden the form's error class (https://docs.djangoproject.com/en/1.3/ref/forms/api/#customizing-the-error-list-format). If we are going to add this (and it would be nice) it needs to be done in a way that will use the form's custom error class if it has been defined. This case should also be tested, in a test similar to: https://code.djangoproject.com/browser/django/tags/releases/1.3.1/tests/regressiontests/forms/tests/error_messages.py#L199
comment:27 by , 12 years ago
The problem is that ErrorDict should be ErrorList agnostic. The latest patch hardcodes the ErrorList type which can be customized at the form level with the error_class argument of Form.__init__() .
The Form.add_errors() method discussed here is IMO a much better approach.
comment:29 by , 12 years ago
Looping through defaultdicts in templates can cause problems. See #16335. I believe the pull request 1481 would stop the following template snippet from working.
<ul>
{% for field, errors in form.errors.items %}
<li>{{ field }}: {{errors}}</li>
{% endford %}
</ul>
I realise it's a slightly artificial example, and that usually you would use {{ form.errors }}.
comment:31 by , 12 years ago
@alasdair: that looks like a nasty incompatibility with no obvious workaround.
The only reason I fixed this despite #20867 was so internally it wouldn't be needed to check if keys existed, it's clearly not worth breaking existing templates over this.
I think we should close this ticket as wontfix, especially now that another ticket addresses the main issue.
comment:32 by , 12 years ago
| Cc: | added |
|---|
comment:33 by , 12 years ago
| Resolution: | → duplicate |
|---|---|
| Status: | new → closed |
I agree that this is not the best way forwards. The ticket #20867 is now covering the Form.add_errors() approach. I'm going to close this ticket as a "duplicate" cos I think that's closest to what's happening.
You can use the clean_FIELD() method validation hooks to get errors messages near fields.