Opened 12 years ago

Closed 10 years ago

#19671 closed Bug (fixed)

Model field option "validators" not working with ManyToManyField

Reported by: fabio@… 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)

test_m2m_validator_called.diff (2.2 KB ) - added by fhahn 12 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 12 years ago.
Same test updated to master
19671_patch1.diff (2.8 KB ) - added by ANUBHAV JOSHI 10 years ago.

Download all attachments as: .zip

Change History (12)

by fhahn, 12 years ago

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

comment:1 by fhahn, 12 years ago

Cc: flo@… added
Component: UncategorizedForms
Owner: changed from nobody to fhahn
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 by Carl Meyer, 12 years ago

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.

by Claude Paroz, 12 years ago

Same test updated to master

by ANUBHAV JOSHI, 10 years ago

Attachment: 19671_patch1.diff added

comment:3 by ANUBHAV JOSHI, 10 years ago

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

comment:4 by ANUBHAV JOSHI, 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.

Version 3, edited 10 years ago by ANUBHAV JOSHI (previous) (next) (diff)

comment:5 by ANUBHAV JOSHI, 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 loic84, 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 loic84, 10 years ago

Cc: loic84 added

comment:9 by Tim Graham <timograham@…>, 10 years ago

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