I understand that the current way of doing things may stop invalid data being sent to the formsets but I believe, as this is the data that has been submitted, and the majority of it will likely be correct, that this would be better than the nothing we currently get.

I propose that lines 994 and 1091 both change to new_object = form.instance. This would mean that in a fair number of cases the errors on the main form would not need to be fixed before the errors on the inline form become visible.

I hope that my concerns/intention are clear, but if not, please ask and I will produce a working example and patch for examination.

I was able to confirm the first part of the bug (add_view), but could not replicate any bug for change_view. Please see the second screenshot attached to this bug for an example of the latter. I have also created a repository for test code to manually duplicate the issue (​https://github.com/kamni/django_20522). Instructions for duplicating these findings are in the README, and can be used later to confirm that the bug has been fixed.

In order to test this bug, we need two models where the validation of one of the child's fields is contingent on the validation of one of the parent's fields, with a special admin that handled this validation. I was hesitant to alter an existing set of models/admin out of concern of breaking other testing.

I already confirmed that adding the new models was valid in this particular case with EvilDMP and one other core dev at the sprints before submitting this patch. If you have suggestions about which models I should change instead, please let me know.

"The cleaning process modifies the model instance passed to the ModelForm constructor in various ways. For instance, any date fields on the model are converted into actual date objects. Failed validation may leave the underlying model instance in an inconsistent state and therefore it’s not recommended to reuse it."

As the OP points out, hopefully "this will be better than the nothing we currently get", but I mention it in case a documentation update might be helpful somewhere. It seems like it could actually break existing formset validation code if model attributes are different types when the form is invalid which makes me a little nervous.

This change would be specific to contrib.admin so the documentation would go there (along with a backwards incompatible note in the release notes). I guess I'm -0 on this change for the backwards compatibility concern and the advice in our own docs that we shouldn't reuse model instances that have failed validation. Feel free to persuade me otherwise (or find another core dev who really wants this).

So one thing to consider: as you pointed out for the tests, the change_view tests already pass with the old code (i.e., the bug is only for the 'add' view -- see comment 4). I even considered the possibility that my tests were running validation against the old version of the model and that's why it was passing; but when I updated the tests so that the formset matched the expected data from the old version but the updated version would cause invalid fields (i.e., the tests should fail if the code wasn't reusing a model with failed validation), the tests still passed with the old options.py.

This seems to imply that the change_view is already disregarding the "Don't re-use models that have failed validation". Clearly we have inconsistent behavior between two views that needs to be remedied in one way or another.

So should we move forward with this ticket and make the add_view consistent with what change_view already seems to be doing, or should we open a new ticket to revert the change_view's behavior to be more consistent with ​Validation on a ModelForm?

I also added a second commit with the requested documentation regarding ​Validation on a ModelForm. It can be found in contrib.admin.index under the InlineModelAdmin.form section. A review for the docs would be much appreciated.

Updated ModelAdmin to use form.instance when passing parent model to
child inlines for add_view. There is effectively no change in the
change_view since the previously passed 'obj' is the same as form.instance.