Opened 8 years ago

Closed 7 years ago

Last modified 6 years ago

#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:
https://snag.gy/DGNVtR.jpg

This is caused by validation ordering:

  1. TypedChoiceField clean method called
  2. Field clean method called (super)
  3. to_python method called, which should transform value to correct, but it doesn't.
  4. TypedChoiceField validate called, which raise ValidationError: no such choice. He is trying to find string value in integers array and can't do this.
  5. 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)

Running_coerce_in_TypedChoiceField_to_python_method.patch (630 bytes ) - added by Roman Karpovich 8 years ago.
patch for ticket

Download all attachments as: .zip

Change History (12)

by Roman Karpovich, 8 years ago

patch for ticket

comment:1 by Claude Paroz, 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 Tim Graham, 8 years ago

Needs tests: set
Patch needs improvement: set
Summary: TypedChoiceField validation bugTypedChoiceField fails to validate properly when used with ArrayField
Triage Stage: UnreviewedAccepted

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 Claude Paroz, 7 years ago

It may be that the proper resolution is #27704. Roman, you might be interested to test the patch there.

comment:4 by Rômulo Rosa Furtado, 7 years ago

Owner: changed from nobody to Rômulo Rosa Furtado
Status: newassigned
Triage Stage: AcceptedUnreviewed

comment:5 by Rômulo Rosa Furtado, 7 years ago

Triage Stage: UnreviewedAccepted

comment:6 by Jeroen Dekkers, 7 years ago

Cc: jeroen@… 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 Tim Graham, 7 years ago

Patch needs improvement: set

comment:8 by Tim Graham, 7 years ago

Patch needs improvement: unset

An alternate PR adds ArrayField.clean() that calls base_field.clean().

comment:9 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In 9dd2443:

Fixed #27161 -- Fixed form validation when an ArrayField's base_field has choices.

comment:10 by José Jorge Lorenzo Vila, 6 years ago

Any plan to port this to django 1.X? Thanks

comment:11 by Tim Graham, 6 years ago

No. Per our supported versions policy, 1.11 is only receiving data loss and security fixes.

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