Opened 9 months ago
Closed 8 months 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 , 9 months ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 by , 9 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:3 by , 9 months ago
| Has patch: | set |
|---|
comment:4 by , 8 months ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Looks good. A test may be relocated per my comment.
Thank you! Confirmed this is inconsistently handled. The following test fails for example:
tests/model_forms/tests.py