Opened 3 weeks ago
Last modified 3 weeks ago
#36618 assigned Cleanup/optimization
BaseForm.add_error() misleading exception message when both field and dict provided
Reported by: | Chris Brand | Owned by: | The5cheduler |
---|---|---|---|
Component: | Forms | Version: | 5.2 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | yes | UI/UX: | no |
Description
This code creates a form and attempts to add an error for a single field to it:
from django import forms from django.core.exceptions import ValidationError class FooForm(forms.Form): the_field = forms.CharField(max_length=100) exc = ValidationError({'the_field': 'Something is wrong with the field'}) form = FooForm() form.add_error('the_field', exc)
It results in the following error message:
TypeError: The argument `field` must be `None` when the `error` argument contains errors for multiple fields.
The reference to "errors for multiple fields" is just wrong - the error in question is for a single field.
For background, in my case the ValidationError is raised by my custom Model.clean(). What I really want to do it to map it from the Model field to the form field (which in general may not have the same name). The error message was quite confusing.
Change History (7)
comment:1 by , 3 weeks ago
Component: | Uncategorized → Forms |
---|---|
Easy pickings: | set |
Summary: | Forms.add_error() misleading exception message → BaseForm.add_error() misleading exception message when both field and dict provided |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 3 weeks ago
Component: | Forms → Uncategorized |
---|---|
Easy pickings: | unset |
Summary: | BaseForm.add_error() misleading exception message when both field and dict provided → Forms.add_error() misleading exception message |
Triage Stage: | Accepted → Unreviewed |
Type: | Cleanup/optimization → Uncategorized |
Raised the PR Here:
comment:3 by , 3 weeks ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:4 by , 3 weeks ago
Component: | Uncategorized → Testing framework |
---|---|
Needs tests: | set |
comment:5 by , 3 weeks ago
Component: | Testing framework → Forms |
---|---|
Easy pickings: | set |
Needs tests: | unset |
Patch needs improvement: | set |
Summary: | Forms.add_error() misleading exception message → BaseForm.add_error() misleading exception message when both field and dict provided |
Type: | Uncategorized → Cleanup/optimization |
Thanks for volunteering. I think my triage came in while you were working on a PR, so be sure to read the triage decision: a PR to adjust the exception message is welcome, but it wouldn't be in scope to change the underlying behavior.
comment:6 by , 3 weeks ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:7 by , 3 weeks ago
Thank you @jacobtylerwalls
I have adjusted the files to update the messaging of the error without updating underlying behavior of the method.
Good spot, happy to take a PR replacing the reference to "multiple fields" with a mention of a dictionary instead. Incidentally this condition isn't tested, so we'll benefit from the addition of a test.