Opened 12 years ago
Closed 10 years ago
#19671 closed Bug (fixed)
Model field option "validators" not working with ManyToManyField
Reported by: | Owned by: | ANUBHAV JOSHI | |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | validators ManyToManyField |
Cc: | flo@…, loic84 | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
# models.py def validate_depends_on(value): # just raise an error, whatever the value raise ValidationError('Error') class Package(models.Model): # ... depends_on = models.ManyToManyField( 'self', symmetrical=False, related_name='required_by', blank=True, null=True, validators=[validate_depends_on] )
My custom validator is not called when saving a model instance through the admin. The same validator perfectly works with other fields. Also, removing blank and null options didn't help.
Attachments (3)
Change History (12)
by , 12 years ago
Attachment: | test_m2m_validator_called.diff added |
---|
comment:1 by , 12 years ago
Cc: | added |
---|---|
Component: | Uncategorized → Forms |
Owner: | changed from | to
Status: | new → assigned |
Version: | 1.4 → master |
I've added a test that checks the behavior described by the author of the ticket.
I think part of the problem is, that m2m validation cannot be done before saving the object, because the object needs a pk before a m2m related object can be added (using .add()). But the validator could be used during form validation. If not, we should at least document that the validators kwarg does not work for ManyToManyField (the null and blank kwarg are ignored too).
comment:2 by , 12 years ago
Triage Stage: | Unreviewed → Accepted |
---|
Not immediately obvious to me whether this will be reasonably fixable or not, but if not then a documentation fix is still clearly required.
by , 10 years ago
Attachment: | 19671_patch1.diff added |
---|
comment:3 by , 10 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
comment:4 by , 10 years ago
As Florian(fhahn) says that validators could be used in case of forms, I too am of the same opinion because in case of forms, when we provide the entire data all at once, we can surely run the validators on it when full_clean()
on ModelForm
is called, for which I have attached a patch.
And also we should update the docs saying that validators for M2M have meaning only when ModelForm is used as otherwise they can't because the m2m validators play no role in model instance
's full_clean()
is called, but definitely when form's full_clean()
is being called as it is part of form data and must be validated.
The patch is also available at https://github.com/coder9042/django/compare/gsoc_19671
comment:5 by , 10 years ago
Regarding the kwargs null
and blank
. They do not seem to be meaningful in case of M2M as if either is null then there is actually no relation, so it should be mentioned in the docs.
comment:6 by , 10 years ago
Proposed patch from Anubhav tries to reuse ManyToManyField.validators
at the form level and I don't think we should do that: model field validators are for model validation only.
It was proposed on IRC to make obj.m2m.add()
use the validators, but that's conflicting with the design decision that model validation should be completely optional.
Regarding null
it obviously has no meaning on a M2M since there is no corresponding field on the model. For blank
I'm pretty sure that model validation would completely ignore it as checking it would require an additional query; however, model forms and admin might, so it's worth double checking.
I'm leaning towards a docs patch, and possibly system checks.
comment:7 by , 10 years ago
Cc: | added |
---|
comment:9 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
test that checks if a custom validator of a m2m field gets called during form validation