Django

Code

Ticket #3457 (closed: fixed)

Opened 2 years ago

Last modified 11 months ago

Allow overriding of error message for each newforms Field type

Reported by: anonymous Assigned to: nobody
Milestone: Component: Forms
Version: SVN Keywords:
Cc: egmanoj@gmail.com, jm.bugtracking@gmail.com, densetsu.no.ero.sennin@gmail.com, leo.soto@gmail.com, niels.busch@gmail.com Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 1

Description

Right now, the only field type that allows custom error messages is RegexField?. For those of us that cannot display errors next to their respective field (for whatever reason), having 6 "This field is required." messages in a row just confuses users.

I propose adding error_text to the base Field definition. Adding a simple

self.error_text = error_text or u'This field is required.'

The only design decision I see that needs to be made is how to handle fields that have multiple error messages, e.g., the min/max errors on IntegerField?. Perhaps arguments would work there, too: max_error_text, min_error_text.

Attachments

newforms_error_messages.patch (22.8 kB) - added by SmileyChris on 09/11/07 21:57:47.
newforms_error_messages.2.patch (22.8 kB) - added by SmileyChris on 09/11/07 22:22:36.
fixed EmailField behaviour and made sure current tests still pass (ErrorList.__repr__ now uses force_unicode)
newforms_error_messages.3.patch (40.6 kB) - added by SmileyChris on 09/12/07 18:41:48.
with full tests and docs
newforms_error_messages.4.patch (42.7 kB) - added by gwilson on 09/14/07 12:05:40.
newforms_error_messages.5.patch (41.2 kB) - added by gwilson on 09/14/07 12:07:35.
removed my changes from testcases.py I made during testing

Change History

02/11/07 21:17:30 changed by adrian

  • needs_better_patch changed.
  • stage changed from Unreviewed to Accepted.
  • needs_tests changed.
  • needs_docs changed.

Sure -- good idea. Could you provide a patch, along with unit tests that confirm this behavior for every field?

02/19/07 20:50:05 changed by adrian

  • summary changed from newforms needs error_text for fields to Allow overriding of error message for each newforms Field type.

02/22/07 11:46:04 changed by Manoj Govindan <egmanoj@gmail.com>

I tried out some code changes (and tests) for implementing a patch. One key question I would like to see answered is how this change should affect the behaviour of RegexField? and its subclasses, say URLField. The tests indicate that these classes expect two kind of error messages depending on the nature of the error.

Consider the first kind of error message, raised when the value of the field doesn't fit a given pattern.

>>> f = URLField()
>>> f.clean('foo')
Traceback (most recent call last):
...
ValidationError: [u'Enter a valid URL.']

The error message displayed is the one used by URLField to override the default provided by RegexField?.

Now consider the second scenario where the field is required but turns up empty.

>>> f.clean('')
Traceback (most recent call last):
...
ValidationError: [u'This field is required.']

The error raised in this case is different (the default error defined in the base 'Field' class is used)

If a URLField object is created with a custom error_message, should it still fall back to the default message if value turns out to be empty? Likewise how should a CharField? object with a custom error_message behave in such a case?

One possible solution would be to insist that custom error messages take precedence and hence will be always used if present. I don't mind this but I can see why others could have problems with this. Another approach would be to "collect" errors applicable to each field and display them together (a la Rails model validation).

The latter implementation would result in something similar to this:

>>> f.clean('')
Traceback (most recent call last):
...
ValidationError: [u'This field is required.', u'Enter a valid URL.']

02/22/07 11:46:21 changed by Manoj Govindan <egmanoj@gmail.com>

  • cc set to egmanoj@gmail.com.

04/09/07 07:54:44 changed by anonymous

  • cc changed from egmanoj@gmail.com to egmanoj@gmail.com, jm.bugtracking@gmail.com.

05/03/07 21:24:56 changed by anonymous

See also #4063, which proposes a more elegant solution.

05/14/07 09:53:38 changed by Gary Wilson <gary.wilson@gmail.com>

  • stage changed from Accepted to Design decision needed.

#4063 marked as a duplicate.

Need to decide what to do with Fields that can raise more than one Error.

