Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #20522 - Allowed use of partially validated object in ModelAdmi…

…n.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.
  • Loading branch information...
commit c74504c2dd21974571ab72805fbfc8d4d76ce151 1 parent 1c7a83e
@kamni kamni authored timgraham committed
View
4 django/contrib/admin/options.py
@@ -1261,7 +1261,7 @@ def add_view(self, request, form_url='', extra_context=None):
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 @@ def change_view(self, request, object_id, form_url='', extra_context=None):
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)
View
6 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
View
27 tests/admin_views/admin.py
@@ -7,6 +7,7 @@
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 @@
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 @@ class RestaurantAdmin(admin.ModelAdmin):
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)
View
20 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)
View
63 tests/admin_views/tests.py
@@ -51,7 +51,8 @@
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 @@ def assert_fieldline_hidden(self, response):
@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 @@ def setUp(self):
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
Please sign in to comment.
Something went wrong with that request. Please try again.