Opened 4 years ago

Closed 2 years ago

#19671 closed Bug (fixed)

Model field option "validators" not working with ManyToManyField

Reported by: fabio@… Owned by: ANUBHAV JOSHI
Component: Forms Version: master
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)

test_m2m_validator_called.diff (2.2 KB) - added by fhahn 4 years ago.
test that checks if a custom validator of a m2m field gets called during form validation
test_m2m_validator_called-2.diff (2.2 KB) - added by Claude Paroz 4 years ago.
Same test updated to master
19671_patch1.diff (2.8 KB) - added by ANUBHAV JOSHI 2 years ago.

Download all attachments as: .zip

Change History (12)

Changed 4 years ago by fhahn

test that checks if a custom validator of a m2m field gets called during form validation

comment:1 Changed 4 years ago by fhahn

Cc: flo@… added
Component: UncategorizedForms
Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to fhahn
Patch needs improvement: unset
Status: newassigned
Version: 1.4master

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 Changed 4 years ago by Carl Meyer

Triage Stage: UnreviewedAccepted

Not immediately obvious to me whether this will be reasonably fixable or not, but if not then a documentation fix is still clearly required.

Changed 4 years ago by Claude Paroz

Same test updated to master

Changed 2 years ago by ANUBHAV JOSHI

Attachment: 19671_patch1.diff added

comment:3 Changed 2 years ago by ANUBHAV JOSHI

Has patch: set
Owner: changed from fhahn to ANUBHAV JOSHI

comment:4 Changed 2 years ago by ANUBHAV JOSHI

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

Last edited 2 years ago by ANUBHAV JOSHI (previous) (diff)

comment:5 Changed 2 years ago by ANUBHAV JOSHI

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 Changed 2 years ago by loic84

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 Changed 2 years ago by loic84

Cc: loic84 added

comment:9 Changed 2 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 011abb7d96c75f6154c15a8a0f997d8c27698679:

Fixed #19671 -- Added warnings that null and validators are ignored for ManyToManyField.

Thanks Loic Bistuer and Tim Graham for help and review.

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