Opened 7 weeks ago

Closed 4 weeks ago

#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, 7 weeks ago

Triage Stage: UnreviewedAccepted

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

  • TabularUnified 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, 7 weeks ago

Owner: set to JaeHyuckSa
Status: newassigned

comment:3 by JaeHyuckSa, 7 weeks ago

Has patch: set

comment:4 by Tim Graham, 5 weeks ago

Triage Stage: AcceptedReady for checkin

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

comment:5 by Sarah Boyce <42296566+sarahboyce@…>, 4 weeks ago

Resolution: fixed
Status: assignedclosed

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