Opened 2 years ago

Last modified 15 months ago

#25594 new New feature

Difficult to customize model field default_validators and have them used on both model and form fields

Reported by: Marcin Nowak Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

There is incosistency between model's URLField and modelform's URLField validation.
Form's URLField does not use model's field validation.

Form and model fields have declared class properties with own validators, whose defaults to validators.URLValidator(). But when you customize validator for model field, the form is still using own (and bad in this context) instance. As a result validation differs for form and model. And this is an incosistency between modelforms and models.

Change History (4)

comment:1 Changed 2 years ago by Tim Graham

Component: FormsDatabase layer (models, ORM)
Summary: ModelForm`s URLField does not use model`s URLField validatorDifficult to customize model field default_validators and have them used on both model and form fields
Triage Stage: UnreviewedAccepted
Type: BugNew feature
Version: 1.8master

This problem exists for more than just URLField so I'm going to retitle the ticket to reflect that. Using the use case from #25593 of customizing a URLValidator's accepted schemes, my idea would be to allow something like:

URLField(validators=[URLValidator(schemes=[])])

and have that validator be used in place of the default validator on both the model and form field. Thinking out loud: would it be acceptable to replace any default_validators with values from validators if the value from validators is a subclass of a value in default_validators? I guess for something like RegexValidator you may want to have multiple regular expressions so we probably need a different way to say "use this validator to replace the default validators". Maybe a default_validators argument to model fields? This change seems tricky and likely needs a discussion on the DevelopersMailingList to nail down the API details, but I agree it'd be a worthwhile improvement.

comment:2 Changed 2 years ago by Claude Paroz

Thinking out loud, too: provide a NO_DEFAULT_VALIDATORS constant/class, when added to the validators list prevent using the field's default validators.

comment:3 Changed 15 months ago by Curtis Maloney

From looking at the code, I suspect the existing reasoning would be as follows:

  • Model fields have default validators, and devs can append to this list
  • Form fields have default validators, and devs can append to this list
  • When performing a clean on a ModelForm, it will invoke Model validation

So, if you reconfigure your model field to be more restrictive, this isn't a problem as it will catch the issue.

However, if you make it more permissive, this will cause problems in your form.

comment:4 Changed 15 months ago by Claude Paroz

Has patch: set
Patch needs improvement: set

I created a pull request with my suggestions: PR.

It's not completely ready mainly because more tests should be added. But I'd like to hear opinions on the design before going too far. It has some backwards compatible issues (one test needed changes). I will ask for comments on the mailing list.

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