Fixed #20522 - Allowed use of partially validated object in ModelAdmin.add_view formset validation.

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.

Thanks to meshy for report, and EvilDMP and timo for review.

django/contrib/admin/options.py

@@ -1261,7 +1261,7 @@ class ModelAdmin(BaseModelAdmin):
                 form_validated = True
             else:
                 form_validated = False
-                new_object = self.model()
+                new_object = form.instance
             formsets, inline_instances = self._create_formsets(request, new_object)
             if all_valid(formsets) and form_validated:
                 self.save_model(request, new_object, form, False)
@@ -1342,7 +1342,7 @@ class ModelAdmin(BaseModelAdmin):
                 new_object = self.save_form(request, form, change=True)
             else:
                 form_validated = False
-                new_object = obj
+                new_object = form.instance
             formsets, inline_instances = self._create_formsets(request, new_object)
             if all_valid(formsets) and form_validated:
                 self.save_model(request, new_object, form, True)

docs/ref/contrib/admin/index.txt

@@ -1870,6 +1870,12 @@ The ``InlineModelAdmin`` class adds:
     through to :func:`~django.forms.models.inlineformset_factory` when
     creating the formset for this inline.
 
+.. warning::
+    When writing custom validation for ``InlineModelAdmin`` forms, be cautious
+    of writing validation that relies on features of the parent model. If the
+    parent model fails to validate, it may be left in an inconsistent state as
+    described in the warning in :ref:`validation-on-modelform`.
+
 .. attribute:: InlineModelAdmin.extra
 
     This controls the number of extra forms the formset will display in

tests/admin_views/admin.py

@@ -7,6 +7,7 @@ import os
 from django import forms
 from django.contrib import admin
 from django.contrib.admin.views.main import ChangeList
+from django.core.exceptions import ValidationError
 from django.core.files.storage import FileSystemStorage
 from django.core.mail import EmailMessage
 from django.core.servers.basehttp import FileWrapper
@@ -31,7 +32,8 @@ from .models import (Article, Chapter, Account, Media, Child, Parent, Picture,
     AdminOrderedCallable, Report, Color2, UnorderedObject, MainPrepopulated,
     RelatedPrepopulated, UndeletableObject, UnchangeableObject, UserMessenger, Simple, Choice,
     ShortMessage, Telegram, FilteredManager, EmptyModelHidden,
-    EmptyModelVisible, EmptyModelMixin, State, City, Restaurant, Worker)
+    EmptyModelVisible, EmptyModelMixin, State, City, Restaurant, Worker,
+    ParentWithDependentChildren, DependentChild)
 
 
 def callable_year(dt_value):
@@ -716,6 +718,28 @@ class ChoiceList(admin.ModelAdmin):
     fields = ['choice']
 
 
+class DependentChildAdminForm(forms.ModelForm):
+    """
+    Issue #20522
+    Form to test child dependency on parent object's validation
+    """
+    def clean(self):
+        parent = self.cleaned_data.get('parent')
+        if parent.family_name and parent.family_name != self.cleaned_data.get('family_name'):
+            raise ValidationError("Children must share a family name with their parents " +
+                                  "in this contrived test case")
+        return super(DependentChildAdminForm, self).clean()
+
+
+class DependentChildInline(admin.TabularInline):
+    model = DependentChild
+    form = DependentChildAdminForm
+
+
+class ParentWithDependentChildrenAdmin(admin.ModelAdmin):
+    inlines = [DependentChildInline]
+
+
 # Tests for ticket 11277 ----------------------------------
 
 class FormWithoutHiddenField(forms.ModelForm):
@@ -872,6 +896,7 @@ site.register(Color2, CustomTemplateFilterColorAdmin)
 site.register(Simple, AttributeErrorRaisingAdmin)
 site.register(UserMessenger, MessageTestingAdmin)
 site.register(Choice, ChoiceList)
+site.register(ParentWithDependentChildren, ParentWithDependentChildrenAdmin)
 site.register(EmptyModelHidden, EmptyModelHiddenAdmin)
 site.register(EmptyModelVisible, EmptyModelVisibleAdmin)
 site.register(EmptyModelMixin, EmptyModelMixinAdmin)

tests/admin_views/models.py

@@ -712,6 +712,26 @@ class Choice(models.Model):
         choices=((1, 'Yes'), (0, 'No'), (None, 'No opinion')))
 
 
