Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ChildFormSet does not validate unique_together constraints involving the foreign key #28

Open
gasman opened this issue May 12, 2015 · 1 comment
Labels

Comments

@gasman
Copy link
Contributor

gasman commented May 12, 2015

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
Copy link
Contributor Author

gasman commented May 12, 2015

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant