Fixed #23656 -- Made FormMixin.get_form's form_class argument optional.

Thanks Tim Graham for the review.

6 files changed, 93 insertions, 11 deletions

diff --git a/django/views/generic/edit.py b/django/views/generic/edit.py
index abc8b99ce2..4bcef9cb6a 100644
--- a/django/views/generic/edit.py
+++ b/django/views/generic/edit.py
@@ -1,13 +1,39 @@
+import inspect
+import warnings
+
 from django.core.exceptions import ImproperlyConfigured
 from django.forms import models as model_forms
 from django.http import HttpResponseRedirect
+from django.utils import six
+from django.utils.deprecation import RemovedInDjango20Warning
 from django.utils.encoding import force_text
 from django.views.generic.base import TemplateResponseMixin, ContextMixin, View
 from django.views.generic.detail import (SingleObjectMixin,
                         SingleObjectTemplateResponseMixin, BaseDetailView)
 
 
-class FormMixin(ContextMixin):
+class FormMixinBase(type):
+    def __new__(cls, name, bases, attrs):
+        get_form = attrs.get('get_form')
+        if get_form and inspect.isfunction(get_form):
+            try:
+                inspect.getcallargs(get_form, None)
+            except TypeError:
+                warnings.warn(
+                    "`%s.%s.get_form` method must define a default value for "
+                    "its `form_class` argument." % (attrs['__module__'], name),
+                    RemovedInDjango20Warning, stacklevel=2
+                )
+
+                def get_form_with_form_class(self, form_class=None):
+                    if form_class is None:
+                        form_class = self.get_form_class()
+                    return get_form(self, form_class=form_class)
+                attrs['get_form'] = get_form_with_form_class
+        return super(FormMixinBase, cls).__new__(cls, name, bases, attrs)
+
+
+class FormMixin(six.with_metaclass(FormMixinBase, ContextMixin)):
     """
     A mixin that provides a way to show and handle a form in a request.
     """
@@ -35,10 +61,12 @@ class FormMixin(ContextMixin):
         """
         return self.form_class
 
-    def get_form(self, form_class):
+    def get_form(self, form_class=None):
         """
         Returns an instance of the form to be used in this view.
         """
+        if form_class is None:
+            form_class = self.get_form_class()
         return form_class(**self.get_form_kwargs())
 
     def get_form_kwargs(self):
@@ -156,8 +184,7 @@ class ProcessFormView(View):
         """
         Handles GET requests and instantiates a blank version of the form.
         """
-        form_class = self.get_form_class()
-        form = self.get_form(form_class)
+        form = self.get_form()
         return self.render_to_response(self.get_context_data(form=form))
 
     def post(self, request, *args, **kwargs):
@@ -165,8 +192,7 @@ class ProcessFormView(View):
         Handles POST requests, instantiating a form instance with the passed
         POST variables and then checked for validity.
         """
-        form_class = self.get_form_class()
-        form = self.get_form(form_class)
+        form = self.get_form()
         if form.is_valid():
             return self.form_valid(form)
         else:
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index 9906119f14..b0fe326937 100644
--- a/docs/internals/deprecation.txt
+++ b/docs/internals/deprecation.txt
@@ -65,6 +65,9 @@ about each item can often be found in the release notes of two versions prior.
 * The ``original_content_type_id`` attribute on
   ``django.contrib.admin.helpers.InlineAdminForm`` will be removed.
 
+* The backwards compatibility shim to allow ``FormMixin.get_form()`` to be
+  defined with no default value for its ``form_class`` argument will be removed.
+
 .. _deprecation-removed-in-1.9:
 
 1.9
diff --git a/docs/ref/class-based-views/mixins-editing.txt b/docs/ref/class-based-views/mixins-editing.txt
index 7a315ad7a3..1a06e2bb97 100644
--- a/docs/ref/class-based-views/mixins-editing.txt
+++ b/docs/ref/class-based-views/mixins-editing.txt
@@ -49,10 +49,15 @@ FormMixin
         Retrieve the form class to instantiate. By default
         :attr:`.form_class`.
 
-    .. method:: get_form(form_class)
+    .. method:: get_form(form_class=None)
 
         Instantiate an instance of ``form_class`` using
         :meth:`~django.views.generic.edit.FormMixin.get_form_kwargs`.
+        If ``form_class`` isn't provided :meth:`get_form_class` will be used.
+
+        .. versionchanged:: 1.8
+
+            The ``form_class`` argument is not required anymore.
 
     .. method:: get_form_kwargs()
 
diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt
index dbdff7b7e0..26dad8d14b 100644
--- a/docs/releases/1.8.txt
+++ b/docs/releases/1.8.txt
@@ -242,6 +242,10 @@ Generic Views
   :meth:`~django.views.generic.detail.SingleObjectMixin.get_object()`
   so that it'll perform its lookup using both the primary key and the slug.
 
+* The :meth:`~django.views.generic.edit.FormMixin.get_form()` method doesn't
+  require a ``form_class`` to be provided anymore. If not provided ``form_class``
+  defaults to :meth:`~django.views.generic.edit.FormMixin.get_form_class()`.
+
 Internationalization
 ^^^^^^^^^^^^^^^^^^^^
 
@@ -892,3 +896,10 @@ The ``original_content_type_id`` attribute on ``InlineAdminForm`` has been
 deprecated and will be removed in Django 2.0. Historically, it was used
 to construct the "view on site" URL. This URL is now accessible using the
 ``absolute_url`` attribute of the form.
+
+``django.views.generic.edit.FormMixin.get_form()``’s ``form_class`` argument
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``FormMixin`` subclasses that override the ``get_form()`` method should make
+sure to provide a default value for the ``form_class`` argument since it's
+now optional.
diff --git a/docs/topics/class-based-views/mixins.txt b/docs/topics/class-based-views/mixins.txt
index 8918d6cbd2..172c4ad3ca 100644
--- a/docs/topics/class-based-views/mixins.txt
+++ b/docs/topics/class-based-views/mixins.txt
@@ -463,16 +463,14 @@ Our new ``AuthorDetail`` looks like this::
 
         def get_context_data(self, **kwargs):
             context = super(AuthorDetail, self).get_context_data(**kwargs)
-            form_class = self.get_form_class()
-            context['form'] = self.get_form(form_class)
+            context['form'] = self.get_form()
             return context
 
         def post(self, request, *args, **kwargs):
             if not request.user.is_authenticated():
                 return HttpResponseForbidden()
             self.object = self.get_object()
-            form_class = self.get_form_class()
-            form = self.get_form(form_class)
+            form = self.get_form()
             if form.is_valid():
                 return self.form_valid(form)
             else:
diff --git a/tests/generic_views/test_edit.py b/tests/generic_views/test_edit.py
index f58b82b88a..9f152c9af3 100644
--- a/tests/generic_views/test_edit.py
+++ b/tests/generic_views/test_edit.py
@@ -1,12 +1,14 @@
 from __future__ import unicode_literals
 
 from unittest import expectedFailure
+import warnings
 
 from django.core.exceptions import ImproperlyConfigured
 from django.core.urlresolvers import reverse
 from django import forms
 from django.test import TestCase, override_settings
 from django.test.client import RequestFactory
+from django.utils.deprecation import RemovedInDjango20Warning
 from django.views.generic.base import View
 from django.views.generic.edit import FormMixin, ModelFormMixin, CreateView
 
@@ -40,6 +42,43 @@ class FormMixinTests(TestCase):
         set_kwargs = set_mixin.get_form_kwargs()
         self.assertEqual(test_string, set_kwargs.get('prefix'))
 
+    def test_get_form(self):
+        class TestFormMixin(FormMixin):
+            request = RequestFactory().get('/')
+
+        self.assertIsInstance(
+            TestFormMixin().get_form(forms.Form), forms.Form,
+            'get_form() should use provided form class.'
+        )
+
+        class FormClassTestFormMixin(TestFormMixin):
+            form_class = forms.Form
+
+        self.assertIsInstance(
+            FormClassTestFormMixin().get_form(), forms.Form,
+            'get_form() should fallback to get_form_class() if none is provided.'
+        )
+
+    def test_get_form_missing_form_class_default_value(self):
+        with warnings.catch_warnings(record=True) as w:
+            class MissingDefaultValue(FormMixin):
+                request = RequestFactory().get('/')
+                form_class = forms.Form
+
+                def get_form(self, form_class):
+                    return form_class(**self.get_form_kwargs())
+        self.assertEqual(len(w), 1)
+        self.assertEqual(w[0].category, RemovedInDjango20Warning)
+        self.assertEqual(
+            str(w[0].message),
+            '`generic_views.test_edit.MissingDefaultValue.get_form` method '
+            'must define a default value for its `form_class` argument.'
+        )
+
+        self.assertIsInstance(
+            MissingDefaultValue().get_form(), forms.Form,
+        )
+
 
 @override_settings(ROOT_URLCONF='generic_views.urls')
 class BasicFormTests(TestCase):