+class ParentWithDependentChildren(models.Model):
+    """
+    Issue #20522
+    Model where the validation of child foreign-key relationships depends
+    on validation of the parent
+    """
+    some_required_info = models.PositiveIntegerField()
+    family_name = models.CharField(max_length=255, blank=False)
+
+
+class DependentChild(models.Model):
+    """
+    Issue #20522
+    Model that depends on validation of the parent class for one of its
+    fields to validate during clean
+    """
+    parent = models.ForeignKey(ParentWithDependentChildren)
+    family_name = models.CharField(max_length=255)
+
+
 class _Manager(models.Manager):
     def get_queryset(self):
         return super(_Manager, self).get_queryset().filter(pk__gt=1)

tests/admin_views/tests.py

@@ -51,7 +51,8 @@ from .models import (Article, BarAccount, CustomArticle, EmptyModel, FooAccount,
     AdminOrderedModelMethod, AdminOrderedAdminMethod, AdminOrderedCallable,
     Report, MainPrepopulated, RelatedPrepopulated, UnorderedObject,
     Simple, UndeletableObject, UnchangeableObject, Choice, ShortMessage,
-    Telegram, Pizza, Topping, FilteredManager, City, Restaurant, Worker)
+    Telegram, Pizza, Topping, FilteredManager, City, Restaurant, Worker,
+    ParentWithDependentChildren)
 from .admin import site, site2, CityAdmin
 
 
@@ -4597,7 +4598,7 @@ class TestLabelVisibility(TestCase):
 
 
 @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',))
-class AdminViewOnSiteTest(TestCase):
+class AdminViewOnSiteTests(TestCase):
     urls = "admin_views.urls"
     fixtures = ['admin-views-users.xml', 'admin-views-restaurants.xml']
 
@@ -4607,6 +4608,64 @@ class AdminViewOnSiteTest(TestCase):
     def tearDown(self):
         self.client.logout()
 
+    def test_add_view_form_and_formsets_run_validation(self):
+        """
+        Issue #20522
+        Verifying that if the parent form fails validation, the inlines also
+        run validation even if validation is contingent on parent form data
+        """
+        # The form validation should fail because 'some_required_info' is
+        # not included on the parent form, and the family_name of the parent
+        # does not match that of the child
+        post_data = {"family_name": "Test1",
+                     "dependentchild_set-TOTAL_FORMS": "1",
+                     "dependentchild_set-INITIAL_FORMS": "0",
+                     "dependentchild_set-MAX_NUM_FORMS": "1",
+                     "dependentchild_set-0-id": "",
+                     "dependentchild_set-0-parent": "",
+                     "dependentchild_set-0-family_name": "Test2"}
+        response = self.client.post('/test_admin/admin/admin_views/parentwithdependentchildren/add/',
+                                    post_data)
+
+        # just verifying the parent form failed validation, as expected --
+        # this isn't the regression test
+        self.assertTrue('some_required_info' in response.context['adminform'].form.errors)
+
+        # actual regression test
+        for error_set in response.context['inline_admin_formset'].formset.errors:
+            self.assertEqual(['Children must share a family name with their parents in this contrived test case'],
+                              error_set.get('__all__'))
+
+    def test_change_view_form_and_formsets_run_validation(self):
+        """
+        Issue #20522
+        Verifying that if the parent form fails validation, the inlines also
+        run validation even if validation is contingent on parent form data
+        """
+        pwdc = ParentWithDependentChildren.objects.create(some_required_info=6,
+                                                          family_name="Test1")
+        # The form validation should fail because 'some_required_info' is
+        # not included on the parent form, and the family_name of the parent
+        # does not match that of the child
+        post_data = {"family_name": "Test2",
+                     "dependentchild_set-TOTAL_FORMS": "1",
+                     "dependentchild_set-INITIAL_FORMS": "0",
+                     "dependentchild_set-MAX_NUM_FORMS": "1",
+                     "dependentchild_set-0-id": "",
+                     "dependentchild_set-0-parent": str(pwdc.id),
+                     "dependentchild_set-0-family_name": "Test1"}
+        response = self.client.post('/test_admin/admin/admin_views/parentwithdependentchildren/%d/'
+                                    % pwdc.id, post_data)
+
+        # just verifying the parent form failed validation, as expected --
+        # this isn't the regression test
+        self.assertTrue('some_required_info' in response.context['adminform'].form.errors)
+
+        # actual regression test
+        for error_set in response.context['inline_admin_formset'].formset.errors:
+            self.assertEqual(['Children must share a family name with their parents in this contrived test case'],
+                              error_set.get('__all__'))
+
     def test_validate(self):
         "Ensure that the view_on_site value is either a boolean or a callable"
         CityAdmin.view_on_site = True