django-modelcluster icon indicating copy to clipboard operation
django-modelcluster copied to clipboard

ChildFormSet does not validate unique_together constraints involving the foreign key

Open gasman opened this issue 10 years ago • 1 comments

Reported here: https://groups.google.com/d/msg/wagtail/zDe5ocpL5xA/_If71AkzCmkJ

With Django's InlineFormSet, this validation happens automatically as part of the standard is_valid method - no need to add a custom validator - so ChildFormSet should do the same. Have created a new branch with failing unit tests: https://github.com/gasman/django-modelcluster/tree/fix/unique_together_validation

It looks like InlineFormSet does this by adding the foreign key field to the form and setting it on the instances, so that ModelFormSet's validation is applied on it:

https://github.com/django/django/blob/681df1aeafb30092430157f7977f713e1ce234ca/django/forms/models.py#L888-893 https://github.com/django/django/blob/681df1aeafb30092430157f7977f713e1ce234ca/django/forms/models.py#L939-944

  • however, this won't work for us in that form, because we can't guarantee that the parent object has been assigned an ID. It looks like ModelFormSet's validation also ignores any groups of values involving None, which will affect us:

https://github.com/django/django/blob/681df1aeafb30092430157f7977f713e1ce234ca/django/forms/models.py#L676

gasman avatar May 12 '15 09:05 gasman

however, this won't work for us in that form, because we can't guarantee that the parent object has been assigned an ID.

Hang on, that makes no sense - InlineFormSet's is_valid has to be able to work in conjunction with unsaved parent models too, because that's what happens on any 'create model' view with inlines. Django must be doing something cleverer, I guess...

gasman avatar May 12 '15 10:05 gasman