#36201 closed Cleanup/optimization (fixed)

ModelChoiceField/ModelMultipleChoiceField.clean() should catch ValidationError raised by the queryset operations

Reported by: Tim Graham Owned by: JaeHyuckSa
Component: Forms Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As seen in cdc25ac4747bf5a6cdc2e70461c2d43c54529d35, ValueError and TypeError are caught by the queryset operations in ModelChoiceField.clean() and ModelMultipleChoiceField.clean(), however, these exceptions are particular to ​IntegerFielld.get_prep_value(). A more common implementation of get_prep_value() delegates to to_python() which is supposed to raise ValidationError for invalid values (​as seen in UUIDField). Thus, ValidationError should also be caught so that the ​invalid choice error is raised rather than the "invalid" error of the model field.

Aside: I discovered this while implementing ObjectIdAutoField in django-mongodb-backend. I had some difficulty figuring out what exception get_prep_value() should raise. I have to do some more digging to understand if IntegerFielld.get_prep_value() raising ValueError and TypeError is really appropriate, but this is the subject of another ticket.

Change History (5)

comment:1 by Sarah Boyce, 20 months ago

Triage Stage: Unreviewed → Accepted

Thank you! Confirmed this is inconsistently handled. The following test fails for example:

  • tests/model_forms/tests.py

    a b from .models import (  
    7474    WriterProfile,
    7575    temp_storage_dir,
    7676    test_images,
     77    UUIDPK,
    7778)
    7879
    7980if test_images:
    … … class ModelMultipleChoiceFieldTests(TestCase):  
    21772178        with self.assertRaises(ValidationError):
    21782179            f.clean([c6.id])
    21792180
     2181    def test_model_multiple_choice_invalid_pk_value_error_messages(self):
     2182        uuid_f = forms.ModelMultipleChoiceField(UUIDPK.objects.all())
     2183        f = forms.ModelMultipleChoiceField(Category.objects.all())
     2184        for model_multiple_choice_form in [f, uuid_f]:
     2185            with self.assertRaisesMessage(
     2186                ValidationError,
     2187                "“invalid” is not a valid value."
     2188            ):
     2189                model_multiple_choice_form.clean(["invalid"])
     2190
    21802191    def test_model_multiple_choice_required_false(self):

comment:2 by JaeHyuckSa, 20 months ago

Owner: set to JaeHyuckSa
Status: new → assigned

comment:3 by JaeHyuckSa, 20 months ago

Has patch: set

comment:4 by Tim Graham, 19 months ago

Triage Stage: Accepted → Ready for checkin

Looks good. A test may be relocated per my comment.

comment:5 by Sarah Boyce <42296566+sarahboyce@…>, 19 months ago

Resolution: → fixed
Status: assigned → closed

In f480d5d3:

Fixed #36201 -- Caught ValidationError in ModelChoiceField/ModelMultipleChoiceField.clean().

Signed-off-by: saJaeHyukc <wogur981208@…>

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