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

Thanks Tim Graham for the review.
This commit is contained in:
Simon Charette 2014-10-14 14:56:39 -04:00
parent 19242c675f
commit f2ddc439b1
6 changed files with 93 additions and 11 deletions

View File

@ -1,13 +1,39 @@
import inspect
import warnings
from django.core.exceptions import ImproperlyConfigured from django.core.exceptions import ImproperlyConfigured
from django.forms import models as model_forms from django.forms import models as model_forms
from django.http import HttpResponseRedirect 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.utils.encoding import force_text
from django.views.generic.base import TemplateResponseMixin, ContextMixin, View from django.views.generic.base import TemplateResponseMixin, ContextMixin, View
from django.views.generic.detail import (SingleObjectMixin, from django.views.generic.detail import (SingleObjectMixin,
SingleObjectTemplateResponseMixin, BaseDetailView) 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. 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 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. 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()) return form_class(**self.get_form_kwargs())
def get_form_kwargs(self): def get_form_kwargs(self):
@ -156,8 +184,7 @@ class ProcessFormView(View):
""" """
Handles GET requests and instantiates a blank version of the form. Handles GET requests and instantiates a blank version of the form.
""" """
form_class = self.get_form_class() form = self.get_form()
form = self.get_form(form_class)
return self.render_to_response(self.get_context_data(form=form)) return self.render_to_response(self.get_context_data(form=form))
def post(self, request, *args, **kwargs): def post(self, request, *args, **kwargs):
@ -165,8 +192,7 @@ class ProcessFormView(View):
Handles POST requests, instantiating a form instance with the passed Handles POST requests, instantiating a form instance with the passed
POST variables and then checked for validity. POST variables and then checked for validity.
""" """
form_class = self.get_form_class() form = self.get_form()
form = self.get_form(form_class)
if form.is_valid(): if form.is_valid():
return self.form_valid(form) return self.form_valid(form)
else: else:

View File

@ -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 * The ``original_content_type_id`` attribute on
``django.contrib.admin.helpers.InlineAdminForm`` will be removed. ``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: .. _deprecation-removed-in-1.9:
1.9 1.9

View File

@ -49,10 +49,15 @@ FormMixin
Retrieve the form class to instantiate. By default Retrieve the form class to instantiate. By default
:attr:`.form_class`. :attr:`.form_class`.
.. method:: get_form(form_class) .. method:: get_form(form_class=None)
Instantiate an instance of ``form_class`` using Instantiate an instance of ``form_class`` using
:meth:`~django.views.generic.edit.FormMixin.get_form_kwargs`. :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() .. method:: get_form_kwargs()

View File

@ -242,6 +242,10 @@ Generic Views
:meth:`~django.views.generic.detail.SingleObjectMixin.get_object()` :meth:`~django.views.generic.detail.SingleObjectMixin.get_object()`
so that it'll perform its lookup using both the primary key and the slug. 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 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 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 to construct the "view on site" URL. This URL is now accessible using the
``absolute_url`` attribute of the form. ``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.

View File

@ -463,16 +463,14 @@ Our new ``AuthorDetail`` looks like this::
def get_context_data(self, **kwargs): def get_context_data(self, **kwargs):
context = super(AuthorDetail, self).get_context_data(**kwargs) context = super(AuthorDetail, self).get_context_data(**kwargs)
form_class = self.get_form_class() context['form'] = self.get_form()
context['form'] = self.get_form(form_class)
return context return context
def post(self, request, *args, **kwargs): def post(self, request, *args, **kwargs):
if not request.user.is_authenticated(): if not request.user.is_authenticated():
return HttpResponseForbidden() return HttpResponseForbidden()
self.object = self.get_object() self.object = self.get_object()
form_class = self.get_form_class() form = self.get_form()
form = self.get_form(form_class)
if form.is_valid(): if form.is_valid():
return self.form_valid(form) return self.form_valid(form)
else: else:

View File

@ -1,12 +1,14 @@
from __future__ import unicode_literals from __future__ import unicode_literals
from unittest import expectedFailure from unittest import expectedFailure
import warnings
from django.core.exceptions import ImproperlyConfigured from django.core.exceptions import ImproperlyConfigured
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django import forms from django import forms
from django.test import TestCase, override_settings from django.test import TestCase, override_settings
from django.test.client import RequestFactory from django.test.client import RequestFactory
from django.utils.deprecation import RemovedInDjango20Warning
from django.views.generic.base import View from django.views.generic.base import View
from django.views.generic.edit import FormMixin, ModelFormMixin, CreateView from django.views.generic.edit import FormMixin, ModelFormMixin, CreateView
@ -40,6 +42,43 @@ class FormMixinTests(TestCase):
set_kwargs = set_mixin.get_form_kwargs() set_kwargs = set_mixin.get_form_kwargs()
self.assertEqual(test_string, set_kwargs.get('prefix')) 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') @override_settings(ROOT_URLCONF='generic_views.urls')
class BasicFormTests(TestCase): class BasicFormTests(TestCase):