Fixed #23934 -- Fixed regression in admin views obj parameter.

5 files changed, 92 insertions, 7 deletions

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)