AndrewIngram / django-extra-views

Django's class-based generic views are awesome, let's have more of them.
MIT License
1.39k stars 172 forks source link

Validation of Inline Formset Based on Outer Form #220

Open idahogray opened 3 years ago

idahogray commented 3 years ago

Background

It took me a while to figure this out so I wanted to see if there was a way to simplify. I have a case where I want to make sure there are at least 1 valid form in a formset which is rendered as part of a larger form. However, the requirement to have at least one valid form is only needed if a field in the main form is in a certain state. I will try to explain this in the table below.

Form Object State At least 1 valid form required in an inline formset
Preliminary No
Ready for Approval Yes
Signatures Pending Yes
Approved Yes

The tricky part for me was that you can't do the validation in the form rendered in the formset because it requires knowledge of multiple forms in the formset. You can't do the validation at the formset level (including validate_min) because you need to know the state of the object in the outer form. You can't do the validation in the outer form because you need knowledge of the forms in the formset. Therefore, you can only do the validation in the CreateWithInlinesView and UpdateWithInlinesView.

The way that I was able to solve this was to override the post() method of CreateWithInlinesView, UpdateWithInlinesView, and their parent class ProcessFormWithInlinesView.

Suggestion

Now that the background is out of the way, my suggestion is to add a new validation step to the post() method in order to perform this type of validation where information is needed from the form and the inline formset. I would be happy to contribute something if you agree this is a worthwhile endeavor.

Thank you for a very useful library.

bernd-wechner commented 3 years ago

One idea is to put a hidden field in the inner forms, given then a class of their own, and then in the outer form, when it's state changes, copy that state to this inner field. The inner field is then available to the inner forms when they are posted, but to validate them in Django may need to have a model field associated with them, not sure.

sdolemelipone commented 3 years ago

Hi @idahogray, thanks for the detailed report of the problem you had and how you fixed it.

I agree the solution you've come up with in the post() is the best way to deal with validation across multiple forms/formsets. ProcessFormWithInlinesView.post() is a bit messy and an intermediate method could add some clarity and help with overloading. Perhaps something called ProcessFormWithInlinesView.validate() which returns True or False?

Then in your code you would overload the new method to carry out your bespoke validation.

Please go ahead and submit a pull request if you'd like to do so.

danizen commented 3 years ago

For one of my concrete sub-classes of UpdateWithInlinesView, my override of forms_valid is getting very long. I was just thinking of something similar to what is proposed above and went to see whether others had thought of this. Definitely better to delegate this from the class-based view to the form associated with the main model.

I don't see much point in a method called validate on the view, because forms_valid is already in the view.

I think it would be better to do something like this in ProcessFormWithInlinesView.post():

    valid_together = getattr(form, 'valid_together', None)
    if isinstance(valid_together, Callable):
         form.valid_together(inlines)

extra_views could also provide a mixin with an abstract implementation but I prefer it a bit looser I guess. The issue is that form.cleaned_data is copied into form.instance after is_valid(), so that we would want to call the right thing to do this again after valid together.

danizen commented 3 years ago

I maybe have a better idea. Maybe we can check if the form class is an instance of new marker mixins called something like "FormsetAwareForm" and "InlinesAwareForm". If so, send the formset as a kwarg to the form's constructor. Or, send the inlines as a kwarg to the form's constructor. Then, the form's clean method has an instance variable for formset or inlines.

This would mean more re-arrangement of code order, I think.