diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index a69356e9e2..c7980116b8 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -1425,7 +1425,7 @@ class ModelAdmin(BaseModelAdmin): else: form_validated = False new_object = form.instance - formsets, inline_instances = self._create_formsets(request, new_object) + formsets, inline_instances = self._create_formsets(request, new_object, change=not add) if all_valid(formsets) and form_validated: self.save_model(request, new_object, form, not add) self.save_related(request, form, formsets, not add) @@ -1440,10 +1440,10 @@ class ModelAdmin(BaseModelAdmin): if add: initial = self.get_changeform_initial_data(request) form = ModelForm(initial=initial) - formsets, inline_instances = self._create_formsets(request, self.model()) + formsets, inline_instances = self._create_formsets(request, self.model(), change=False) else: form = ModelForm(instance=obj) - formsets, inline_instances = self._create_formsets(request, obj) + formsets, inline_instances = self._create_formsets(request, obj, change=True) adminForm = helpers.AdminForm( form, @@ -1729,13 +1729,13 @@ class ModelAdmin(BaseModelAdmin): "admin/object_history.html" ], context, current_app=self.admin_site.name) - def _create_formsets(self, request, obj): + def _create_formsets(self, request, obj, change): "Helper function to generate formsets for add/change_view." formsets = [] inline_instances = [] prefixes = {} get_formsets_args = [request] - if obj.pk: + if change: get_formsets_args.append(obj) for FormSet, inline in self.get_formsets_with_inlines(*get_formsets_args): prefix = FormSet.get_default_prefix() diff --git a/docs/releases/1.7.2.txt b/docs/releases/1.7.2.txt index c01dca9f58..85ebd1a6dc 100644 --- a/docs/releases/1.7.2.txt +++ b/docs/releases/1.7.2.txt @@ -94,3 +94,7 @@ Bugfixes * Fixed a crash when ``RunSQL`` SQL content was collected by the schema editor, typically when using ``sqlmigrate`` (:ticket:`23909`). + +* Fixed a regression in ``contrib.admin`` add/change views which caused some + ``ModelAdmin`` methods to receive the incorrect ``obj`` value + (:ticket:`23934`). diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index 2493c309ce..b81de1d4ac 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -18,7 +18,8 @@ from django.contrib.admin import BooleanFieldListFilter from django.utils.safestring import mark_safe from django.utils.six import StringIO -from .models import (Article, Chapter, Child, Parent, Picture, Widget, +from .models import ( + Article, Chapter, Child, Parent, Picture, Widget, DooHickey, Grommet, Whatsit, FancyDoodad, Category, Link, PrePopulatedPost, PrePopulatedSubPost, CustomArticle, Section, ModelWithStringPrimaryKey, Color, Thing, Actor, Inquisition, Sketch, @@ -37,7 +38,9 @@ from .models import (Article, Chapter, Child, Parent, Picture, Widget, State, City, Restaurant, Worker, ParentWithDependentChildren, DependentChild, StumpJoke, FieldOverridePost, FunkyTag, ReferencedByParent, ChildOfReferer, ReferencedByInline, - InlineReference, InlineReferer, Recipe, Ingredient, NotReferenced) + InlineReference, InlineReferer, Recipe, Ingredient, NotReferenced, + ExplicitlyProvidedPK, ImplicitlyGeneratedPK, +) def callable_year(dt_value): @@ -842,6 +845,25 @@ class InlineRefererAdmin(admin.ModelAdmin): inlines = [InlineReferenceInline] +class GetFormsetsArgumentCheckingAdmin(admin.ModelAdmin): + fields = ['name'] + + def add_view(self, request, *args, **kwargs): + request.is_add_view = True + return super(GetFormsetsArgumentCheckingAdmin, self).add_view(request, *args, **kwargs) + + def change_view(self, request, *args, **kwargs): + request.is_add_view = False + return super(GetFormsetsArgumentCheckingAdmin, self).change_view(request, *args, **kwargs) + + def get_formsets_with_inlines(self, request, obj=None): + if request.is_add_view and obj is not None: + raise Exception("'obj' passed to get_formsets_with_inlines wasn't None during add_view") + if not request.is_add_view and obj is None: + raise Exception("'obj' passed to get_formsets_with_inlines was None during change_view") + return super(GetFormsetsArgumentCheckingAdmin, self).get_formsets_with_inlines(request, obj) + + site = admin.AdminSite(name="admin") site.site_url = '/my-site-url/' site.register(Article, ArticleAdmin) @@ -942,6 +964,8 @@ site.register(StumpJoke) site.register(Recipe) site.register(Ingredient) site.register(NotReferenced) +site.register(ExplicitlyProvidedPK, GetFormsetsArgumentCheckingAdmin) +site.register(ImplicitlyGeneratedPK, GetFormsetsArgumentCheckingAdmin) # Register core models we need in our tests from django.contrib.auth.models import User, Group diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index abf6035e86..6c12030341 100644 --- a/tests/admin_views/models.py +++ b/tests/admin_views/models.py @@ -874,3 +874,12 @@ class RecipeIngredient(models.Model): class NotReferenced(models.Model): # Don't point any FK at this model. pass + + +# Models for #23934 +class ExplicitlyProvidedPK(models.Model): + name = models.IntegerField(primary_key=True) + + +class ImplicitlyGeneratedPK(models.Model): + name = models.IntegerField(unique=True) diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 6482786928..2eedae7ea6 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -5207,3 +5207,51 @@ class TestEtagWithAdminView(TestCase): response = self.client.get('/test_admin/admin/') self.assertEqual(response.status_code, 302) self.assertTrue(response.has_header('ETag')) + + +@override_settings( + PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',), + ROOT_URLCONF="admin_views.urls", +) +class GetFormsetsWithInlinesArgumentTest(TestCase): + """ + #23934 - When adding a new model instance in the admin, the 'obj' argument + of get_formsets_with_inlines() should be None. When changing, it should be + equal to the existing model instance. + The GetFormsetsArgumentCheckingAdmin ModelAdmin throws an exception + if obj is not None during add_view or obj is None during change_view. + """ + fixtures = ['admin-views-users.xml'] + + def setUp(self): + self.client.login(username='super', password='secret') + + def test_explicitly_provided_pk(self): + post_data = {'name': '1'} + try: + response = self.client.post('/test_admin/admin/admin_views/explicitlyprovidedpk/add/', post_data) + except Exception as e: + self.fail(e) + self.assertEqual(response.status_code, 302) + + post_data = {'name': '2'} + try: + response = self.client.post('/test_admin/admin/admin_views/explicitlyprovidedpk/1/', post_data) + except Exception as e: + self.fail(e) + self.assertEqual(response.status_code, 302) + + def test_implicitly_generated_pk(self): + post_data = {'name': '1'} + try: + response = self.client.post('/test_admin/admin/admin_views/implicitlygeneratedpk/add/', post_data) + except Exception as e: + self.fail(e) + self.assertEqual(response.status_code, 302) + + post_data = {'name': '2'} + try: + response = self.client.post('/test_admin/admin/admin_views/implicitlygeneratedpk/1/', post_data) + except Exception as e: + self.fail(e) + self.assertEqual(response.status_code, 302)