#3406 closed (fixed)
[patch] newforms: choices checking should unicode choices list before comparing to a value
| Reported by: | Owned by: | Adrian Holovaty | |
|---|---|---|---|
| Component: | Forms | Version: | dev |
| Severity: | Keywords: | unicode unicode-branch | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | yes | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Hello everyone
There is a mistake in django/newforms/fields.py:
when one uses ChoiceField to select a value from predefined choices it tries to validate selected choice against a list of string values, should be a list of unicode values as it's done in MultipleChoiceField
Without the patch one always gets "Select a valid choice. %s is not one of the available choices." ValidationError.
Patch works with my previous #3401
Attachments (1)
Change History (17)
by , 18 years ago
| Attachment: | unicode-choices-checking.patch added |
|---|
comment:1 by , 18 years ago
| Keywords: | unicode added |
|---|---|
| Needs tests: | set |
| Patch needs improvement: | set |
| Triage Stage: | Unreviewed → Design decision needed |
Oh, yet another unicode bug ;-)
Hmm, since newforms is unicode country, I guess this is the wrong fix. formfield.choices should contain unicode strings, not bytestrings. IMHO, the bug is rather in formfield() (probably ForeignKey.formfield() ), it should decode the options to unicode during formfield construction, using settings.DEFAULT_CHARSET as encoding. Can you please specify how you created the ChoiceField?
A unit test would be great (it's needed anyway, no matter how this turns out).
But I leave this to Adrian --> decision needed.
comment:2 by , 18 years ago
Michael Radziej, you seem misunderstood it.
clean() always converts value to unicode and then unicode value is compared against list of choices and if this list is not in unicode, this check fails. My patch converts choices list to unicode if needs just for comparing because unicode should be compared against unicode, not bytestrings.
And again: take a look a few lines below my patch, there is absolutely the same code for MultipleChoiceField and it does exactly the same as my patch does
comment:3 by , 18 years ago
Detailed.
line 354: value = smart_unicode(value)
the value is in unicode after this line
line 357 (original): valid_values = set([str(k) for k, v in self.choices])
here we create the list of available choices.
line 358: if value not in valid_values:
any ideas how this may work if valid_values contains a list of bytestrings ?
And finally, compare my new line 357: valid_values = set([smart_unicode(k) for k, v in self.choices]) to original line 383: valid_values = set([smart_unicode(k) for k, v in self.choices]) :)
comment:4 by , 18 years ago
| Triage Stage: | Design decision needed → Unreviewed |
|---|
I think I understood you perfectly. I argue that formfield.choices should be a list of tuples of unicode strings. Well, you can do it this way or either, it's Adrian's decisions, anyway.
But leaving this aside, I tried to reproduce your bug, but I cannot:
>>> u"bla" == "bla" True >>> u"a" in set(["a"]) True
Then I try with newforms:
>>> from django.newforms import *
>>> f=ChoiceField(choices=[("a","a"),("b","b")])
>>> f.clean(u"a")
u'a'
>>> f.clean(u"c")
django.newforms.util.ValidationError Traceback (most recent call last)
/home/mir/src/kundebunt/<console>
/home/mir/lib/python/django/newforms/fields.py in clean(self, value)
357 valid_values = set([str(k) for k, v in self.choices])
358 if value not in valid_values:
--> 359 raise ValidationError(gettext(u'Select a valid choice. %s is not one of the available choices.') % value)
360 return value
361
ValidationError: [u'Select a valid choice. c is not one of the available choices.']
So, I this works for me. Without converting the choices to unicode. Can you show me a way how to get this bug?
comment:5 by , 18 years ago
Then try this:
>>> u"превед" == "превед" Traceback (most recent call last): File "<stdin>", line 1, in ? UnicodeDecodeError: 'ascii' codec can't decode byte 0xef in position 0: ordinal not in range(128) >>> u"превед" in ["превед"] Traceback (most recent call last): File "<stdin>", line 1, in ? UnicodeDecodeError: 'ascii' codec can't decode byte 0xef in position 0: ordinal not in range(128)
and the code that fails:
class InvoiceBody(models.Model):
....
UNITS = (
('шт.', 'шт.'),
('мес.', 'мес.'),
)
units = models.CharField(maxlength=10, choices=UNITS)
and then try to validate a form built using form_for_model()
And btw, you suggest to define choices as unicode strings. Then this line (original) would do:
>>> str(u"превед") Traceback (most recent call last): File "<stdin>", line 1, in ? UnicodeEncodeError: 'ascii' codec can't encode characters in position 0-5: ordinal not in range(128)
Doesn't work either.
comment:7 by , 18 years ago
| Triage Stage: | Design decision needed → Accepted |
|---|
I meant 'Accepted'! Probably all this unicode stuff should be written by Russians ;-)
comment:9 by , 18 years ago
Michael, if Adrian won't merge this patch to svn without tests until tomorrow (local time) I'd add tests too.
But I am happy anyway that you have changed your mind about all my work :)
follow-up: 11 comment:10 by , 18 years ago
Umm there is a problem with tests: because it touches local (national) environments, it seems I need a new test directory similar to tests/modeltests/model_forms/models.py but with sources in coding: utf-8 or windows-1251. Is that ok ?
comment:11 by , 18 years ago
I really don't know, better ask on the list. Perhaps you can also use unicode escapes like '\u1234', but better discuss this on the list.
comment:12 by , 18 years ago
comment:13 by , 18 years ago
| Keywords: | unicode-branch added |
|---|
This bug is fixed in the above commit -- [5236] -- to the unicode branch. I'll close this ticket once that branch is merged back into trunk.
comment:14 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:15 by , 17 years ago
seduction Here's my post on how to get laid in the next month or two. This post does not maximize your chances of "getting really good at pickup" - in fact, it does the opposite.
quick fix