05/14/07 15:58:00 changed by ville.anttonen@gmail.com

I suggest to use dictionary when multiple errors could occur.

09/11/07 21:57:47 changed by SmileyChris

  • attachment newforms_error_messages.patch added.

09/11/07 21:59:02 changed by SmileyChris

  • needs_docs set to 1.
  • needs_tests set to 1.
  • stage changed from Design decision needed to Accepted.

Ok, here you go: a working patch. Just needs documentation and tests...

09/11/07 21:59:17 changed by SmileyChris

  • has_patch set to 1.

09/11/07 22:22:36 changed by SmileyChris

  • attachment newforms_error_messages.2.patch added.

fixed EmailField behaviour and made sure current tests still pass (ErrorList.__repr__ now uses force_unicode)

09/11/07 22:30:04 changed by SmileyChris

For a quick summary of how it works:

  1. each Field instance gets an error_messages dictionary built from the default_error_messages in it's class (and base classes, recursively)
  1. a user can pass in their own error_messages dictionary which .update()s the error_messages dictionary for that instance
>>> from django.newforms import *
>>> d = DecimalField(error_messages={'invalid':'Mr T says, "Enter a valid number, foo!"'})
>>> d.clean('jibberjabber')

Traceback (most recent call last):
  File "<pyshell#72>", line 1, in -toplevel-
    d.clean('jibberjabber')
  File "[...]\django\newforms\fields.py", line 237, in clean
    raise ValidationError(self.error_messages['invalid'])
ValidationError: [u'Mr T says, "Enter a valid number, foo!"']

09/12/07 05:22:58 changed by Densetsu no Ero-sennin <densetsu.no.ero.sennin@gmail.com>

  • cc changed from egmanoj@gmail.com, jm.bugtracking@gmail.com to egmanoj@gmail.com, jm.bugtracking@gmail.com, densetsu.no.ero.sennin@gmail.com.

09/12/07 18:41:48 changed by SmileyChris

  • attachment newforms_error_messages.3.patch added.

with full tests and docs

09/12/07 18:42:34 changed by SmileyChris

  • needs_docs deleted.
  • needs_tests deleted.
  • stage changed from Accepted to Ready for checkin.

In reality, noone else was going to do docs or tests. So here they are.

09/13/07 23:17:28 changed by anonymous

  • cc changed from egmanoj@gmail.com, jm.bugtracking@gmail.com, densetsu.no.ero.sennin@gmail.com to egmanoj@gmail.com, jm.bugtracking@gmail.com, densetsu.no.ero.sennin@gmail.com, leo.soto@gmail.com.

09/14/07 10:30:59 changed by gwilson

  • owner changed from nobody to gwilson.
  • status changed from new to assigned.

09/14/07 12:05:15 changed by gwilson

  • needs_better_patch set to 1.
  • stage changed from Ready for checkin to Accepted.

brought patch up to date, but tests still failing for python 2.3 due to non-evaluation of lazy objects when printing out ErrorList?.

09/14/07 12:05:40 changed by gwilson

  • attachment newforms_error_messages.4.patch added.

09/14/07 12:07:35 changed by gwilson

  • attachment newforms_error_messages.5.patch added.

removed my changes from testcases.py I made during testing

09/14/07 15:28:55 changed by gwilson

  • owner changed from gwilson to nobody.
  • status changed from assigned to new.

10/19/07 17:33:20 changed by niels

  • cc changed from egmanoj@gmail.com, jm.bugtracking@gmail.com, densetsu.no.ero.sennin@gmail.com, leo.soto@gmail.com to egmanoj@gmail.com, jm.bugtracking@gmail.com, densetsu.no.ero.sennin@gmail.com, leo.soto@gmail.com, niels.busch@gmail.com.

10/28/07 00:40:26 changed by gwilson

  • status changed from new to closed.
  • resolution set to fixed.

(In [6625]) Fixed #3457 -- Allow overridding of error messages for newforms Fields.

10/28/07 00:43:32 changed by gwilson

Sorry that I forgot to mention it in the commit message, but many thanks to SmileyChris for the bulk of the work done on this patch.


Add/Change #3457 (Allow overriding of error message for each newforms Field type)




Change Properties
Action