#27161 closed Bug (fixed)
TypedChoiceField fails to validate properly when used with ArrayField
Reported by: | Roman Karpovich | Owned by: | Rômulo Rosa Furtado |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | TypedChoiceField |
Cc: | jeroen@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Example model:
from django.contrib.postgres.fields import ArrayField class ArrayFieldTestModel(models.Model): CHOICES = map(lambda x: (x, str(x)), range(10)) test_field = ArrayField(models.IntegerField(choices=CHOICES), blank=True, null=True)
On form saving we'll got validation error for valid choices:
This is caused by validation ordering:
- TypedChoiceField clean method called
- Field clean method called (super)
- to_python method called, which should transform value to correct, but it doesn't.
- TypedChoiceField validate called, which raise ValidationError: no such choice. He is trying to find string value in integers array and can't do this.
- after super clean method, TypedChoiceField is trying to coerce value (it's too late, error was raised before this).
this can be easily fixed by moving _coerce call into to_python
.
Attachments (1)
Change History (12)
by , 8 years ago
Attachment: | Running_coerce_in_TypedChoiceField_to_python_method.patch added |
---|
comment:1 by , 8 years ago
I've already struggled with this in the past. Did you check if tests are passing with your proposal?
comment:2 by , 8 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
Summary: | TypedChoiceField validation bug → TypedChoiceField fails to validate properly when used with ArrayField |
Triage Stage: | Unreviewed → Accepted |
I observed one test failure with the patch: test_typedchoicefield_special_coerce
.
If it's possible to add a test that's not dependent on contrib.postgres
, that would be ideal. Of course, we could add that test too.
comment:3 by , 8 years ago
It may be that the proper resolution is #27704. Roman, you might be interested to test the patch there.
comment:4 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Triage Stage: | Accepted → Unreviewed |
comment:5 by , 8 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:6 by , 8 years ago
Cc: | added |
---|---|
Needs tests: | unset |
Patch needs improvement: | unset |
The problem with moving _coerce
into to_python
is that you can coerce to a value not present in choices
and clean
calls to_python
before checking whether the value is in choices
. I think the best way to fix this is having SimpleArrayField's to_python
call clean
on each array item to get the coerced value instead of calling to_python
. Pull request with that fix and test case: https://github.com/django/django/pull/8358
comment:7 by , 8 years ago
Patch needs improvement: | set |
---|
comment:8 by , 8 years ago
Patch needs improvement: | unset |
---|
An alternate PR adds ArrayField.clean()
that calls base_field.clean()
.
comment:11 by , 7 years ago
No. Per our supported versions policy, 1.11 is only receiving data loss and security fixes.
patch for ticket