From f2ddc439b1938acb6cae693bda9d8cf83a4583be Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Tue, 14 Oct 2014 14:56:39 -0400 Subject: [PATCH] Fixed #23656 -- Made FormMixin.get_form's form_class argument optional. Thanks Tim Graham for the review. --- django/views/generic/edit.py | 38 +++++++++++++++--- docs/internals/deprecation.txt | 3 ++ docs/ref/class-based-views/mixins-editing.txt | 7 +++- docs/releases/1.8.txt | 11 ++++++ docs/topics/class-based-views/mixins.txt | 6 +-- tests/generic_views/test_edit.py | 39 +++++++++++++++++++ 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):