Opened 3 months ago

Closed 2 days ago

#36618 closed Cleanup/optimization (fixed)

BaseForm.add_error() misleading exception message when both field and dict provided

Reported by: Chris Brand Owned by: Nilesh Pahari
Component: Forms Version: 5.2
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
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 (14)

comment:1 by Jacob Walls, 3 months ago

Component: UncategorizedForms
Easy pickings: set
Summary: Forms.add_error() misleading exception messageBaseForm.add_error() misleading exception message when both field and dict provided
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

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.

comment:2 by The5cheduler, 3 months ago

Component: FormsUncategorized
Easy pickings: unset
Summary: BaseForm.add_error() misleading exception message when both field and dict providedForms.add_error() misleading exception message
Triage Stage: AcceptedUnreviewed
Type: Cleanup/optimizationUncategorized

comment:3 by The5cheduler, 3 months ago

Owner: set to The5cheduler
Status: newassigned

comment:4 by The5cheduler, 3 months ago

Component: UncategorizedTesting framework
Needs tests: set

comment:5 by Jacob Walls, 3 months ago

Component: Testing frameworkForms
Easy pickings: set
Needs tests: unset
Patch needs improvement: set
Summary: Forms.add_error() misleading exception messageBaseForm.add_error() misleading exception message when both field and dict provided
Type: UncategorizedCleanup/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 Sarah Boyce, 3 months ago

Triage Stage: UnreviewedAccepted

comment:7 by The5cheduler, 3 months ago

Thank you @jacobtylerwalls
I have adjusted the files to update the messaging of the error without updating underlying behavior of the method.

comment:8 by Nilesh Pahari, 12 days ago

Hi! Since this has been open for a while, would it be okay if I take this on?

Last edited 12 days ago by Nilesh Pahari (previous) (diff)

comment:9 by Nilesh Pahari, 9 days ago

Owner: changed from The5cheduler to Nilesh Pahari

comment:10 by Nilesh Pahari, 3 days ago

Patch needs improvement: unset

comment:12 by Nilesh Pahari, 3 days ago

Has patch: set

comment:13 by Jacob Walls, 2 days ago

Triage Stage: AcceptedReady for checkin

comment:14 by Jacob Walls <jacobtylerwalls@…>, 2 days ago

Resolution: fixed
Status: assignedclosed

In e49e14f:

Fixed #36618 -- Corrected error message in BaseForm.add_error().

The error message now correctly states that the error argument
is a dictionary.

Note: See TracTickets for help on using tickets.
Back to Top