Opened 17 years ago
Closed 11 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 , 17 years ago
Attachment: | form_error_append.diff added |
---|
comment:1 by , 17 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 17 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 , 17 years ago
Needs tests: | set |
---|---|
Triage Stage: | Unreviewed → Design decision needed |
by , 17 years ago
Attachment: | form_error_append2.diff added |
---|
comment:5 by , 17 years ago
Cc: | added |
---|
comment:6 by , 17 years ago
Cc: | added |
---|
comment:7 by , 16 years ago
Cc: | added |
---|
comment:9 by , 16 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 , 16 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 , 16 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 , 16 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 , 16 years ago
Attachment: | append-take3-r9698.diff added |
---|
Fixed patch, create an ErrorList
instead of a simple list
comment:14 by , 16 years ago
milestone: | → 1.1 |
---|
According to Version1.1Features seems to be in scope for 1.1?
comment:15 by , 16 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 , 16 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 , 15 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 , 15 years ago
Cc: | added |
---|
comment:19 by , 15 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 , 14 years ago
Triage Stage: | Design decision needed → Accepted |
---|
comment:21 by , 14 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 , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:23 by , 13 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 errorsfoo without actually altering it doesn't cause any problem?
comment:24 by , 13 years ago
Cc: | added |
---|
comment:25 by , 13 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 , 11 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 , 11 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> {% endfor %} </ul>
I realise it's a slightly artificial example, and that usually you would use {{ form.errors }}
.
comment:31 by , 11 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 , 11 years ago
Cc: | added |
---|
comment:33 by , 11 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.